From 2708e540ec42b01f7dffc0a99cbd2958ac67de70 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Mon, 14 Oct 2024 17:14:50 +0200 Subject: [PATCH] chore: use client.OjectKeyFromObject in client.Get() (#1301) Signed-off-by: Wen Zhou --- controllers/dscinitialization/monitoring.go | 2 +- controllers/dscinitialization/utils.go | 22 +++++-------------- pkg/cluster/resources.go | 10 +++------ pkg/cluster/roles.go | 4 ++-- pkg/trustedcabundle/trustedcabundle.go | 13 +++++------ tests/e2e/deletion_test.go | 6 ++--- tests/e2e/helper_test.go | 12 +++++----- .../integration/features/cleanup_int_test.go | 10 ++++----- .../features/servicemesh_feature_test.go | 4 ++-- 9 files changed, 32 insertions(+), 51 deletions(-) diff --git a/controllers/dscinitialization/monitoring.go b/controllers/dscinitialization/monitoring.go index cc5b7112175..96537c76241 100644 --- a/controllers/dscinitialization/monitoring.go +++ b/controllers/dscinitialization/monitoring.go @@ -418,7 +418,7 @@ func createMonitoringProxySecret(ctx context.Context, cli client.Client, name st } foundProxySecret := &corev1.Secret{} - err = cli.Get(ctx, client.ObjectKey{Name: name, Namespace: dsciInit.Spec.Monitoring.Namespace}, foundProxySecret) + err = cli.Get(ctx, client.ObjectKeyFromObject(desiredProxySecret), foundProxySecret) if err != nil { if k8serr.IsNotFound(err) { // Set Controller reference diff --git a/controllers/dscinitialization/utils.go b/controllers/dscinitialization/utils.go index e2e0cd10b22..be4d6ff071b 100644 --- a/controllers/dscinitialization/utils.go +++ b/controllers/dscinitialization/utils.go @@ -51,7 +51,7 @@ func (r *DSCInitializationReconciler) createOdhNamespace(ctx context.Context, ds // Create Application Namespace if it doesn't exist foundNamespace := &corev1.Namespace{} - err := r.Get(ctx, client.ObjectKey{Name: name}, foundNamespace) + err := r.Get(ctx, client.ObjectKeyFromObject(desiredNamespace), foundNamespace) if err != nil { if k8serr.IsNotFound(err) { log.Info("Creating namespace", "name", name) @@ -169,10 +169,7 @@ func (r *DSCInitializationReconciler) createDefaultRoleBinding(ctx context.Conte // Create RoleBinding if doesn't exists foundRoleBinding := &rbacv1.RoleBinding{} - err := r.Client.Get(ctx, client.ObjectKey{ - Name: name, - Namespace: name, - }, foundRoleBinding) + err := r.Client.Get(ctx, client.ObjectKeyFromObject(desiredRoleBinding), foundRoleBinding) if err != nil { if k8serr.IsNotFound(err) { // Set Controller reference @@ -292,10 +289,7 @@ func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context. // Create NetworkPolicy if it doesn't exist foundNetworkPolicy := &networkingv1.NetworkPolicy{} justCreated := false - err := r.Client.Get(ctx, client.ObjectKey{ - Name: name, - Namespace: name, - }, foundNetworkPolicy) + err := r.Client.Get(ctx, client.ObjectKeyFromObject(desiredNetworkPolicy), foundNetworkPolicy) if err != nil { if k8serr.IsNotFound(err) { // Set Controller reference @@ -397,10 +391,7 @@ func (r *DSCInitializationReconciler) createOdhCommonConfigMap(ctx context.Conte // Create Configmap if doesn't exists foundConfigMap := &corev1.ConfigMap{} - err := r.Client.Get(ctx, client.ObjectKey{ - Name: name, - Namespace: name, - }, foundConfigMap) + err := r.Client.Get(ctx, client.ObjectKeyFromObject(desiredConfigMap), foundConfigMap) if err != nil { if k8serr.IsNotFound(err) { // Set Controller reference @@ -430,10 +421,7 @@ func (r *DSCInitializationReconciler) createUserGroup(ctx context.Context, dscIn // Otherwise is errors with "error": "Group.user.openshift.io \"odh-admins\" is invalid: users: Invalid value: \"null\": users in body must be of type array: \"null\""} Users: []string{}, } - err := r.Client.Get(ctx, client.ObjectKey{ - Name: userGroup.Name, - Namespace: dscInit.Spec.ApplicationsNamespace, - }, userGroup) + err := r.Client.Get(ctx, client.ObjectKeyFromObject(userGroup), userGroup) if err != nil { if k8serr.IsNotFound(err) { err = r.Client.Create(ctx, userGroup) diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 5e41eb25bc2..b1b0f84cdb5 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -72,7 +72,7 @@ func CreateSecret(ctx context.Context, cli client.Client, name, namespace string } foundSecret := &corev1.Secret{} - err := cli.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, foundSecret) + err := cli.Get(ctx, client.ObjectKeyFromObject(desiredSecret), foundSecret) if err != nil { if k8serr.IsNotFound(err) { err = cli.Create(ctx, desiredSecret) @@ -100,11 +100,7 @@ func CreateOrUpdateConfigMap(ctx context.Context, c client.Client, desiredCfgMap } existingCfgMap := &corev1.ConfigMap{} - err := c.Get(ctx, client.ObjectKey{ - Name: desiredCfgMap.Name, - Namespace: desiredCfgMap.Namespace, - }, existingCfgMap) - + err := c.Get(ctx, client.ObjectKeyFromObject(desiredCfgMap), existingCfgMap) if k8serr.IsNotFound(err) { return c.Create(ctx, desiredCfgMap) } else if err != nil { @@ -144,7 +140,7 @@ func CreateNamespace(ctx context.Context, cli client.Client, namespace string, m } foundNamespace := &corev1.Namespace{} - if getErr := cli.Get(ctx, client.ObjectKey{Name: namespace}, foundNamespace); client.IgnoreNotFound(getErr) != nil { + if getErr := cli.Get(ctx, client.ObjectKeyFromObject(desiredNamespace), foundNamespace); client.IgnoreNotFound(getErr) != nil { return nil, getErr } diff --git a/pkg/cluster/roles.go b/pkg/cluster/roles.go index c989915aefe..96ccbae0eb4 100644 --- a/pkg/cluster/roles.go +++ b/pkg/cluster/roles.go @@ -23,7 +23,7 @@ func CreateOrUpdateClusterRole(ctx context.Context, cli client.Client, name stri } foundClusterRole := &rbacv1.ClusterRole{} - err := cli.Get(ctx, client.ObjectKey{Name: desiredClusterRole.GetName()}, foundClusterRole) + err := cli.Get(ctx, client.ObjectKeyFromObject(desiredClusterRole), foundClusterRole) if k8serr.IsNotFound(err) { return desiredClusterRole, cli.Create(ctx, desiredClusterRole) } @@ -63,7 +63,7 @@ func CreateOrUpdateClusterRoleBinding(ctx context.Context, cli client.Client, na } foundClusterRoleBinding := &rbacv1.ClusterRoleBinding{} - err := cli.Get(ctx, client.ObjectKey{Name: desiredClusterRoleBinding.GetName()}, foundClusterRoleBinding) + err := cli.Get(ctx, client.ObjectKeyFromObject(desiredClusterRoleBinding), foundClusterRoleBinding) if k8serr.IsNotFound(err) { return desiredClusterRoleBinding, cli.Create(ctx, desiredClusterRoleBinding) } diff --git a/pkg/trustedcabundle/trustedcabundle.go b/pkg/trustedcabundle/trustedcabundle.go index 7e05256f34b..5be58c77283 100644 --- a/pkg/trustedcabundle/trustedcabundle.go +++ b/pkg/trustedcabundle/trustedcabundle.go @@ -71,10 +71,7 @@ func CreateOdhTrustedCABundleConfigMap(ctx context.Context, cli client.Client, n // Create Configmap if doesn't exist foundConfigMap := &corev1.ConfigMap{} - if err := cli.Get(ctx, client.ObjectKey{ - Name: CAConfigMapName, - Namespace: namespace, - }, foundConfigMap); err != nil { + if err := cli.Get(ctx, client.ObjectKeyFromObject(desiredConfigMap), foundConfigMap); err != nil { if k8serr.IsNotFound(err) { err = cli.Create(ctx, desiredConfigMap) if err != nil && !k8serr.IsAlreadyExists(err) { @@ -105,12 +102,12 @@ func DeleteOdhTrustedCABundleConfigMap(ctx context.Context, cli client.Client, n return cli.Delete(ctx, foundConfigMap) } -// IsTrustedCABundleUpdated check if data in CM "odh-trusted-ca-bundle" from applciation namespace matches DSCI's TrustedCABundle.CustomCABundle +// IsTrustedCABundleUpdated check if data in CM "odh-trusted-ca-bundle" from application namespace matches DSCI's TrustedCABundle.CustomCABundle // return false when these two are matching => skip update // return true when not match => need upate. func IsTrustedCABundleUpdated(ctx context.Context, cli client.Client, dscInit *dsciv1.DSCInitialization) (bool, error) { - userNamespace := &corev1.Namespace{} - if err := cli.Get(ctx, client.ObjectKey{Name: dscInit.Spec.ApplicationsNamespace}, userNamespace); err != nil { + appNamespace := &corev1.Namespace{} + if err := cli.Get(ctx, client.ObjectKey{Name: dscInit.Spec.ApplicationsNamespace}, appNamespace); err != nil { if k8serr.IsNotFound(err) { // if namespace is not found, return true. This is to ensure we reconcile, and check for other namespaces. return true, nil @@ -118,7 +115,7 @@ func IsTrustedCABundleUpdated(ctx context.Context, cli client.Client, dscInit *d return false, err } - if !ShouldInjectTrustedBundle(userNamespace) { + if !ShouldInjectTrustedBundle(appNamespace) { return false, nil } diff --git a/tests/e2e/deletion_test.go b/tests/e2e/deletion_test.go index 654ec01d62a..bb7caa7ada2 100644 --- a/tests/e2e/deletion_test.go +++ b/tests/e2e/deletion_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/api/errors" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" @@ -57,7 +57,7 @@ func (tc *testContext) testDeletionExistDSC() error { if dscerr != nil { return fmt.Errorf("error deleting DSC instance %s: %w", expectedDSC.Name, dscerr) } - } else if !errors.IsNotFound(err) { + } else if !k8serr.IsNotFound(err) { if err != nil { return fmt.Errorf("could not find DSC instance to delete: %w", err) } @@ -120,7 +120,7 @@ func (tc *testContext) testDeletionExistDSCI() error { if dscierr != nil { return fmt.Errorf("error deleting DSCI instance %s: %w", expectedDSCI.Name, dscierr) } - } else if !errors.IsNotFound(err) { + } else if !k8serr.IsNotFound(err) { if err != nil { return fmt.Errorf("could not find DSCI instance to delete :%w", err) } diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 3de13ee0b90..62d9e92b24a 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -14,7 +14,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/api/errors" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -59,7 +59,7 @@ func (tc *testContext) waitForOperatorDeployment(name string, replicas int32) er err := wait.PollUntilContextTimeout(tc.ctx, generalRetryInterval, operatorReadyTimeout, false, func(ctx context.Context) (bool, error) { controllerDeployment, err := tc.kubeClient.AppsV1().Deployments(tc.operatorNamespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { - if errors.IsNotFound(err) { + if k8serr.IsNotFound(err) { return false, nil } log.Printf("Failed to get %s controller deployment", name) @@ -207,7 +207,7 @@ func (tc *testContext) validateCRD(crdName string) error { err := wait.PollUntilContextTimeout(tc.ctx, generalRetryInterval, crdReadyTimeout, false, func(ctx context.Context) (bool, error) { err := tc.customClient.Get(ctx, obj, crd) if err != nil { - if errors.IsNotFound(err) { + if k8serr.IsNotFound(err) { return false, nil } log.Printf("Failed to get CRD %s", crdName) @@ -256,7 +256,7 @@ func getCSV(ctx context.Context, cli client.Client, name string, namespace strin } } - return nil, errors.NewNotFound(schema.GroupResource{}, name) + return nil, k8serr.NewNotFound(schema.GroupResource{}, name) } // Use existing or create a new one. @@ -279,7 +279,7 @@ func getSubscription(tc *testContext, name string, ns string) (*ofapi.Subscripti } err := tc.customClient.Get(tc.ctx, key, sub) - if errors.IsNotFound(err) { + if k8serr.IsNotFound(err) { return createSubscription(name, ns) } if err != nil { @@ -293,7 +293,7 @@ func waitCSV(tc *testContext, name string, ns string) error { interval := generalRetryInterval isReady := func(ctx context.Context) (bool, error) { csv, err := getCSV(ctx, tc.customClient, name, ns) - if errors.IsNotFound(err) { + if k8serr.IsNotFound(err) { return false, nil } if err != nil { diff --git a/tests/integration/features/cleanup_int_test.go b/tests/integration/features/cleanup_int_test.go index 8d0634cfc0c..070346ccbb3 100644 --- a/tests/integration/features/cleanup_int_test.go +++ b/tests/integration/features/cleanup_int_test.go @@ -4,7 +4,7 @@ import ( "context" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -81,7 +81,7 @@ var _ = Describe("feature cleanup", func() { WithContext(ctx). WithTimeout(fixtures.Timeout). WithPolling(fixtures.Interval). - Should(WithTransform(errors.IsNotFound, BeTrue())) + Should(WithTransform(k8serr.IsNotFound, BeTrue())) }) }) @@ -154,11 +154,11 @@ var _ = Describe("feature cleanup", func() { WithContext(ctx). WithTimeout(fixtures.Timeout). WithPolling(fixtures.Interval). - Should(WithTransform(errors.IsNotFound, BeTrue())) + Should(WithTransform(k8serr.IsNotFound, BeTrue())) Consistently(func() error { _, err := fixtures.GetFeatureTracker(ctx, envTestClient, namespace, featureName) - if errors.IsNotFound(err) { + if k8serr.IsNotFound(err) { return nil } return err @@ -213,7 +213,7 @@ func createdSecretHasOwnerReferenceToOwningFeature(namespace, featureName string func namespaceExists(ctx context.Context, cli client.Client, f *feature.Feature) (bool, error) { namespace, err := fixtures.GetNamespace(ctx, cli, "conditional-ns") - if errors.IsNotFound(err) { + if k8serr.IsNotFound(err) { return false, nil } // ensuring it fails if namespace is still deleting diff --git a/tests/integration/features/servicemesh_feature_test.go b/tests/integration/features/servicemesh_feature_test.go index 16a7b419063..0ced2db9745 100644 --- a/tests/integration/features/servicemesh_feature_test.go +++ b/tests/integration/features/servicemesh_feature_test.go @@ -5,7 +5,7 @@ import ( "path" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/api/errors" + k8serr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/controller-runtime/pkg/client" @@ -251,7 +251,7 @@ var _ = Describe("Service Mesh setup", func() { Expect(found).To(BeTrue()) _, err = fixtures.GetNamespace(ctx, envTestClient, serviceMeshSpec.Auth.Namespace) - Expect(errors.IsNotFound(err)).To(BeTrue()) + Expect(k8serr.IsNotFound(err)).To(BeTrue()) return extensionProviders