From 974218f8f5fae813f3e087b4847aff48dea44b0c Mon Sep 17 00:00:00 2001
From: Yauheni Kaliuta <ykaliuta@redhat.com>
Date: Mon, 10 Jun 2024 10:40:19 +0300
Subject: [PATCH] deploy: refactor getResource to return NotFound for both
 cases

Avoid returning (nil, nil), convert error if CRD not present to
NotFound and handle them in the caller code.

Avoid checking for found == nil but handle only error code.

This is more idiomatic and less error prone.

Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
---
 pkg/deploy/deploy.go | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go
index 4279f11e20d..3de42f92bc3 100644
--- a/pkg/deploy/deploy.go
+++ b/pkg/deploy/deploy.go
@@ -32,9 +32,11 @@ import (
 	"strings"
 
 	"golang.org/x/exp/maps"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/yaml"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -208,23 +210,25 @@ func manageResource(ctx context.Context, cli client.Client, obj *unstructured.Un
 		return nil
 	}
 
-	// Return if error getting resource in cluster
 	found, err := getResource(ctx, cli, obj)
-	if err != nil {
-		return err
-	}
 
-	if !enabled {
+	// err == nil means found
+	if err == nil {
+		if enabled {
+			return updateResource(ctx, cli, obj, found, owner, componentName)
+		}
 		return handleDisabledComponent(ctx, cli, found, componentName)
 	}
 
-	// Create resource if it doesn't exist
-	if found == nil {
-		return createResource(ctx, cli, obj, owner)
+	if apierrs.IsNotFound(err) {
+		// Create resource if it doesn't exist and enabled
+		if enabled {
+			return createResource(ctx, cli, obj, owner)
+		}
+		return nil
 	}
 
-	// If resource already exists, update it.
-	return updateResource(ctx, cli, obj, found, owner, componentName)
+	return err
 }
 
 /*
@@ -358,23 +362,18 @@ func getResource(ctx context.Context, cli client.Client, obj *unstructured.Unstr
 	found := &unstructured.Unstructured{}
 	// Setting gvk is required to do Get request
 	found.SetGroupVersionKind(obj.GroupVersionKind())
-	if err := cli.Get(ctx, types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}, found); err != nil {
-		if errors.Is(err, &meta.NoKindMatchError{}) {
-			return nil, nil // ignore mising CRD error
-		}
-		if client.IgnoreNotFound(err) == nil {
-			return nil, nil // not found resource
-		}
+	err := cli.Get(ctx, types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}, found)
+	if errors.Is(err, &meta.NoKindMatchError{}) {
+		// convert the error to NotFound to handle both the same way in the caller
+		return nil, apierrs.NewNotFound(schema.GroupResource{Group: obj.GroupVersionKind().Group}, obj.GetName())
+	}
+	if err != nil {
 		return nil, err
 	}
 	return found, nil
 }
 
 func handleDisabledComponent(ctx context.Context, cli client.Client, found *unstructured.Unstructured, componentName string) error {
-	if found == nil {
-		return nil
-	}
-
 	resourceLabels := found.GetLabels()
 	componentCounter := getComponentCounter(resourceLabels)