Merge pull request #118 from SomtochiAma/helper-functions

Refactors code to set and get status from object

Author: k8s-ci-robot

pkg/patterns/addon/application.go

@@ -19,7 +19,6 @@ package addon
 import (
 	"context"
 	"errors"
-	"fmt"
 
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/utils"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative"
@@ -42,15 +41,17 @@ const (
 
 // TransformApplicationFromStatus modifies the Application in the deployment based off the CommonStatus
 func TransformApplicationFromStatus(ctx context.Context, instance declarative.DeclarativeObject, objects *manifest.Objects) error {
-	version, err := utils.GetCommonVersion(instance)
+	status, err := utils.GetCommonStatus(instance)
 	if err != nil {
-		return fmt.Errorf("unable to get version from object: %v", err)
+		return err
 	}
+	healthy := status.Healthy
 
-	healthy, err := utils.GetCommonHealth(instance)
+	spec, err := utils.GetCommonSpec(instance)
 	if err != nil {
-		return fmt.Errorf("unable to get health from object: %v", err)
+		return err
 	}
+	version := spec.Version
 
 	app, err := declarative.ExtractApplication(objects)
 	if err != nil {

pkg/patterns/addon/pkg/loaders/fs.go

@@ -65,21 +65,18 @@ func (c *ManifestLoader) ResolveManifest(ctx context.Context, object runtime.Obj
 		componentName string
 	)
 
-	version, err := utils.GetCommonVersion(object)
+	spec, err := utils.GetCommonSpec(object)
 	if err != nil {
 		return nil, err
 	}
+	version = spec.Version
+	channelName = spec.Channel
 
 	componentName, err = utils.GetCommonName(object)
 	if err != nil {
 		return nil, err
 	}
 
-	channelName, err = utils.GetCommonChannel(object)
-	if err != nil {
-		return nil, err
-	}
-
 	// TODO: We should actually do id (1.1.2-aws or 1.1.1-nginx). But maybe YAGNI
 	id := version
 

pkg/patterns/addon/pkg/status/aggregate.go

@@ -21,13 +21,12 @@ import (
 	"fmt"
 	"reflect"
 
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/utils"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	addonv1alpha1 "sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/apis/v1alpha1"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/manifest"
 )
@@ -80,54 +79,23 @@ func (a *aggregator) Reconciled(ctx context.Context, src declarative.Declarative
 
 	log.WithValues("object", src).WithValues("status", statusHealthy).V(2).Info("built status")
 
-	unstruct, ok := src.(*unstructured.Unstructured)
-	instance, commonOkay := src.(addonv1alpha1.CommonObject)
-	changed := false
-
-	if commonOkay {
-		var status = instance.GetCommonStatus()
-		status.Errors = statusErrors
-		status.Healthy = statusHealthy
-
-		if !reflect.DeepEqual(status, instance.GetCommonStatus()) {
-			status.Healthy = statusHealthy
+	currentStatus, err := utils.GetCommonStatus(src)
+	if err != nil {
+		return err
+	}
 
-			log.WithValues("name", instance.GetName()).WithValues("status", status).Info("updating status")
-			changed = true
-		}
-	} else if ok {
-		unstructStatus := make(map[string]interface{})
+	status := currentStatus
+	status.Healthy = statusHealthy
+	status.Errors = statusErrors
 
-		s, _, err := unstructured.NestedMap(unstruct.Object, "status")
+	if !reflect.DeepEqual(status, currentStatus) {
+		err := utils.SetCommonStatus(src, status)
 		if err != nil {
-			log.Error(err, "getting status")
-			return fmt.Errorf("unable to get status from unstructured: %v", err)
-		}
-
-		unstructStatus = s
-		unstructStatus["Healthy"] = statusHealthy
-		unstructStatus["Errors"] = statusErrors
-		if !reflect.DeepEqual(unstruct, s) {
-			err = unstructured.SetNestedField(unstruct.Object, statusHealthy, "status", "healthy")
-			if err != nil {
-				log.Error(err, "updating status")
-				return fmt.Errorf("unable to set status in unstructured: %v", err)
-			}
-
-			err = unstructured.SetNestedStringSlice(unstruct.Object, statusErrors, "status", "errors")
-			if err != nil {
-				log.Error(err, "updating status")
-				return fmt.Errorf("unable to set status in unstructured: %v", err)
-			}
-			changed = true
+			return err
 		}
-	} else {
-		return fmt.Errorf("instance %T was not an addonsv1alpha1.CommonObject or unstructured.Unstructured", src)
-	}
 
-	if changed == true {
-		log.WithValues("name", src.GetName()).WithValues("status", statusHealthy).Info("updating status")
-		err := a.client.Status().Update(ctx, src)
+		log.WithValues("name", src.GetName()).WithValues("status", status).Info("updating status")
+		err = a.client.Status().Update(ctx, src)
 		if err != nil {
 			log.Error(err, "updating status")
 			return err

pkg/patterns/addon/pkg/status/kstatus.go

@@ -4,11 +4,10 @@ import (
 	"context"
 	"fmt"
 
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/cli-utils/pkg/kstatus/status"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	addonsv1alpha1 "sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/apis/v1alpha1"
+	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/utils"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/manifest"
 )
@@ -47,41 +46,21 @@ func (k *kstatusAggregator) Reconciled(ctx context.Context, src declarative.Decl
 		}
 	}
 
-	addonObject, ok := src.(addonsv1alpha1.CommonObject)
-	unstruct, unstructOk := src.(*unstructured.Unstructured)
 	aggregatedPhase := string(aggregateStatus(statusMap))
-	changed := false
-	if ok {
-		addonStatus := addonObject.GetCommonStatus()
-		if addonStatus.Phase != aggregatedPhase {
-			addonStatus.Phase = aggregatedPhase
-			addonObject.SetCommonStatus(addonStatus)
-			changed = true
-		}
-	} else if unstructOk {
-		statusPhase, _, err := unstructured.NestedString(unstruct.Object, "status", "phase")
+
+	currentStatus, err := utils.GetCommonStatus(src)
+	if err != nil {
+		log.Error(err, "error retrieving status")
+		return err
+	}
+	if currentStatus.Phase != aggregatedPhase {
+		currentStatus.Phase = aggregatedPhase
+		err := utils.SetCommonStatus(src, currentStatus)
 		if err != nil {
-			log.Error(err, "error retrieving status")
 			return err
 		}
-
-		if statusPhase != aggregatedPhase {
-			err := unstructured.SetNestedField(unstruct.Object, aggregatedPhase, "status", "phase")
-			if err != nil {
-				log.Error(err, "error retrieving status")
-				return err
-			}
-			changed = true
-		}
-	} else {
-		return fmt.Errorf("instance %T was not an addonsv1alpha1.CommonObject or unstructured."+
-			"Unstructured",
-			src)
-	}
-
-	if changed == true {
 		log.WithValues("name", src.GetName()).WithValues("phase", aggregatedPhase).Info("updating status")
-		err := k.client.Status().Update(ctx, src)
+		err = k.client.Status().Update(ctx, src)
 		if err != nil {
 			log.Error(err, "error updating status")
 			return fmt.Errorf("error error status: %v", err)

pkg/patterns/addon/pkg/status/version.go

@@ -6,10 +6,9 @@ import (
 	"reflect"
 
 	"github.com/blang/semver"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	addonsv1alpha1 "sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/apis/v1alpha1"
+	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/utils"
 
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/manifest"
@@ -64,44 +63,22 @@ func (p *versionCheck) VersionCheck(
 		fmt.Sprintf("manifest needs operator version >= %v, this operator is version %v", minOperatorVersion.String(),
 			p.operatorVersion.String()),
 	}
-	unstruct, ok := src.(*unstructured.Unstructured)
-	addonObject, commonOkay := src.(addonsv1alpha1.CommonObject)
 
-	if ok {
-		unstructStatus := make(map[string]interface{})
-
-		s, _, err := unstructured.NestedMap(unstruct.Object, "status")
-		if err != nil {
-			log.Error(err, "getting status")
-			return false, fmt.Errorf("unable to get status from unstructured: %v", err)
-		}
-
-		unstructStatus = s
-		unstructStatus["Healthy"] = false
-		unstructStatus["Errors"] = errors
+	currentStatus, err := utils.GetCommonStatus(src)
+	if err != nil {
+		log.Error(err, "getting status")
+		return false, err
+	}
 
-		if !reflect.DeepEqual(unstruct, s) {
-			err = unstructured.SetNestedField(unstruct.Object, false, "status", "healthy")
-			if err != nil {
-				log.Error(err, "unable to updating status in unstructured")
-			}
+	status := currentStatus
+	status.Healthy = false
+	status.Errors = errors
 
-			err = unstructured.SetNestedStringSlice(unstruct.Object, errors, "status", "errors")
-			if err != nil {
-				log.Error(err, "unable to updating status in unstructured")
-			}
-		}
-	} else if commonOkay {
-		status := addonObject.GetCommonStatus()
-		status.Healthy = false
-		status.Errors = errors
-		if !reflect.DeepEqual(status, addonObject.GetCommonStatus()) {
-			status.Healthy = false
-			status.Errors = errors
-			addonObject.SetCommonStatus(status)
+	if !reflect.DeepEqual(status, currentStatus) {
+		err := utils.SetCommonStatus(src, status)
+		if err != nil {
+			log.Error(err, "unable to updating status")
 		}
-	} else {
-		return false, fmt.Errorf("instance %T was not an addonsv1alpha1.CommonObject or unstructured.Unstructured", src)
 	}
 
 	return false, fmt.Errorf("operator not qualified, manifest needs operator version >= %v", minOperatorVersion.String())

pkg/patterns/addon/pkg/utils/helpers.go

@@ -9,48 +9,71 @@ import (
 	addonsv1alpha1 "sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/apis/v1alpha1"
 )
 
-func GetCommonVersion(instance runtime.Object) (string, error) {
+func genError(v runtime.Object) error {
+	return fmt.Errorf("instance %T is not addonsv1alpha1.CommonObject or unstructured", v)
+}
+
+func GetCommonStatus(instance runtime.Object) (addonsv1alpha1.CommonStatus, error) {
 	switch v := instance.(type) {
 	case addonsv1alpha1.CommonObject:
-		return v.CommonSpec().Version, nil
+		return v.GetCommonStatus(), nil
 	case *unstructured.Unstructured:
-		version, _, err := unstructured.NestedString(v.Object, "spec", "version")
+		unstructStatus, _, err := unstructured.NestedMap(v.Object, "status")
+		if err != nil {
+			return addonsv1alpha1.CommonStatus{}, fmt.Errorf("unable to get status from unstuctured: %v", err)
+		}
+		var addonStatus addonsv1alpha1.CommonStatus
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructStatus, &addonStatus)
 		if err != nil {
-			return "", fmt.Errorf("unable to get version from unstuctured: %v", err)
+			return addonStatus, err
 		}
-		return version, nil
+
+		return addonStatus, nil
 	default:
-		return "", fmt.Errorf("instance %T is not addonsv1alpha1.CommonObject or unstructured", v)
+		return addonsv1alpha1.CommonStatus{}, genError(v)
 	}
 }
 
-func GetCommonHealth(instance runtime.Object) (bool, error) {
+func SetCommonStatus(instance runtime.Object, status addonsv1alpha1.CommonStatus) error {
 	switch v := instance.(type) {
 	case addonsv1alpha1.CommonObject:
-		return v.GetCommonStatus().Healthy, nil
+		v.SetCommonStatus(status)
 	case *unstructured.Unstructured:
-		version, _, err := unstructured.NestedBool(v.Object, "status", "healthy")
+		unstructStatus, err := runtime.DefaultUnstructuredConverter.ToUnstructured(status)
+		if err != nil {
+			return fmt.Errorf("unable to convert unstructured to addonStatus: %v", err)
+		}
+
+		err = unstructured.SetNestedMap(v.Object, unstructStatus, "status")
 		if err != nil {
-			return false, fmt.Errorf("unable to get version from unstuctured: %v", err)
+			return fmt.Errorf("unable to set status in unstructured: %v", err)
 		}
-		return version, nil
+
+		return nil
 	default:
-		return false, fmt.Errorf("instance %T is not addonsv1alpha1.CommonObject or unstructured", v)
+		return genError(v)
 	}
+	return nil
 }
 
-func GetCommonChannel(instance runtime.Object) (string, error) {
+func GetCommonSpec(instance runtime.Object) (addonsv1alpha1.CommonSpec, error) {
 	switch v := instance.(type) {
 	case addonsv1alpha1.CommonObject:
-		return v.CommonSpec().Channel, nil
+		return v.CommonSpec(), nil
 	case *unstructured.Unstructured:
-		channel, _, err := unstructured.NestedString(v.Object, "spec", "channel")
+		unstructSpec, _, err := unstructured.NestedMap(v.Object, "spec")
 		if err != nil {
-			return "", fmt.Errorf("unable to get version from unstuctured: %v", err)
+			return addonsv1alpha1.CommonSpec{}, fmt.Errorf("unable to get spec from unstuctured: %v", err)
 		}
-		return channel, nil
+		var addonSpec addonsv1alpha1.CommonSpec
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructSpec, &addonSpec)
+		if err != nil {
+			return addonSpec, err
+		}
+
+		return addonSpec, nil
 	default:
-		return "", fmt.Errorf("instance %T is not addonsv1alpha1.CommonObject or unstructured", v)
+		return addonsv1alpha1.CommonSpec{}, genError(v)
 	}
 }
 
@@ -61,6 +84,6 @@ func GetCommonName(instance runtime.Object) (string, error) {
 	case *unstructured.Unstructured:
 		return strings.ToLower(v.GetKind()), nil
 	default:
-		return "", fmt.Errorf("instance %T is not addonsv1alpha1.CommonObject or unstructured", v)
+		return "", genError(v)
 	}
 }