Base preflight check function

Author: somtochiama

pkg/patterns/addon/pkg/status/basic.go

@@ -32,10 +32,15 @@ func NewBasic(client client.Client) declarative.Status {
 
 // NewBasicVersionCheck provides an implementation of declarative.Status that
 // performs version checks for the version of the operator that the manifest requires.
-func NewBasicVersionChecks(client client.Client, version string) declarative.Status {
+func NewBasicVersionChecks(client client.Client, version string) (declarative.Status, error) {
+	v, err := NewVersionCheck(client, version)
+	if err != nil {
+		return nil, err
+	}
+
 	return &declarative.StatusBuilder{
-		ReconciledImpl: NewAggregator(client),
-		VersionCheckImpl:  NewVersionCheck(client, version),
+		ReconciledImpl:   NewAggregator(client),
+		VersionCheckImpl: v,
 		// no preflight checks
-	}
+	}, nil
 }

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

@@ -3,26 +3,29 @@ package status
 import (
 	"context"
 	"fmt"
+
 	"github.com/blang/semver"
 	"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/controller-runtime/pkg/log"
-
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/manifest"
 )
 
 // NewVersionCheck provides an implementation of declarative.Reconciled that
 // checks the version of the operator if it is up to the version required by the manifest
-func NewVersionCheck(client client.Client, version string) *versionCheck {
-	return &versionCheck{client, version}
+func NewVersionCheck(client client.Client, operatorVersionString string) (*versionCheck, error) {
+	operatorVersion, err := semver.Parse(operatorVersionString)
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse operator version %q: %v", operatorVersionString, err)
+	}
+	return &versionCheck{client: client, operatorVersion: operatorVersion}, nil
 }
 
 type versionCheck struct {
-	client  client.Client
-	version string
+	client          client.Client
+	operatorVersion semver.Version
 }
 
 func (p *versionCheck) VersionCheck(
@@ -31,39 +34,27 @@ func (p *versionCheck) VersionCheck(
 	objs *manifest.Objects,
 ) (bool, error) {
 	log := log.Log
-	zeroVersion := semver.Version{}
-	var versionNeededStr string
-	maxVersion := zeroVersion
+	var minOperatorVersion semver.Version
 
 	// Look for annotation from any resource with the max version
 	for _, obj := range objs.Items {
-		unstruct := obj.UnstructuredObject().Object
-		metadata := unstruct["metadata"].(map[string]interface{})
-		annotations, ok := metadata["annotations"].(map[string]interface{})
-		if ok {
-			versionNeededStr, _ = annotations["addons.k8s.io/operator-version"].(string)
-			log.WithValues("version", versionNeededStr).Info("Got version, %v")
+		annotations := obj.UnstructuredObject().GetAnnotations()
+		if versionNeededStr, ok := annotations["addons.k8s.io/min-operator-version"]; ok {
+			log.WithValues("min-operator-version", versionNeededStr).Info("Got version requirement addons.k8s.io/operator-version")
 
-			versionActual, err := semver.Make(versionNeededStr)
+			versionNeeded, err := semver.Parse(versionNeededStr)
 			if err != nil {
-				log.WithValues("version", versionNeededStr).Info("Unable to convert string to version, skipping this object")
-				continue
+				log.WithValues("version", versionNeededStr).Error(err, "Unable to parse version restriction")
+				return false, err
 			}
 
-			if versionActual.GT(maxVersion) {
-				maxVersion = versionActual
+			if versionNeeded.GT(minOperatorVersion) {
+				minOperatorVersion = versionNeeded
 			}
 		}
 	}
 
-	// TODO(somtochi): Do we want to return an error when the version is invalid or just skip and use the operator?
-	operatorVersion, err := semver.Make(p.version)
-	if err != nil {
-		log.WithValues("version", p.version).Info("Unable to convert string to version, skipping check")
-		return true, nil
-	}
-
-	if maxVersion.Equals(zeroVersion) || !maxVersion.GT(operatorVersion) {
+	if p.operatorVersion.GTE(minOperatorVersion) {
 		return true, nil
 	}
 
@@ -75,11 +66,11 @@ func (p *versionCheck) VersionCheck(
 	status := addonsv1alpha1.CommonStatus{
 		Healthy: false,
 		Errors: []string{
-			fmt.Sprintf("Addons needs version %v, this operator is version %v", maxVersion.String(), operatorVersion.String()),
+			fmt.Sprintf("manifest needs operator version >= %v, this operator is version %v", minOperatorVersion.String(), p.operatorVersion.String()),
 		},
 	}
 	log.WithValues("name", addonObject.GetName()).WithValues("status", status).Info("updating status")
 	addonObject.SetCommonStatus(status)
 
-	return false, fmt.Errorf("operator not qualified, manifest needs operator >= %v", maxVersion.String())
+	return false, fmt.Errorf("operator not qualified, manifest needs operator version >= %v", minOperatorVersion.String())
 }

pkg/patterns/declarative/reconciler.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -52,6 +53,9 @@ type Reconciler struct {
 
 	mgr manager.Manager
 
+	// recorder is the EventRecorder for creating k8s events
+	recorder record.EventRecorder
+
 	options reconcilerParams
 }
 
@@ -71,6 +75,10 @@ func (r *Reconciler) Init(mgr manager.Manager, prototype DeclarativeObject, opts
 	r.prototype = prototype
 	r.kubectl = kubectl
 
+	// TODO: Can we derive the name from prototype?
+	controllerName := "addon-controller"
+	r.recorder = mgr.GetEventRecorderFor(controllerName)
+
 	r.client = mgr.GetClient()
 	r.config = mgr.GetConfig()
 	r.mgr = mgr
@@ -115,11 +123,7 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (result reconcile.Resu
 	}
 
 	if r.options.status != nil {
-<<<<<<< HEAD
-		if err = r.options.status.Preflight(ctx, instance); err != nil {
-=======
 		if err := r.options.status.Preflight(ctx, instance); err != nil {
->>>>>>> 3128f67... Changes preflight to version checks
 			log.Error(err, "preflight check failed, not reconciling")
 			return reconcile.Result{}, err
 		}
@@ -145,13 +149,14 @@ func (r *Reconciler) reconcileExists(ctx context.Context, name types.NamespacedN
 	log.WithValues("objects", fmt.Sprintf("%d", len(objects.Items))).Info("built deployment objects")
 
 	if r.options.status != nil {
-		bool, err := r.options.status.VersionCheck(ctx, instance, objects)
-		if  err != nil {
-			if !bool {
+		isValidVersion, err := r.options.status.VersionCheck(ctx, instance, objects)
+		if err != nil {
+			if !isValidVersion {
 				// r.client isn't exported so can't be updated in version check function
-				r.client.Status().Update(ctx, instance)
-				recorder := r.mgr.GetEventRecorderFor(fmt.Sprintf("%v", instance.GetName()))
-				recorder.Event(instance, "Warning", "Failed version check", err.Error())
+				if err := r.client.Status().Update(ctx, instance); err != nil {
+					return reconcile.Result{}, err
+				}
+				r.recorder.Event(instance, "Warning", "Failed version check", err.Error())
 				log.Error(err, "Version check failed, not reconciling")
 				return reconcile.Result{}, nil
 			}

pkg/patterns/declarative/status.go

@@ -45,15 +45,15 @@ type Preflight interface {
 
 type VersionCheck interface {
 	// VersionCheck checks if the version of the operator is greater than or equal to the
-	//version requested by objects in the manifest, if it isn't it updates the status and
+	// version requested by objects in the manifest, if it isn't it updates the status and
 	// events and stops reconciling
 	VersionCheck(context.Context, DeclarativeObject, *manifest.Objects) (bool, error)
 }
 
 // StatusBuilder provides a pluggable implementation of Status
 type StatusBuilder struct {
-	ReconciledImpl Reconciled
-	PreflightImpl  Preflight
+	ReconciledImpl   Reconciled
+	PreflightImpl    Preflight
 	VersionCheckImpl VersionCheck
 }
 
@@ -71,7 +71,7 @@ func (s *StatusBuilder) Preflight(ctx context.Context, src DeclarativeObject) er
 	return nil
 }
 
-func (s *StatusBuilder) VersionCheck(ctx context.Context, src DeclarativeObject,objs *manifest.Objects) (bool, error) {
+func (s *StatusBuilder) VersionCheck(ctx context.Context, src DeclarativeObject, objs *manifest.Objects) (bool, error) {
 	if s.VersionCheckImpl != nil {
 		return s.VersionCheckImpl.VersionCheck(ctx, src, objs)
 	}