diff --git a/controllers/consoleplugin.go b/controllers/consoleplugin.go index 85de4a528..9b2cc4fb7 100644 --- a/controllers/consoleplugin.go +++ b/controllers/consoleplugin.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "reflect" argocommon "github.com/argoproj-labs/argocd-operator/common" argocdutil "github.com/argoproj-labs/argocd-operator/controllers/argoutil" @@ -16,6 +15,7 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/redhat-developer/gitops-operator/common" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" resourcev1 "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -55,10 +55,12 @@ func getPluginPodSpec(crImagePullPolicy corev1.PullPolicy) corev1.PodSpec { podSpec := corev1.PodSpec{ Containers: []corev1.Container{ { - Env: util.ProxyEnvVars(), - Name: gitopsPluginName, - Image: consolePluginImage, - ImagePullPolicy: argocdutil.GetImagePullPolicy(crImagePullPolicy), + Env: util.ProxyEnvVars(), + Name: gitopsPluginName, + Image: consolePluginImage, + ImagePullPolicy: argocdutil.GetImagePullPolicy(crImagePullPolicy), + TerminationMessagePath: corev1.TerminationMessagePathDefault, + TerminationMessagePolicy: corev1.TerminationMessageReadFile, Ports: []corev1.ContainerPort{ { Name: "http", @@ -309,17 +311,18 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop } else { existingSpecTemplate := &existingPluginDeployment.Spec.Template newSpecTemplate := newPluginDeployment.Spec.Template - changed := !reflect.DeepEqual(existingPluginDeployment.Labels, newPluginDeployment.Labels) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) || - !reflect.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) || - !reflect.DeepEqual(existingSpecTemplate.Spec.Containers, newSpecTemplate.Spec.Containers) || - !reflect.DeepEqual(existingSpecTemplate.Spec.Volumes, newSpecTemplate.Spec.Volumes) || - !reflect.DeepEqual(existingSpecTemplate.Spec.RestartPolicy, newSpecTemplate.Spec.RestartPolicy) || - !reflect.DeepEqual(existingSpecTemplate.Spec.DNSPolicy, newSpecTemplate.Spec.DNSPolicy) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Template.Spec.NodeSelector, newPluginDeployment.Spec.Template.Spec.NodeSelector) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Template.Spec.Tolerations, newPluginDeployment.Spec.Template.Spec.Tolerations) || - !reflect.DeepEqual(existingSpecTemplate.Spec.SecurityContext, existingSpecTemplate.Spec.SecurityContext) || !reflect.DeepEqual(existingSpecTemplate.Spec.Containers[0].Resources, newSpecTemplate.Spec.Containers[0].Resources) + // Use semantic equality instead of reflect.DeepEqual to handle Kubernetes defaults properly + changed := !equality.Semantic.DeepEqual(existingPluginDeployment.Labels, newPluginDeployment.Labels) || + !equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) || + !equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.Containers, newSpecTemplate.Spec.Containers) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.Volumes, newSpecTemplate.Spec.Volumes) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.RestartPolicy, newSpecTemplate.Spec.RestartPolicy) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.DNSPolicy, newSpecTemplate.Spec.DNSPolicy) || + !equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Template.Spec.NodeSelector, newPluginDeployment.Spec.Template.Spec.NodeSelector) || + !equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Template.Spec.Tolerations, newPluginDeployment.Spec.Template.Spec.Tolerations) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.SecurityContext, newSpecTemplate.Spec.SecurityContext) || !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.Containers[0].Resources, newSpecTemplate.Spec.Containers[0].Resources) if changed { reqLogger.Info("Reconciling plugin deployment", "Namespace", existingPluginDeployment.Namespace, "Name", existingPluginDeployment.Name) @@ -364,10 +367,10 @@ func (r *ReconcileGitopsService) reconcileService(instance *pipelinesv1alpha1.Gi return reconcile.Result{}, err } } else { - changed := !reflect.DeepEqual(existingServiceRef.Annotations, pluginServiceRef.Annotations) || - !reflect.DeepEqual(existingServiceRef.Labels, pluginServiceRef.Labels) || - !reflect.DeepEqual(existingServiceRef.Spec.Selector, pluginServiceRef.Spec.Selector) || - !reflect.DeepEqual(existingServiceRef.Spec.Ports, pluginServiceRef.Spec.Ports) + changed := !equality.Semantic.DeepEqual(existingServiceRef.Annotations, pluginServiceRef.Annotations) || + !equality.Semantic.DeepEqual(existingServiceRef.Labels, pluginServiceRef.Labels) || + !equality.Semantic.DeepEqual(existingServiceRef.Spec.Selector, pluginServiceRef.Spec.Selector) || + !equality.Semantic.DeepEqual(existingServiceRef.Spec.Ports, pluginServiceRef.Spec.Ports) if changed { reqLogger.Info("Reconciling plugin service", "Namespace", existingServiceRef.Namespace, "Name", existingServiceRef.Name) @@ -375,7 +378,7 @@ func (r *ReconcileGitopsService) reconcileService(instance *pipelinesv1alpha1.Gi existingServiceRef.Labels = pluginServiceRef.Labels existingServiceRef.Spec.Selector = pluginServiceRef.Spec.Selector existingServiceRef.Spec.Ports = pluginServiceRef.Spec.Ports - return reconcile.Result{}, r.Client.Update(context.TODO(), pluginServiceRef) + return reconcile.Result{}, r.Client.Update(context.TODO(), existingServiceRef) } } return reconcile.Result{}, nil @@ -405,14 +408,14 @@ func (r *ReconcileGitopsService) reconcileConsolePlugin(instance *pipelinesv1alp return reconcile.Result{}, err } } else { - changed := !reflect.DeepEqual(existingPlugin.Spec.DisplayName, newConsolePlugin.Spec.DisplayName) || - !reflect.DeepEqual(existingPlugin.Spec.Backend.Service, newConsolePlugin.Spec.Backend.Service) + changed := !equality.Semantic.DeepEqual(existingPlugin.Spec.DisplayName, newConsolePlugin.Spec.DisplayName) || + !equality.Semantic.DeepEqual(existingPlugin.Spec.Backend.Service, newConsolePlugin.Spec.Backend.Service) if changed { reqLogger.Info("Reconciling Console Plugin", "Namespace", existingPlugin.Namespace, "Name", existingPlugin.Name) existingPlugin.Spec.DisplayName = newConsolePlugin.Spec.DisplayName existingPlugin.Spec.Backend.Service = newConsolePlugin.Spec.Backend.Service - return reconcile.Result{}, r.Client.Update(context.TODO(), newConsolePlugin) + return reconcile.Result{}, r.Client.Update(context.TODO(), existingPlugin) } } return reconcile.Result{}, nil @@ -440,13 +443,13 @@ func (r *ReconcileGitopsService) reconcileConfigMap(instance *pipelinesv1alpha1. return reconcile.Result{}, err } } else { - changed := !reflect.DeepEqual(existingPluginConfigMap.Data, newPluginConfigMap.Data) || - !reflect.DeepEqual(existingPluginConfigMap.Labels, newPluginConfigMap.Labels) + changed := !equality.Semantic.DeepEqual(existingPluginConfigMap.Data, newPluginConfigMap.Data) || + !equality.Semantic.DeepEqual(existingPluginConfigMap.Labels, newPluginConfigMap.Labels) if changed { reqLogger.Info("Reconciling plugin configMap", "Namespace", existingPluginConfigMap.Namespace, "Name", existingPluginConfigMap.Name) existingPluginConfigMap.Data = newPluginConfigMap.Data existingPluginConfigMap.Labels = newPluginConfigMap.Labels - return reconcile.Result{}, r.Client.Update(context.TODO(), newPluginConfigMap) + return reconcile.Result{}, r.Client.Update(context.TODO(), existingPluginConfigMap) } } return reconcile.Result{}, nil diff --git a/controllers/gitopsservice_controller.go b/controllers/gitopsservice_controller.go index 5bb7ffc6c..ecd2e0619 100644 --- a/controllers/gitopsservice_controller.go +++ b/controllers/gitopsservice_controller.go @@ -21,7 +21,6 @@ import ( "fmt" "log" "os" - "reflect" "strings" argoapp "github.com/argoproj-labs/argocd-operator/api/v1beta1" @@ -39,6 +38,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" resourcev1 "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -103,7 +103,8 @@ func (r *ReconcileGitopsService) SetupWithManager(mgr ctrl.Manager) error { Owns(&rbacv1.ClusterRole{}). Owns(&corev1.ServiceAccount{}). Owns(&corev1.ConfigMap{}). - Owns(&appsv1.Deployment{}, builder.WithPredicates(pred)). + // Removed Owns(&appsv1.Deployment{}) watch to prevent hot loop from deployment updates + // Deployments are still reconciled when GitopsService or ArgoCD changes Owns(&corev1.Service{}, builder.WithPredicates(pred)). Owns(&routev1.Route{}, builder.WithPredicates(pred)). Watches( @@ -114,9 +115,24 @@ func (r *ReconcileGitopsService) SetupWithManager(mgr ctrl.Manager) error { })), ).Watches(&argoapp.ArgoCD{}, &handler.EnqueueRequestForObject{}, - builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { - return obj.GetName() == "openshift-gitops" && obj.GetNamespace() == "openshift-gitops" - }))). + builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + // Only watch openshift-gitops ArgoCD + return e.Object.GetName() == "openshift-gitops" && e.Object.GetNamespace() == "openshift-gitops" + }, + UpdateFunc: func(e event.UpdateEvent) bool { + // Only watch openshift-gitops ArgoCD + if e.ObjectNew.GetName() != "openshift-gitops" || e.ObjectNew.GetNamespace() != "openshift-gitops" { + return false + } + // Ignore updates to CR status in which case metadata.Generation does not change + return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() + }, + DeleteFunc: func(e event.DeleteEvent) bool { + // Only watch openshift-gitops ArgoCD + return e.Object.GetName() == "openshift-gitops" && e.Object.GetNamespace() == "openshift-gitops" + }, + })). Complete(r) } @@ -502,7 +518,7 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli // if user is patching nodePlacement through GitopsService CR, then existingArgoCD NodePlacement is updated. if defaultArgoCDInstance.Spec.NodePlacement != nil { - if !reflect.DeepEqual(existingArgoCD.Spec.NodePlacement, defaultArgoCDInstance.Spec.NodePlacement) { + if !equality.Semantic.DeepEqual(existingArgoCD.Spec.NodePlacement, defaultArgoCDInstance.Spec.NodePlacement) { existingArgoCD.Spec.NodePlacement = defaultArgoCDInstance.Spec.NodePlacement changed = true } @@ -572,7 +588,7 @@ func (r *ReconcileGitopsService) reconcileBackend(gitopsserviceNamespacedName ty } else { return reconcile.Result{}, err } - } else if !reflect.DeepEqual(existingClusterRole.Rules, clusterRoleObj.Rules) { + } else if !equality.Semantic.DeepEqual(existingClusterRole.Rules, clusterRoleObj.Rules) { reqLogger.Info("Reconciling existing Cluster Role", "Name", clusterRoleObj.Name) existingClusterRole.Rules = clusterRoleObj.Rules err = r.Client.Update(context.TODO(), existingClusterRole) @@ -651,39 +667,32 @@ func (r *ReconcileGitopsService) reconcileBackend(gitopsserviceNamespacedName ty found.Spec.Template.Spec.Containers[0].Image = desiredImage changed = true } - if !reflect.DeepEqual(found.Spec.Template.Spec.Containers[0].Env, deploymentObj.Spec.Template.Spec.Containers[0].Env) { + if !equality.Semantic.DeepEqual(found.Spec.Template.Spec.Containers[0].Env, deploymentObj.Spec.Template.Spec.Containers[0].Env) { found.Spec.Template.Spec.Containers[0].Env = deploymentObj.Spec.Template.Spec.Containers[0].Env changed = true } - if !reflect.DeepEqual(found.Spec.Template.Spec.Containers[0].ImagePullPolicy, deploymentObj.Spec.Template.Spec.Containers[0].ImagePullPolicy) { - found.Spec.Template.Spec.Containers[0].ImagePullPolicy = deploymentObj.Spec.Template.Spec.Containers[0].ImagePullPolicy - changed = true - } - if !reflect.DeepEqual(found.Spec.Template.Spec.Containers[0].Resources, deploymentObj.Spec.Template.Spec.Containers[0].Resources) { - found.Spec.Template.Spec.Containers[0].Resources = deploymentObj.Spec.Template.Spec.Containers[0].Resources - changed = true - } - if !reflect.DeepEqual(found.Spec.Template.Spec.Containers[0].Args, deploymentObj.Spec.Template.Spec.Containers[0].Args) { + if !equality.Semantic.DeepEqual(found.Spec.Template.Spec.Containers[0].Args, deploymentObj.Spec.Template.Spec.Containers[0].Args) { found.Spec.Template.Spec.Containers[0].Args = deploymentObj.Spec.Template.Spec.Containers[0].Args changed = true } - if !reflect.DeepEqual(found.Spec.Template.Spec.Containers[0].Resources, deploymentObj.Spec.Template.Spec.Containers[0].Resources) { + // Use semantic equality to handle Kubernetes defaults properly + if !equality.Semantic.DeepEqual(found.Spec.Template.Spec.Containers[0].Resources, deploymentObj.Spec.Template.Spec.Containers[0].Resources) { found.Spec.Template.Spec.Containers[0].Resources = deploymentObj.Spec.Template.Spec.Containers[0].Resources changed = true } - if !reflect.DeepEqual(found.Spec.Template.Spec.Containers[0].SecurityContext, deploymentObj.Spec.Template.Spec.Containers[0].SecurityContext) { + if !equality.Semantic.DeepEqual(found.Spec.Template.Spec.Containers[0].SecurityContext, deploymentObj.Spec.Template.Spec.Containers[0].SecurityContext) { found.Spec.Template.Spec.Containers[0].SecurityContext = deploymentObj.Spec.Template.Spec.Containers[0].SecurityContext changed = true } - if !reflect.DeepEqual(found.Spec.Template.Spec.NodeSelector, deploymentObj.Spec.Template.Spec.NodeSelector) { + if !equality.Semantic.DeepEqual(found.Spec.Template.Spec.NodeSelector, deploymentObj.Spec.Template.Spec.NodeSelector) { found.Spec.Template.Spec.NodeSelector = deploymentObj.Spec.Template.Spec.NodeSelector changed = true } - if !reflect.DeepEqual(found.Spec.Template.Spec.Tolerations, deploymentObj.Spec.Template.Spec.Tolerations) { + if !equality.Semantic.DeepEqual(found.Spec.Template.Spec.Tolerations, deploymentObj.Spec.Template.Spec.Tolerations) { found.Spec.Template.Spec.Tolerations = deploymentObj.Spec.Template.Spec.Tolerations changed = true } - if !reflect.DeepEqual(found.Spec.Template.Spec.SecurityContext, deploymentObj.Spec.Template.Spec.SecurityContext) { + if !equality.Semantic.DeepEqual(found.Spec.Template.Spec.SecurityContext, deploymentObj.Spec.Template.Spec.SecurityContext) { found.Spec.Template.Spec.SecurityContext = deploymentObj.Spec.Template.Spec.SecurityContext changed = true }