Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: use client.OjectKeyFromObject in client.Get() and add k8serr as alias #1301

Merged
merged 1 commit into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controllers/dscinitialization/monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
lburgazzoli marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
if k8serr.IsNotFound(err) {
// Set Controller reference
Expand Down
22 changes: 5 additions & 17 deletions controllers/dscinitialization/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 3 additions & 7 deletions pkg/cluster/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/cluster/roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
13 changes: 5 additions & 8 deletions pkg/trustedcabundle/trustedcabundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -105,20 +102,20 @@ 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
}
return false, err
}

if !ShouldInjectTrustedBundle(userNamespace) {
if !ShouldInjectTrustedBundle(appNamespace) {
return false, nil
}

Expand Down
6 changes: 3 additions & 3 deletions tests/e2e/deletion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
12 changes: 6 additions & 6 deletions tests/e2e/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/features/cleanup_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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()))
})

})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/features/servicemesh_feature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down