diff --git a/lib/resourceapply/apiext.go b/lib/resourceapply/apiext.go index 1649c488e..93e0068df 100644 --- a/lib/resourceapply/apiext.go +++ b/lib/resourceapply/apiext.go @@ -3,17 +3,17 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" "github.com/openshift/cluster-version-operator/lib/resourcemerge" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/klog/v2" "k8s.io/utils/pointer" ) -func ApplyCustomResourceDefinitionv1(ctx context.Context, client apiextclientv1.CustomResourceDefinitionsGetter, required *apiextv1.CustomResourceDefinition) (*apiextv1.CustomResourceDefinition, bool, error) { +func ApplyCustomResourceDefinitionv1(ctx context.Context, client apiextclientv1.CustomResourceDefinitionsGetter, required *apiextv1.CustomResourceDefinition, reconciling bool) (*apiextv1.CustomResourceDefinition, bool, error) { existing, err := client.CustomResourceDefinitions().Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("CRD %s not found, creating", required.Name) @@ -34,7 +34,9 @@ func ApplyCustomResourceDefinitionv1(ctx context.Context, client apiextclientv1. return existing, false, nil } - klog.V(2).Infof("Updating CRD %s due to diff: %v", required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating CRD %s due to diff: %v", required.Name, cmp.Diff(existing, required)) + } actual, err := client.CustomResourceDefinitions().Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err diff --git a/lib/resourceapply/apps.go b/lib/resourceapply/apps.go index b337f463b..a4ec9dc45 100644 --- a/lib/resourceapply/apps.go +++ b/lib/resourceapply/apps.go @@ -3,19 +3,18 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" "github.com/openshift/cluster-version-operator/lib/resourcemerge" appsv1 "k8s.io/api/apps/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/diff" appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1" - appslisterv1 "k8s.io/client-go/listers/apps/v1" "k8s.io/klog/v2" "k8s.io/utils/pointer" ) // ApplyDeploymentv1 applies the required deployment to the cluster. -func ApplyDeploymentv1(ctx context.Context, client appsclientv1.DeploymentsGetter, required *appsv1.Deployment) (*appsv1.Deployment, bool, error) { +func ApplyDeploymentv1(ctx context.Context, client appsclientv1.DeploymentsGetter, required *appsv1.Deployment, reconciling bool) (*appsv1.Deployment, bool, error) { existing, err := client.Deployments(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("Deployment %s/%s not found, creating", required.Namespace, required.Name) @@ -35,42 +34,16 @@ func ApplyDeploymentv1(ctx context.Context, client appsclientv1.DeploymentsGette if !*modified { return existing, false, nil } - klog.V(2).Infof("Updating Deployment %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) - - actual, err := client.Deployments(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) - return actual, true, err -} - -// ApplyDeploymentFromCache applies the required deployment to the cluster. -func ApplyDeploymentFromCache(ctx context.Context, lister appslisterv1.DeploymentLister, client appsclientv1.DeploymentsGetter, required *appsv1.Deployment) (*appsv1.Deployment, bool, error) { - existing, err := lister.Deployments(required.Namespace).Get(required.Name) - if apierrors.IsNotFound(err) { - klog.V(2).Infof("Deployment %s/%s not found, creating", required.Namespace, required.Name) - actual, err := client.Deployments(required.Namespace).Create(ctx, required, metav1.CreateOptions{}) - return actual, true, err - } - if err != nil { - return nil, false, err - } - // if we only create this resource, we have no need to continue further - if IsCreateOnly(required) { - return nil, false, nil + if reconciling { + klog.V(4).Infof("Updating Deployment %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) } - existing = existing.DeepCopy() - modified := pointer.BoolPtr(false) - resourcemerge.EnsureDeployment(modified, existing, *required) - if !*modified { - return existing, false, nil - } - klog.V(2).Infof("Updating Deployment %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) - actual, err := client.Deployments(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err } // ApplyDaemonSetv1 applies the required daemonset to the cluster. -func ApplyDaemonSetv1(ctx context.Context, client appsclientv1.DaemonSetsGetter, required *appsv1.DaemonSet) (*appsv1.DaemonSet, bool, error) { +func ApplyDaemonSetv1(ctx context.Context, client appsclientv1.DaemonSetsGetter, required *appsv1.DaemonSet, reconciling bool) (*appsv1.DaemonSet, bool, error) { existing, err := client.DaemonSets(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("DaemonSet %s/%s not found, creating", required.Namespace, required.Name) @@ -91,36 +64,9 @@ func ApplyDaemonSetv1(ctx context.Context, client appsclientv1.DaemonSetsGetter, return existing, false, nil } - klog.V(2).Infof("Updating DaemonSet %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) - - actual, err := client.DaemonSets(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) - return actual, true, err -} - -// ApplyDaemonSetFromCache applies the required deployment to the cluster. -func ApplyDaemonSetFromCache(ctx context.Context, lister appslisterv1.DaemonSetLister, client appsclientv1.DaemonSetsGetter, required *appsv1.DaemonSet) (*appsv1.DaemonSet, bool, error) { - existing, err := lister.DaemonSets(required.Namespace).Get(required.Name) - if apierrors.IsNotFound(err) { - klog.V(2).Infof("DaemonSet %s/%s not found, creating", required.Namespace, required.Name) - actual, err := client.DaemonSets(required.Namespace).Create(ctx, required, metav1.CreateOptions{}) - return actual, true, err - } - if err != nil { - return nil, false, err + if reconciling { + klog.V(4).Infof("Updating DaemonSet %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) } - // if we only create this resource, we have no need to continue further - if IsCreateOnly(required) { - return nil, false, nil - } - - existing = existing.DeepCopy() - modified := pointer.BoolPtr(false) - resourcemerge.EnsureDaemonSet(modified, existing, *required) - if !*modified { - return existing, false, nil - } - - klog.V(2).Infof("Updating DaemonSet %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) actual, err := client.DaemonSets(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err diff --git a/lib/resourceapply/batch.go b/lib/resourceapply/batch.go index 964e34968..16d378eec 100644 --- a/lib/resourceapply/batch.go +++ b/lib/resourceapply/batch.go @@ -3,18 +3,18 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" "github.com/openshift/cluster-version-operator/lib/resourcemerge" batchv1 "k8s.io/api/batch/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/diff" batchclientv1 "k8s.io/client-go/kubernetes/typed/batch/v1" "k8s.io/klog/v2" "k8s.io/utils/pointer" ) // ApplyJobv1 applies the required Job to the cluster. -func ApplyJobv1(ctx context.Context, client batchclientv1.JobsGetter, required *batchv1.Job) (*batchv1.Job, bool, error) { +func ApplyJobv1(ctx context.Context, client batchclientv1.JobsGetter, required *batchv1.Job, reconciling bool) (*batchv1.Job, bool, error) { existing, err := client.Jobs(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("Job %s/%s not found, creating", required.Namespace, required.Name) @@ -35,7 +35,9 @@ func ApplyJobv1(ctx context.Context, client batchclientv1.JobsGetter, required * return existing, false, nil } - klog.V(2).Infof("Updating Job %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating Job %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) + } actual, err := client.Jobs(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err diff --git a/lib/resourceapply/core.go b/lib/resourceapply/core.go index dc772e8b0..2ed75c31b 100644 --- a/lib/resourceapply/core.go +++ b/lib/resourceapply/core.go @@ -3,13 +3,13 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" "k8s.io/klog/v2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/diff" coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/utils/pointer" @@ -17,7 +17,7 @@ import ( ) // ApplyNamespacev1 merges objectmeta, does not worry about anything else -func ApplyNamespacev1(ctx context.Context, client coreclientv1.NamespacesGetter, required *corev1.Namespace) (*corev1.Namespace, bool, error) { +func ApplyNamespacev1(ctx context.Context, client coreclientv1.NamespacesGetter, required *corev1.Namespace, reconciling bool) (*corev1.Namespace, bool, error) { existing, err := client.Namespaces().Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("Namespace %s not found, creating", required.Name) @@ -37,7 +37,9 @@ func ApplyNamespacev1(ctx context.Context, client coreclientv1.NamespacesGetter, if !*modified { return existing, false, nil } - klog.V(2).Infof("Updating Namespace %s due to diff: %v", required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating Namespace %s due to diff: %v", required.Name, cmp.Diff(existing, required)) + } actual, err := client.Namespaces().Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err @@ -46,7 +48,7 @@ func ApplyNamespacev1(ctx context.Context, client coreclientv1.NamespacesGetter, // ApplyServicev1 merges objectmeta and requires // TODO, since this cannot determine whether changes are due to legitimate actors (api server) or illegitimate ones (users), we cannot update // TODO I've special cased the selector for now -func ApplyServicev1(ctx context.Context, client coreclientv1.ServicesGetter, required *corev1.Service) (*corev1.Service, bool, error) { +func ApplyServicev1(ctx context.Context, client coreclientv1.ServicesGetter, required *corev1.Service, reconciling bool) (*corev1.Service, bool, error) { existing, err := client.Services(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("Service %s/%s not found, creating", required.Namespace, required.Name) @@ -72,14 +74,16 @@ func ApplyServicev1(ctx context.Context, client coreclientv1.ServicesGetter, req } existing.Spec.Selector = required.Spec.Selector - klog.V(2).Infof("Updating Service %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating Service %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) + } actual, err := client.Services(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err } // ApplyServiceAccountv1 applies the required serviceaccount to the cluster. -func ApplyServiceAccountv1(ctx context.Context, client coreclientv1.ServiceAccountsGetter, required *corev1.ServiceAccount) (*corev1.ServiceAccount, bool, error) { +func ApplyServiceAccountv1(ctx context.Context, client coreclientv1.ServiceAccountsGetter, required *corev1.ServiceAccount, reconciling bool) (*corev1.ServiceAccount, bool, error) { existing, err := client.ServiceAccounts(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("ServiceAccount %s/%s not found, creating", required.Namespace, required.Name) @@ -100,14 +104,16 @@ func ApplyServiceAccountv1(ctx context.Context, client coreclientv1.ServiceAccou return existing, false, nil } - klog.V(2).Infof("Updating ServiceAccount %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating ServiceAccount %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) + } actual, err := client.ServiceAccounts(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err } // ApplyConfigMapv1 applies the required serviceaccount to the cluster. -func ApplyConfigMapv1(ctx context.Context, client coreclientv1.ConfigMapsGetter, required *corev1.ConfigMap) (*corev1.ConfigMap, bool, error) { +func ApplyConfigMapv1(ctx context.Context, client coreclientv1.ConfigMapsGetter, required *corev1.ConfigMap, reconciling bool) (*corev1.ConfigMap, bool, error) { existing, err := client.ConfigMaps(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("ConfigMap %s/%s not found, creating", required.Namespace, required.Name) @@ -128,7 +134,9 @@ func ApplyConfigMapv1(ctx context.Context, client coreclientv1.ConfigMapsGetter, return existing, false, nil } - klog.V(2).Infof("Updating ConfigMap %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating ConfigMap %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) + } actual, err := client.ConfigMaps(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err diff --git a/lib/resourceapply/cv.go b/lib/resourceapply/cv.go index 357d67e3f..8f4edb6af 100644 --- a/lib/resourceapply/cv.go +++ b/lib/resourceapply/cv.go @@ -3,43 +3,17 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" configv1 "github.com/openshift/api/config/v1" configclientv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" "github.com/openshift/cluster-version-operator/lib/resourcemerge" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/klog/v2" "k8s.io/utils/pointer" ) -func ApplyClusterVersionv1(ctx context.Context, client configclientv1.ClusterVersionsGetter, required *configv1.ClusterVersion) (*configv1.ClusterVersion, bool, error) { - existing, err := client.ClusterVersions().Get(ctx, required.Name, metav1.GetOptions{}) - if errors.IsNotFound(err) { - actual, err := client.ClusterVersions().Create(ctx, required, metav1.CreateOptions{}) - return actual, true, err - } - if err != nil { - return nil, false, err - } - // if we only create this resource, we have no need to continue further - if IsCreateOnly(required) { - return nil, false, nil - } - - modified := pointer.BoolPtr(false) - resourcemerge.EnsureClusterVersion(modified, existing, *required) - if !*modified { - return existing, false, nil - } - - klog.V(2).Infof("Updating ClusterVersion %s due to diff: %v", required.Name, diff.ObjectDiff(existing, required)) - - actual, err := client.ClusterVersions().Update(ctx, existing, metav1.UpdateOptions{}) - return actual, true, err -} - func ApplyClusterVersionFromCache(ctx context.Context, lister configlistersv1.ClusterVersionLister, client configclientv1.ClusterVersionsGetter, required *configv1.ClusterVersion) (*configv1.ClusterVersion, bool, error) { obj, err := lister.Get(required.Name) if errors.IsNotFound(err) { @@ -62,7 +36,7 @@ func ApplyClusterVersionFromCache(ctx context.Context, lister configlistersv1.Cl return existing, false, nil } - klog.V(2).Infof("Updating ClusterVersion %s due to diff: %v", required.Name, diff.ObjectDiff(existing, required)) + klog.V(4).Infof("Updating ClusterVersion %s due to diff: %v", required.Name, cmp.Diff(existing, required)) actual, err := client.ClusterVersions().Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err diff --git a/lib/resourceapply/imagestream.go b/lib/resourceapply/imagestream.go index 732982ece..9a5113c07 100644 --- a/lib/resourceapply/imagestream.go +++ b/lib/resourceapply/imagestream.go @@ -3,15 +3,17 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" imagev1 "github.com/openshift/api/image/v1" imageclientv1 "github.com/openshift/client-go/image/clientset/versioned/typed/image/v1" "github.com/openshift/cluster-version-operator/lib/resourcemerge" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" "k8s.io/utils/pointer" ) -func ApplyImageStreamv1(ctx context.Context, client imageclientv1.ImageStreamsGetter, required *imagev1.ImageStream) (*imagev1.ImageStream, bool, error) { +func ApplyImageStreamv1(ctx context.Context, client imageclientv1.ImageStreamsGetter, required *imagev1.ImageStream, reconciling bool) (*imagev1.ImageStream, bool, error) { existing, err := client.ImageStreams(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if errors.IsNotFound(err) { actual, err := client.ImageStreams(required.Namespace).Create(ctx, required, metav1.CreateOptions{}) @@ -31,6 +33,9 @@ func ApplyImageStreamv1(ctx context.Context, client imageclientv1.ImageStreamsGe return existing, false, nil } + if reconciling { + klog.V(4).Infof("Updating Namespace %s due to diff: %v", required.Name, cmp.Diff(existing, required)) + } actual, err := client.ImageStreams(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err } diff --git a/lib/resourceapply/rbac.go b/lib/resourceapply/rbac.go index 7b5c3bcd8..4f7fd184f 100644 --- a/lib/resourceapply/rbac.go +++ b/lib/resourceapply/rbac.go @@ -3,18 +3,18 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" "github.com/openshift/cluster-version-operator/lib/resourcemerge" rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/diff" rbacclientv1 "k8s.io/client-go/kubernetes/typed/rbac/v1" "k8s.io/klog/v2" "k8s.io/utils/pointer" ) // ApplyClusterRoleBindingv1 applies the required clusterrolebinding to the cluster. -func ApplyClusterRoleBindingv1(ctx context.Context, client rbacclientv1.ClusterRoleBindingsGetter, required *rbacv1.ClusterRoleBinding) (*rbacv1.ClusterRoleBinding, bool, error) { +func ApplyClusterRoleBindingv1(ctx context.Context, client rbacclientv1.ClusterRoleBindingsGetter, required *rbacv1.ClusterRoleBinding, reconciling bool) (*rbacv1.ClusterRoleBinding, bool, error) { existing, err := client.ClusterRoleBindings().Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("ClusterRoleBinding %s not found, creating", required.Name) @@ -34,14 +34,16 @@ func ApplyClusterRoleBindingv1(ctx context.Context, client rbacclientv1.ClusterR if !*modified { return existing, false, nil } - klog.V(2).Infof("Updating ClusterRoleBinding %s due to diff: %v", required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating ClusterRoleBinding %s due to diff: %v", required.Name, cmp.Diff(existing, required)) + } actual, err := client.ClusterRoleBindings().Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err } // ApplyClusterRolev1 applies the required clusterrole to the cluster. -func ApplyClusterRolev1(ctx context.Context, client rbacclientv1.ClusterRolesGetter, required *rbacv1.ClusterRole) (*rbacv1.ClusterRole, bool, error) { +func ApplyClusterRolev1(ctx context.Context, client rbacclientv1.ClusterRolesGetter, required *rbacv1.ClusterRole, reconciling bool) (*rbacv1.ClusterRole, bool, error) { existing, err := client.ClusterRoles().Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("ClusterRole %s not found, creating", required.Name) @@ -61,14 +63,16 @@ func ApplyClusterRolev1(ctx context.Context, client rbacclientv1.ClusterRolesGet if !*modified { return existing, false, nil } - klog.V(2).Infof("Updating ClusterRole %s due to diff: %v", required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating ClusterRole %s due to diff: %v", required.Name, cmp.Diff(existing, required)) + } actual, err := client.ClusterRoles().Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err } // ApplyRoleBindingv1 applies the required clusterrolebinding to the cluster. -func ApplyRoleBindingv1(ctx context.Context, client rbacclientv1.RoleBindingsGetter, required *rbacv1.RoleBinding) (*rbacv1.RoleBinding, bool, error) { +func ApplyRoleBindingv1(ctx context.Context, client rbacclientv1.RoleBindingsGetter, required *rbacv1.RoleBinding, reconciling bool) (*rbacv1.RoleBinding, bool, error) { existing, err := client.RoleBindings(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("RoleBinding %s/%s not found, creating", required.Namespace, required.Name) @@ -88,14 +92,16 @@ func ApplyRoleBindingv1(ctx context.Context, client rbacclientv1.RoleBindingsGet if !*modified { return existing, false, nil } - klog.V(2).Infof("Updating RoleBinding %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating RoleBinding %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) + } actual, err := client.RoleBindings(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err } // ApplyRolev1 applies the required clusterrole to the cluster. -func ApplyRolev1(ctx context.Context, client rbacclientv1.RolesGetter, required *rbacv1.Role) (*rbacv1.Role, bool, error) { +func ApplyRolev1(ctx context.Context, client rbacclientv1.RolesGetter, required *rbacv1.Role, reconciling bool) (*rbacv1.Role, bool, error) { existing, err := client.Roles(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("Role %s/%s not found, creating", required.Namespace, required.Name) @@ -115,7 +121,9 @@ func ApplyRolev1(ctx context.Context, client rbacclientv1.RolesGetter, required if !*modified { return existing, false, nil } - klog.V(2).Infof("Updating Role %s/%s due to diff: %v", required.Namespace, required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating Role %s/%s due to diff: %v", required.Namespace, required.Name, cmp.Diff(existing, required)) + } actual, err := client.Roles(required.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err diff --git a/lib/resourceapply/security.go b/lib/resourceapply/security.go index 1ab9c44ac..c4b2b54cc 100644 --- a/lib/resourceapply/security.go +++ b/lib/resourceapply/security.go @@ -3,18 +3,18 @@ package resourceapply import ( "context" + "github.com/google/go-cmp/cmp" securityv1 "github.com/openshift/api/security/v1" securityclientv1 "github.com/openshift/client-go/security/clientset/versioned/typed/security/v1" "github.com/openshift/cluster-version-operator/lib/resourcemerge" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/klog/v2" "k8s.io/utils/pointer" ) // ApplySecurityContextConstraintsv1 applies the required SecurityContextConstraints to the cluster. -func ApplySecurityContextConstraintsv1(ctx context.Context, client securityclientv1.SecurityContextConstraintsGetter, required *securityv1.SecurityContextConstraints) (*securityv1.SecurityContextConstraints, bool, error) { +func ApplySecurityContextConstraintsv1(ctx context.Context, client securityclientv1.SecurityContextConstraintsGetter, required *securityv1.SecurityContextConstraints, reconciling bool) (*securityv1.SecurityContextConstraints, bool, error) { existing, err := client.SecurityContextConstraints().Get(ctx, required.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { klog.V(2).Infof("SCC %s not found, creating", required.Name) @@ -35,7 +35,9 @@ func ApplySecurityContextConstraintsv1(ctx context.Context, client securityclien return existing, false, nil } - klog.V(2).Infof("Updating SCC %s due to diff: %v", required.Name, diff.ObjectDiff(existing, required)) + if reconciling { + klog.V(4).Infof("Updating SCC %s due to diff: %v", required.Name, cmp.Diff(existing, required)) + } actual, err := client.SecurityContextConstraints().Update(ctx, existing, metav1.UpdateOptions{}) return actual, true, err diff --git a/lib/resourcebuilder/podspec_test.go b/lib/resourcebuilder/podspec_test.go index ea462ebaf..a3ce778ef 100644 --- a/lib/resourcebuilder/podspec_test.go +++ b/lib/resourcebuilder/podspec_test.go @@ -4,8 +4,7 @@ import ( "reflect" "testing" - "k8s.io/apimachinery/pkg/util/diff" - + "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" ) @@ -105,7 +104,7 @@ func TestUpdatePodSpecWithProxy(t *testing.T) { } if !reflect.DeepEqual(test.input, test.expected) { - t.Error(diff.ObjectDiff(test.input, test.expected)) + t.Error(cmp.Diff(test.input, test.expected)) } }) } @@ -264,7 +263,7 @@ func TestUpdatePodSpecWithInternalLoadBalancerKubeService(t *testing.T) { } if !reflect.DeepEqual(test.input, test.expected) { - t.Error(diff.ObjectDiff(test.input, test.expected)) + t.Error(cmp.Diff(test.input, test.expected)) } }) } diff --git a/lib/resourcebuilder/resourcebuilder.go b/lib/resourcebuilder/resourcebuilder.go index 29aad4906..f95e00e9a 100644 --- a/lib/resourcebuilder/resourcebuilder.go +++ b/lib/resourcebuilder/resourcebuilder.go @@ -75,7 +75,8 @@ func (b *builder) WithModifier(f MetaV1ObjectModifierFunc) Interface { func (b *builder) Do(ctx context.Context) error { obj := resourceread.ReadOrDie(b.raw) - updatingMode := (b.mode == UpdatingMode) + updatingMode := b.mode == UpdatingMode + reconcilingMode := b.mode == ReconcilingMode switch typedObject := obj.(type) { case *imagev1.ImageStream: @@ -86,7 +87,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyImageStreamv1(ctx, b.imageClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyImageStreamv1(ctx, b.imageClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -98,7 +99,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplySecurityContextConstraintsv1(ctx, b.securityClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplySecurityContextConstraintsv1(ctx, b.securityClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -113,7 +114,7 @@ func (b *builder) Do(ctx context.Context) error { if err := b.modifyDaemonSet(ctx, typedObject); err != nil { return err } - if _, _, err := resourceapply.ApplyDaemonSetv1(ctx, b.appsClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyDaemonSetv1(ctx, b.appsClientv1, typedObject, reconcilingMode); err != nil { return err } return b.checkDaemonSetHealth(ctx, typedObject) @@ -129,7 +130,7 @@ func (b *builder) Do(ctx context.Context) error { if err := b.modifyDeployment(ctx, typedObject); err != nil { return err } - if _, _, err := resourceapply.ApplyDeploymentv1(ctx, b.appsClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyDeploymentv1(ctx, b.appsClientv1, typedObject, reconcilingMode); err != nil { return err } return b.checkDeploymentHealth(ctx, typedObject) @@ -142,7 +143,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyJobv1(ctx, b.batchClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyJobv1(ctx, b.batchClientv1, typedObject, reconcilingMode); err != nil { return err } return b.checkJobHealth(ctx, typedObject) @@ -155,7 +156,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyConfigMapv1(ctx, b.coreClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyConfigMapv1(ctx, b.coreClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -167,7 +168,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyNamespacev1(ctx, b.coreClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyNamespacev1(ctx, b.coreClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -179,7 +180,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyServicev1(ctx, b.coreClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyServicev1(ctx, b.coreClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -191,7 +192,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyServiceAccountv1(ctx, b.coreClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyServiceAccountv1(ctx, b.coreClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -203,7 +204,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyClusterRolev1(ctx, b.rbacClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyClusterRolev1(ctx, b.rbacClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -215,7 +216,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyClusterRoleBindingv1(ctx, b.rbacClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyClusterRoleBindingv1(ctx, b.rbacClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -227,7 +228,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyRolev1(ctx, b.rbacClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyRolev1(ctx, b.rbacClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -239,7 +240,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyRoleBindingv1(ctx, b.rbacClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyRoleBindingv1(ctx, b.rbacClientv1, typedObject, reconcilingMode); err != nil { return err } } @@ -251,7 +252,7 @@ func (b *builder) Do(ctx context.Context) error { updatingMode); err != nil { return err } else if !deleteReq { - if _, _, err := resourceapply.ApplyCustomResourceDefinitionv1(ctx, b.apiextensionsClientv1, typedObject); err != nil { + if _, _, err := resourceapply.ApplyCustomResourceDefinitionv1(ctx, b.apiextensionsClientv1, typedObject, reconcilingMode); err != nil { return err } } diff --git a/pkg/cvo/internal/generic.go b/pkg/cvo/internal/generic.go index 40d145ab1..111f888f5 100644 --- a/pkg/cvo/internal/generic.go +++ b/pkg/cvo/internal/generic.go @@ -4,12 +4,12 @@ import ( "context" "fmt" + "github.com/google/go-cmp/cmp" "github.com/openshift/cluster-version-operator/lib/resourceapply" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/dynamic" "k8s.io/klog/v2" @@ -55,7 +55,7 @@ func deleteUnstructured(ctx context.Context, client dynamic.ResourceInterface, r return true, nil } -func applyUnstructured(ctx context.Context, client dynamic.ResourceInterface, required *unstructured.Unstructured) (*unstructured.Unstructured, bool, error) { +func applyUnstructured(ctx context.Context, client dynamic.ResourceInterface, required *unstructured.Unstructured, reconciling bool) (*unstructured.Unstructured, bool, error) { if required.GetName() == "" { return nil, false, fmt.Errorf("invalid object: name cannot be empty") } @@ -84,7 +84,7 @@ func applyUnstructured(ctx context.Context, client dynamic.ResourceInterface, re } } - objDiff := diff.ObjectDiff(expected, existing) + objDiff := cmp.Diff(expected, existing) if objDiff == "" { // Skip update, as no changes found return existing, false, nil @@ -102,7 +102,9 @@ func applyUnstructured(ctx context.Context, client dynamic.ResourceInterface, re existing.SetLabels(required.GetLabels()) existing.SetOwnerReferences(required.GetOwnerReferences()) - klog.V(2).Infof("Updating %s %s/%s due to diff: %v", required.GetKind(), required.GetNamespace(), required.GetName(), objDiff) + if reconciling { + klog.V(4).Infof("Updating %s %s/%s due to diff: %v", required.GetKind(), required.GetNamespace(), required.GetName(), objDiff) + } actual, err := client.Update(ctx, existing, metav1.UpdateOptions{}) if err != nil { @@ -115,6 +117,7 @@ type genericBuilder struct { client dynamic.ResourceInterface raw []byte modifier resourcebuilder.MetaV1ObjectModifierFunc + mode resourcebuilder.Mode } // NewGenericBuilder returns an implementation of resourcebuilder.Interface that @@ -127,6 +130,7 @@ func NewGenericBuilder(client dynamic.ResourceInterface, m manifest.Manifest) (r } func (b *genericBuilder) WithMode(m resourcebuilder.Mode) resourcebuilder.Interface { + b.mode = m return b } @@ -136,6 +140,7 @@ func (b *genericBuilder) WithModifier(f resourcebuilder.MetaV1ObjectModifierFunc } func (b *genericBuilder) Do(ctx context.Context) error { + reconciling := b.mode == resourcebuilder.ReconcilingMode ud := readUnstructuredV1OrDie(b.raw) if b.modifier != nil { b.modifier(ud) @@ -145,7 +150,7 @@ func (b *genericBuilder) Do(ctx context.Context) error { if err != nil { return err } else if !deleteReq { - _, _, err = applyUnstructured(ctx, b.client, ud) + _, _, err = applyUnstructured(ctx, b.client, ud, reconciling) } return err } diff --git a/pkg/cvo/internal/generic_test.go b/pkg/cvo/internal/generic_test.go index 325743c86..1fd7c7e44 100644 --- a/pkg/cvo/internal/generic_test.go +++ b/pkg/cvo/internal/generic_test.go @@ -31,10 +31,7 @@ func TestCreateOnlyCreate(t *testing.T) { } fakeClient := fake.NewSimpleDynamicClient(runtime.NewScheme()) - _, modified, err := applyUnstructured( - ctx, - fakeClient.Resource(schema.GroupVersionResource{Group: "config.openshift.io", Version: "v1", Resource: "featuregates"}), - obj.(*unstructured.Unstructured)) + _, modified, err := applyUnstructured(ctx, fakeClient.Resource(schema.GroupVersionResource{Group: "config.openshift.io", Version: "v1", Resource: "featuregates"}), obj.(*unstructured.Unstructured), false) if err != nil { t.Fatal(err) } @@ -76,10 +73,7 @@ func TestCreateOnlyUpdate(t *testing.T) { } fakeClient := fake.NewSimpleDynamicClient(runtime.NewScheme(), existingObj) - _, modified, err := applyUnstructured( - ctx, - fakeClient.Resource(schema.GroupVersionResource{Group: "config.openshift.io", Version: "v1", Resource: "featuregates"}), - obj.(*unstructured.Unstructured)) + _, modified, err := applyUnstructured(ctx, fakeClient.Resource(schema.GroupVersionResource{Group: "config.openshift.io", Version: "v1", Resource: "featuregates"}), obj.(*unstructured.Unstructured), false) if err != nil { t.Fatal(err) }