diff --git a/pkg/operator/controller/canary/route.go b/pkg/operator/controller/canary/route.go index f5c698f950..806e81458a 100644 --- a/pkg/operator/controller/canary/route.go +++ b/pkg/operator/controller/canary/route.go @@ -75,10 +75,12 @@ func (r *reconciler) updateCanaryRoute(current, desired *routev1.Route) (bool, e return false, nil } + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { return false, fmt.Errorf("failed to update canary route %s/%s: %v", updated.Namespace, updated.Name, err) } - log.Info("updated canary route", "namespace", updated.Namespace, "name", updated.Name) + log.Info("updated canary route", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil } diff --git a/pkg/operator/controller/ingress/cluster-role.go b/pkg/operator/controller/ingress/cluster-role.go index a25f4e917c..888cef489e 100644 --- a/pkg/operator/controller/ingress/cluster-role.go +++ b/pkg/operator/controller/ingress/cluster-role.go @@ -5,6 +5,9 @@ import ( "fmt" "reflect" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -45,7 +48,6 @@ func (r *reconciler) ensureClusterRole() (bool, *rbacv1.ClusterRole, error) { if updated, err := r.updateClusterRole(current, desired); err != nil { return true, current, fmt.Errorf("failed to update cluster role: %v", err) } else if updated { - log.Info("updated cluster role", "desired", desired) return r.currentClusterRole() } } @@ -77,11 +79,14 @@ func (r *reconciler) updateClusterRole(current, desired *rbacv1.ClusterRole) (bo } updated := current.DeepCopy() updated.Rules = desired.Rules + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { if errors.IsAlreadyExists(err) { return false, nil } return false, err } + log.Info("updated cluster role", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil } diff --git a/pkg/operator/controller/ingress/dns.go b/pkg/operator/controller/ingress/dns.go index 62a0ce3b43..78e72691f6 100644 --- a/pkg/operator/controller/ingress/dns.go +++ b/pkg/operator/controller/ingress/dns.go @@ -51,7 +51,6 @@ func (r *reconciler) ensureWildcardDNSRecord(ic *operatorv1.IngressController, s if updated, err := r.updateDNSRecord(current, desired); err != nil { return true, current, fmt.Errorf("failed to update dnsrecord %s/%s: %v", desired.Namespace, desired.Name, err) } else if updated { - log.Info("updated dnsrecord", "dnsrecord", desired) return r.currentWildcardDNSRecord(ic) } } @@ -170,9 +169,12 @@ func (r *reconciler) updateDNSRecord(current, desired *iov1.DNSRecord) (bool, er return false, nil } + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { return false, err } + log.Info("updated dnsrecord", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil } diff --git a/pkg/operator/controller/ingress/load_balancer_service.go b/pkg/operator/controller/ingress/load_balancer_service.go index cc718b6930..0326af9e1e 100644 --- a/pkg/operator/controller/ingress/load_balancer_service.go +++ b/pkg/operator/controller/ingress/load_balancer_service.go @@ -4,6 +4,9 @@ import ( "context" "fmt" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/cluster-ingress-operator/pkg/manifests" "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" @@ -160,7 +163,6 @@ func (r *reconciler) ensureLoadBalancerService(ci *operatorv1.IngressController, if updated, err := r.updateLoadBalancerService(currentLBService, desiredLBService, platform); err != nil { return true, currentLBService, fmt.Errorf("failed to update load balancer service: %v", err) } else if updated { - log.Info("updated load balancer service", "namespace", desiredLBService.Namespace, "name", desiredLBService.Name) return r.currentLoadBalancerService(ci) } } @@ -299,9 +301,12 @@ func (r *reconciler) updateLoadBalancerService(current, desired *corev1.Service, if !changed { return false, nil } + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { return false, err } + log.Info("updated load balancer service", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil } diff --git a/pkg/operator/controller/ingress/monitoring.go b/pkg/operator/controller/ingress/monitoring.go index 18d5b0e6e4..93175d535b 100644 --- a/pkg/operator/controller/ingress/monitoring.go +++ b/pkg/operator/controller/ingress/monitoring.go @@ -5,6 +5,9 @@ import ( "fmt" "reflect" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" operatorv1 "github.com/openshift/api/operator/v1" @@ -41,7 +44,6 @@ func (r *reconciler) ensureServiceMonitor(ic *operatorv1.IngressController, svc if updated, err := r.updateServiceMonitor(current, desired); err != nil { return true, current, fmt.Errorf("failed to update servicemonitor %s/%s: %v", desired.GetNamespace(), desired.GetName(), err) } else if updated { - log.Info("updated servicemonitor", "namespace", desired.GetNamespace(), "name", desired.GetName()) return r.currentServiceMonitor(ic) } } @@ -131,9 +133,12 @@ func (r *reconciler) updateServiceMonitor(current, desired *unstructured.Unstruc return false, nil } + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { return false, err } + log.Info("updated servicemonitor", "namespace", updated.GetNamespace(), "name", updated.GetName(), "diff", diff) return true, nil } diff --git a/pkg/operator/controller/ingress/nodeport_service.go b/pkg/operator/controller/ingress/nodeport_service.go index 13413d8a94..f85b4fca5b 100644 --- a/pkg/operator/controller/ingress/nodeport_service.go +++ b/pkg/operator/controller/ingress/nodeport_service.go @@ -53,7 +53,6 @@ func (r *reconciler) ensureNodePortService(ic *operatorv1.IngressController, dep if updated, err := r.updateNodePortService(current, desired); err != nil { return true, current, fmt.Errorf("failed to update NodePort service: %v", err) } else if updated { - log.Info("updated NodePort service", "service", desired) return r.currentNodePortService(ic) } } @@ -126,9 +125,12 @@ func (r *reconciler) updateNodePortService(current, desired *corev1.Service) (bo return false, nil } + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { return false, err } + log.Info("updated NodePort service", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil } @@ -139,7 +141,7 @@ func nodePortServiceChanged(current, expected *corev1.Service) (bool, *corev1.Se // Ignore fields that the API, other controllers, or user may // have modified. cmpopts.IgnoreFields(corev1.ServicePort{}, "NodePort"), - cmpopts.IgnoreFields(corev1.ServiceSpec{}, "ClusterIP", "ExternalIPs", "HealthCheckNodePort"), + cmpopts.IgnoreFields(corev1.ServiceSpec{}, "ClusterIP", "ClusterIPs", "ExternalIPs", "HealthCheckNodePort"), cmp.Comparer(cmpServiceAffinity), cmpopts.EquateEmpty(), } diff --git a/pkg/operator/controller/ingress/nodeport_service_test.go b/pkg/operator/controller/ingress/nodeport_service_test.go index 6448d8c881..ebdea019d8 100644 --- a/pkg/operator/controller/ingress/nodeport_service_test.go +++ b/pkg/operator/controller/ingress/nodeport_service_test.go @@ -75,6 +75,7 @@ func TestNodePortServiceChanged(t *testing.T) { description: "if .spec.clusterIP changes", mutate: func(svc *corev1.Service) { svc.Spec.ClusterIP = "2.3.4.5" + svc.Spec.ClusterIPs = []string{"2.3.4.5"} }, expect: false, }, diff --git a/pkg/operator/controller/ingress/poddisruptionbudget.go b/pkg/operator/controller/ingress/poddisruptionbudget.go index 975662b2cc..f75b4d9ebe 100644 --- a/pkg/operator/controller/ingress/poddisruptionbudget.go +++ b/pkg/operator/controller/ingress/poddisruptionbudget.go @@ -53,7 +53,6 @@ func (r *reconciler) ensureRouterPodDisruptionBudget(ic *operatorv1.IngressContr if updated, err := r.updateRouterPodDisruptionBudget(current, desired); err != nil { return true, current, fmt.Errorf("failed to update pod disruption budget: %v", err) } else if updated { - log.Info("updated pod disruption budget", "poddisruptionbudget", desired) return r.currentRouterPodDisruptionBudget(ic) } } @@ -115,9 +114,12 @@ func (r *reconciler) updateRouterPodDisruptionBudget(current, desired *policyv1b return false, nil } + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { return false, err } + log.Info("updated pod disruption budget", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil } diff --git a/pkg/operator/controller/ingress/rsyslog_configmap.go b/pkg/operator/controller/ingress/rsyslog_configmap.go index 41f0eddf61..b1f691f9b6 100644 --- a/pkg/operator/controller/ingress/rsyslog_configmap.go +++ b/pkg/operator/controller/ingress/rsyslog_configmap.go @@ -5,6 +5,9 @@ import ( "fmt" "reflect" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" @@ -60,7 +63,6 @@ func (r *reconciler) ensureRsyslogConfigMap(ic *operatorv1.IngressController, de if updated, err := r.updateRsyslogConfigMap(current, desired); err != nil { return true, current, fmt.Errorf("failed to update configmap: %v", err) } else if updated { - log.Info("updated configmap", "configmap", desired) return r.currentRsyslogConfigMap(ic) } } @@ -114,12 +116,15 @@ func (r *reconciler) updateRsyslogConfigMap(current, desired *corev1.ConfigMap) } updated := current.DeepCopy() updated.Data = desired.Data + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { if errors.IsAlreadyExists(err) { return false, nil } return false, err } + log.Info("updated configmap", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil } diff --git a/pkg/operator/controller/ingress/serviceca_configmap.go b/pkg/operator/controller/ingress/serviceca_configmap.go index dd451a1fa6..dc537825a1 100644 --- a/pkg/operator/controller/ingress/serviceca_configmap.go +++ b/pkg/operator/controller/ingress/serviceca_configmap.go @@ -4,6 +4,9 @@ import ( "context" "fmt" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" corev1 "k8s.io/api/core/v1" @@ -47,7 +50,6 @@ func (r *reconciler) ensureServiceCAConfigMap() (bool, *corev1.ConfigMap, error) if updated, err := r.updateServiceCAConfigMap(current, desired); err != nil { return true, current, fmt.Errorf("failed to update configmap: %v", err) } else if updated { - log.Info("updated configmap", "configmap", desired) return r.currentServiceCAConfigMap() } } @@ -101,11 +103,14 @@ func (r *reconciler) updateServiceCAConfigMap(current, desired *corev1.ConfigMap updated := current.DeepCopy() updated.Annotations["service.beta.openshift.io/inject-cabundle"] = "true" + // Diff before updating because the client may mutate the object. + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { if errors.IsAlreadyExists(err) { return false, nil } return false, err } + log.Info("updated configmap", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) return true, nil }