Skip to content

Commit 8b43d45

Browse files
aali309aali309
committed
fix: GitOps Operator hot loop issue
Signed-off-by: Atif Ali <atali@redhat.com>
1 parent 0755156 commit 8b43d45

File tree

2 files changed

+48
-30
lines changed

2 files changed

+48
-30
lines changed

‎controllers/consoleplugin.go‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66
"os"
7-
"reflect"
87

98
argocommon "github.com/argoproj-labs/argocd-operator/common"
109
argocdutil "github.com/argoproj-labs/argocd-operator/controllers/argoutil"
@@ -16,6 +15,7 @@ import (
1615
corev1 "k8s.io/api/core/v1"
1716

1817
"github.com/redhat-developer/gitops-operator/common"
18+
"k8s.io/apimachinery/pkg/api/equality"
1919
"k8s.io/apimachinery/pkg/api/errors"
2020
resourcev1 "k8s.io/apimachinery/pkg/api/resource"
2121
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -55,10 +55,12 @@ func getPluginPodSpec(crImagePullPolicy corev1.PullPolicy) corev1.PodSpec {
5555
podSpec := corev1.PodSpec{
5656
Containers: []corev1.Container{
5757
{
58-
Env: util.ProxyEnvVars(),
59-
Name: gitopsPluginName,
60-
Image: consolePluginImage,
61-
ImagePullPolicy: argocdutil.GetImagePullPolicy(crImagePullPolicy),
58+
Env: util.ProxyEnvVars(),
59+
Name: gitopsPluginName,
60+
Image: consolePluginImage,
61+
ImagePullPolicy: argocdutil.GetImagePullPolicy(crImagePullPolicy),
62+
TerminationMessagePath: corev1.TerminationMessagePathDefault,
63+
TerminationMessagePolicy: corev1.TerminationMessageReadFile,
6264
Ports: []corev1.ContainerPort{
6365
{
6466
Name: "http",
@@ -309,17 +311,18 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop
309311
} else {
310312
existingSpecTemplate := &existingPluginDeployment.Spec.Template
311313
newSpecTemplate := newPluginDeployment.Spec.Template
312-
changed := !reflect.DeepEqual(existingPluginDeployment.Labels, newPluginDeployment.Labels) ||
313-
!reflect.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) ||
314-
!reflect.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) ||
315-
!reflect.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) ||
316-
!reflect.DeepEqual(existingSpecTemplate.Spec.Containers, newSpecTemplate.Spec.Containers) ||
317-
!reflect.DeepEqual(existingSpecTemplate.Spec.Volumes, newSpecTemplate.Spec.Volumes) ||
318-
!reflect.DeepEqual(existingSpecTemplate.Spec.RestartPolicy, newSpecTemplate.Spec.RestartPolicy) ||
319-
!reflect.DeepEqual(existingSpecTemplate.Spec.DNSPolicy, newSpecTemplate.Spec.DNSPolicy) ||
320-
!reflect.DeepEqual(existingPluginDeployment.Spec.Template.Spec.NodeSelector, newPluginDeployment.Spec.Template.Spec.NodeSelector) ||
321-
!reflect.DeepEqual(existingPluginDeployment.Spec.Template.Spec.Tolerations, newPluginDeployment.Spec.Template.Spec.Tolerations) ||
322-
!reflect.DeepEqual(existingSpecTemplate.Spec.SecurityContext, existingSpecTemplate.Spec.SecurityContext) || !reflect.DeepEqual(existingSpecTemplate.Spec.Containers[0].Resources, newSpecTemplate.Spec.Containers[0].Resources)
314+
// Use semantic equality instead of reflect.DeepEqual to handle Kubernetes defaults properly
315+
changed := !equality.Semantic.DeepEqual(existingPluginDeployment.Labels, newPluginDeployment.Labels) ||
316+
!equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) ||
317+
!equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) ||
318+
!equality.Semantic.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) ||
319+
!equality.Semantic.DeepEqual(existingSpecTemplate.Spec.Containers, newSpecTemplate.Spec.Containers) ||
320+
!equality.Semantic.DeepEqual(existingSpecTemplate.Spec.Volumes, newSpecTemplate.Spec.Volumes) ||
321+
!equality.Semantic.DeepEqual(existingSpecTemplate.Spec.RestartPolicy, newSpecTemplate.Spec.RestartPolicy) ||
322+
!equality.Semantic.DeepEqual(existingSpecTemplate.Spec.DNSPolicy, newSpecTemplate.Spec.DNSPolicy) ||
323+
!equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Template.Spec.NodeSelector, newPluginDeployment.Spec.Template.Spec.NodeSelector) ||
324+
!equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Template.Spec.Tolerations, newPluginDeployment.Spec.Template.Spec.Tolerations) ||
325+
!equality.Semantic.DeepEqual(existingSpecTemplate.Spec.SecurityContext, newSpecTemplate.Spec.SecurityContext) || !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.Containers[0].Resources, newSpecTemplate.Spec.Containers[0].Resources)
323326

324327
if changed {
325328
reqLogger.Info("Reconciling plugin deployment", "Namespace", existingPluginDeployment.Namespace, "Name", existingPluginDeployment.Name)
@@ -364,18 +367,18 @@ func (r *ReconcileGitopsService) reconcileService(instance *pipelinesv1alpha1.Gi
364367
return reconcile.Result{}, err
365368
}
366369
} else {
367-
changed := !reflect.DeepEqual(existingServiceRef.Annotations, pluginServiceRef.Annotations) ||
368-
!reflect.DeepEqual(existingServiceRef.Labels, pluginServiceRef.Labels) ||
369-
!reflect.DeepEqual(existingServiceRef.Spec.Selector, pluginServiceRef.Spec.Selector) ||
370-
!reflect.DeepEqual(existingServiceRef.Spec.Ports, pluginServiceRef.Spec.Ports)
370+
changed := !equality.Semantic.DeepEqual(existingServiceRef.Annotations, pluginServiceRef.Annotations) ||
371+
!equality.Semantic.DeepEqual(existingServiceRef.Labels, pluginServiceRef.Labels) ||
372+
!equality.Semantic.DeepEqual(existingServiceRef.Spec.Selector, pluginServiceRef.Spec.Selector) ||
373+
!equality.Semantic.DeepEqual(existingServiceRef.Spec.Ports, pluginServiceRef.Spec.Ports)
371374

372375
if changed {
373376
reqLogger.Info("Reconciling plugin service", "Namespace", existingServiceRef.Namespace, "Name", existingServiceRef.Name)
374377
existingServiceRef.Annotations = pluginServiceRef.Annotations
375378
existingServiceRef.Labels = pluginServiceRef.Labels
376379
existingServiceRef.Spec.Selector = pluginServiceRef.Spec.Selector
377380
existingServiceRef.Spec.Ports = pluginServiceRef.Spec.Ports
378-
return reconcile.Result{}, r.Client.Update(context.TODO(), pluginServiceRef)
381+
return reconcile.Result{}, r.Client.Update(context.TODO(), existingServiceRef)
379382
}
380383
}
381384
return reconcile.Result{}, nil
@@ -405,14 +408,14 @@ func (r *ReconcileGitopsService) reconcileConsolePlugin(instance *pipelinesv1alp
405408
return reconcile.Result{}, err
406409
}
407410
} else {
408-
changed := !reflect.DeepEqual(existingPlugin.Spec.DisplayName, newConsolePlugin.Spec.DisplayName) ||
409-
!reflect.DeepEqual(existingPlugin.Spec.Backend.Service, newConsolePlugin.Spec.Backend.Service)
411+
changed := !equality.Semantic.DeepEqual(existingPlugin.Spec.DisplayName, newConsolePlugin.Spec.DisplayName) ||
412+
!equality.Semantic.DeepEqual(existingPlugin.Spec.Backend.Service, newConsolePlugin.Spec.Backend.Service)
410413

411414
if changed {
412415
reqLogger.Info("Reconciling Console Plugin", "Namespace", existingPlugin.Namespace, "Name", existingPlugin.Name)
413416
existingPlugin.Spec.DisplayName = newConsolePlugin.Spec.DisplayName
414417
existingPlugin.Spec.Backend.Service = newConsolePlugin.Spec.Backend.Service
415-
return reconcile.Result{}, r.Client.Update(context.TODO(), newConsolePlugin)
418+
return reconcile.Result{}, r.Client.Update(context.TODO(), existingPlugin)
416419
}
417420
}
418421
return reconcile.Result{}, nil
@@ -440,13 +443,13 @@ func (r *ReconcileGitopsService) reconcileConfigMap(instance *pipelinesv1alpha1.
440443
return reconcile.Result{}, err
441444
}
442445
} else {
443-
changed := !reflect.DeepEqual(existingPluginConfigMap.Data, newPluginConfigMap.Data) ||
444-
!reflect.DeepEqual(existingPluginConfigMap.Labels, newPluginConfigMap.Labels)
446+
changed := !equality.Semantic.DeepEqual(existingPluginConfigMap.Data, newPluginConfigMap.Data) ||
447+
!equality.Semantic.DeepEqual(existingPluginConfigMap.Labels, newPluginConfigMap.Labels)
445448
if changed {
446449
reqLogger.Info("Reconciling plugin configMap", "Namespace", existingPluginConfigMap.Namespace, "Name", existingPluginConfigMap.Name)
447450
existingPluginConfigMap.Data = newPluginConfigMap.Data
448451
existingPluginConfigMap.Labels = newPluginConfigMap.Labels
449-
return reconcile.Result{}, r.Client.Update(context.TODO(), newPluginConfigMap)
452+
return reconcile.Result{}, r.Client.Update(context.TODO(), existingPluginConfigMap)
450453
}
451454
}
452455
return reconcile.Result{}, nil

‎controllers/gitopsservice_controller.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,24 @@ func (r *ReconcileGitopsService) SetupWithManager(mgr ctrl.Manager) error {
114114
})),
115115
).Watches(&argoapp.ArgoCD{},
116116
&handler.EnqueueRequestForObject{},
117-
builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool {
118-
return obj.GetName() == "openshift-gitops" && obj.GetNamespace() == "openshift-gitops"
119-
}))).
117+
builder.WithPredicates(predicate.Funcs{
118+
CreateFunc: func(e event.CreateEvent) bool {
119+
// Only watch openshift-gitops ArgoCD
120+
return e.Object.GetName() == "openshift-gitops" && e.Object.GetNamespace() == "openshift-gitops"
121+
},
122+
UpdateFunc: func(e event.UpdateEvent) bool {
123+
// Only watch openshift-gitops ArgoCD
124+
if e.ObjectNew.GetName() != "openshift-gitops" || e.ObjectNew.GetNamespace() != "openshift-gitops" {
125+
return false
126+
}
127+
// Ignore updates to CR status in which case metadata.Generation does not change
128+
return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()
129+
},
130+
DeleteFunc: func(e event.DeleteEvent) bool {
131+
// Only watch openshift-gitops ArgoCD
132+
return e.Object.GetName() == "openshift-gitops" && e.Object.GetNamespace() == "openshift-gitops"
133+
},
134+
})).
120135
Complete(r)
121136
}
122137

0 commit comments

Comments
 (0)