Differentiate status into own package, iterate all resourceTypes in c…

…heckProviderStatus

Signed-off-by: Kyle Squizzato <[email protected]>

internal/controller/managedcluster_controller.go

@@ -46,7 +46,7 @@ import (
 	hmc "github.com/Mirantis/hmc/api/v1alpha1"
 	"github.com/Mirantis/hmc/internal/helm"
 	"github.com/Mirantis/hmc/internal/telemetry"
-	"github.com/Mirantis/hmc/internal/utils"
+	"github.com/Mirantis/hmc/internal/utils/status"
 )
 
 // ManagedClusterReconciler reconciles a ManagedCluster object
@@ -119,13 +119,13 @@ func (r *ManagedClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 func (r *ManagedClusterReconciler) setStatusFromClusterStatus(
 	ctx context.Context, l logr.Logger, managedCluster *hmc.ManagedCluster) (bool, error) {
-	_, _, conditions, err := utils.GetConditionsFromResource(ctx, r.DynamicClient, schema.GroupVersionResource{
+	_, _, conditions, err := status.ConditionsFromResource(ctx, r.DynamicClient, schema.GroupVersionResource{
 		Group:    "cluster.x-k8s.io",
 		Version:  "v1beta1",
 		Resource: "clusters",
 	}, labels.SelectorFromSet(map[string]string{hmc.FluxHelmChartNameKey: managedCluster.Name}).String())
 	if err != nil {
-		notFoundErr := utils.ResourceNotFoundError{}
+		notFoundErr := status.ResourceNotFoundError{}
 		if errors.As(err, &notFoundErr) {
 			l.Info(err.Error())
 			return true, nil

internal/controller/management_controller.go

@@ -42,6 +42,7 @@ import (
 	"github.com/Mirantis/hmc/internal/certmanager"
 	"github.com/Mirantis/hmc/internal/helm"
 	"github.com/Mirantis/hmc/internal/utils"
+	"github.com/Mirantis/hmc/internal/utils/status"
 )
 
 // ManagementReconciler reconciles a Management object
@@ -165,9 +166,10 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag
 
 // checkProviderStatus checks the status of a provider associated with a given
 // ProviderTemplate name.  Since there's no way to determine resource Kind from
-// the given template iterate over all possible provider types only returning a
-// not found error if no resources of any provider type are found.
+// the given template iterate over all possible provider types.
 func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, providerTemplateName string) error {
+	var errs error
+
 	for _, resourceType := range []string{
 		"coreproviders",
 		"infrastructureproviders",
@@ -180,11 +182,11 @@ func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, provider
 			Resource: resourceType,
 		}
 
-		name, kind, conditions, err := utils.GetConditionsFromResource(ctx, r.DynamicClient, gvr,
+		name, kind, conditions, err := status.ConditionsFromResource(ctx, r.DynamicClient, gvr,
 			labels.SelectorFromSet(map[string]string{hmc.FluxHelmChartNameKey: providerTemplateName}).String(),
 		)
 		if err != nil {
-			notFoundErr := utils.ResourceNotFoundError{}
+			notFoundErr := status.ResourceNotFoundError{}
 			if errors.As(err, &notFoundErr) {
 				// Check the next resource type.
 				continue
@@ -201,14 +203,16 @@ func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, provider
 		}
 
 		if len(falseConditionTypes) > 0 {
-			return fmt.Errorf("%s: %s is not yet ready, has false conditions: %s", name, kind, strings.Join(falseConditionTypes, ", "))
+			errs = errors.Join(fmt.Errorf("%s: %s is not yet ready, has false conditions: %s",
+				name, kind, strings.Join(falseConditionTypes, ", ")))
 		}
+	}
 
-		return nil
+	if errs != nil {
+		return errs
 	}
 
-	// We've exhausted all possible provider types and found no items.
-	return fmt.Errorf("failed to find any provider resources affiliated with template: %s", providerTemplateName)
+	return nil
 }
 
 func (r *ManagementReconciler) Delete(ctx context.Context, management *hmc.Management) (ctrl.Result, error) {

internal/utils/conditions.go → internal/utils/status/status.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package utils
+package status
 
 import (
 	"context"
@@ -72,12 +72,12 @@ func (e ResourceNotFoundError) Error() string {
 	return fmt.Sprintf("no %s found, ignoring since object must be deleted or not yet created", e.Resource)
 }
 
-// GetConditionsFromResource fetches the conditions from a resource identified by
+// ConditionsFromResource fetches the conditions from a resource identified by
 // the provided GroupVersionResource and labelSelector.  The function returns
 // the name and kind of the resource and a slice of metav1.Condition.  If the
 // resource is not found, returns a ResourceNotFoundError which can be checked
 // by the caller to prevent reconciliation loops.
-func GetConditionsFromResource(
+func ConditionsFromResource(
 	ctx context.Context, dynamicClient dynamic.Interface,
 	gvr schema.GroupVersionResource, labelSelector string) (string, string, []metav1.Condition, error) {
 
@@ -95,7 +95,8 @@ func GetConditionsFromResource(
 	}
 
 	if len(list.Items) > 1 {
-		return "", "", nil, fmt.Errorf("expected to find only one of resource: %s with label: %q, found: %d", gvr.Resource, labelSelector, len(list.Items))
+		return "", "", nil, fmt.Errorf("expected to find only one of resource: %s with label: %q, found: %d",
+			gvr.Resource, labelSelector, len(list.Items))
 	}
 
 	kind, name := ObjKindName(&list.Items[0])

test/managedcluster/validate_deleted.go

@@ -18,7 +18,7 @@ import (
 	"errors"
 	"fmt"
 
-	internalutils "github.com/Mirantis/hmc/internal/utils"
+	"github.com/Mirantis/hmc/internal/utils/status"
 	"github.com/Mirantis/hmc/test/kubeclient"
 	"github.com/Mirantis/hmc/test/utils"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -43,7 +43,7 @@ func validateClusterDeleted(ctx context.Context, kc *kubeclient.KubeClient, clus
 			return fmt.Errorf("cluster: %q exists, but is not in 'Deleting' phase", clusterName)
 		}
 
-		conditions, err := internalutils.ConditionsFromUnstructured(cluster)
+		conditions, err := status.ConditionsFromUnstructured(cluster)
 		if err != nil {
 			return fmt.Errorf("failed to get conditions from unstructured object: %w", err)
 		}

test/managedcluster/validate_deployed.go

@@ -19,7 +19,7 @@ import (
 	"fmt"
 	"strings"
 
-	internalutils "github.com/Mirantis/hmc/internal/utils"
+	"github.com/Mirantis/hmc/internal/utils/status"
 	"github.com/Mirantis/hmc/test/kubeclient"
 	"github.com/Mirantis/hmc/test/utils"
 	. "github.com/onsi/ginkgo/v2"
@@ -91,7 +91,7 @@ func validateK0sControlPlanes(ctx context.Context, kc *kubeclient.KubeClient, cl
 			Fail(err.Error())
 		}
 
-		objKind, objName := internalutils.ObjKindName(&controlPlane)
+		objKind, objName := status.ObjKindName(&controlPlane)
 
 		// k0s does not use the metav1.Condition type for status.conditions,
 		// instead it uses a custom type so we can't use

test/utils/utils.go

@@ -21,7 +21,7 @@ import (
 	"os/exec"
 	"strings"
 
-	internalutils "github.com/Mirantis/hmc/internal/utils"
+	"github.com/Mirantis/hmc/internal/utils/status"
 	. "github.com/onsi/ginkgo/v2" //nolint:golint,revive
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -110,9 +110,9 @@ func GetProjectDir() (string, error) {
 // unstructured object and returns an error if any of the conditions are not
 // true.  Conditions are expected to be of type metav1.Condition.
 func ValidateConditionsTrue(unstrObj *unstructured.Unstructured) error {
-	objKind, objName := internalutils.ObjKindName(unstrObj)
+	objKind, objName := status.ObjKindName(unstrObj)
 
-	conditions, err := internalutils.ConditionsFromUnstructured(unstrObj)
+	conditions, err := status.ConditionsFromUnstructured(unstrObj)
 	if err != nil {
 		return fmt.Errorf("failed to get conditions from unstructured object: %w", err)
 	}
@@ -141,7 +141,7 @@ func ConvertConditionsToString(condition metav1.Condition) string {
 
 // ValidateObjectNamePrefix checks if the given object name has the given prefix.
 func ValidateObjectNamePrefix(unstrObj *unstructured.Unstructured, clusterName string) error {
-	objKind, objName := internalutils.ObjKindName(unstrObj)
+	objKind, objName := status.ObjKindName(unstrObj)
 
 	// Verify the machines are prefixed with the cluster name and fail
 	// the test if they are not.