incorporate review comments

Mirantis · Nov 20, 2023 · 7caab5f · 7caab5f
1 parent eea5ddb
commit 7caab5f
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 18 deletions.
diff --git a/api/v1alpha1/manifest_types.go b/api/v1alpha1/manifest_types.go
@@ -12,7 +12,6 @@ type ManifestSpec struct {
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 	// Important: Run "make" to regenerate code after modifying this file
 
-	// Foo is an example field of Manifest. Edit manifest_types.go to remove/update
 	Url      string           `json:"url"`
 	Checksum string           `json:"checksum"`
 	Objects  []ManifestObject `json:"objects,omitempty"`
@@ -24,6 +23,7 @@ type ManifestStatus struct {
 	// Important: Run "make" to regenerate code after modifying this file
 }
 
+// ManifestObject consists of the fields required to update/delete an object
 type ManifestObject struct {
 	Kind      string `json:"kind"`
 	Name      string `json:"name"`

diff --git a/config/crd/bases/boundless.mirantis.com_manifests.yaml b/config/crd/bases/boundless.mirantis.com_manifests.yaml
@@ -39,6 +39,8 @@ spec:
                 type: string
               objects:
                 items:
+                  description: ManifestObject consists of the fields required to update/delete
+                    an object
                   properties:
                     kind:
                       type: string
@@ -53,8 +55,6 @@ spec:
                   type: object
                 type: array
               url:
-                description: Foo is an example field of Manifest. Edit manifest_types.go
-                  to remove/update
                 type: string
             required:
             - checksum

diff --git a/controllers/addon_controller.go b/controllers/addon_controller.go
@@ -17,9 +17,11 @@ import (
 )
 
 const (
-	kindManifest       = "manifest"
-	kindChart          = "chart"
-	BoundlessNamespace = "boundless-system"
+	kindManifest            = "manifest"
+	kindChart               = "chart"
+	BoundlessNamespace      = "boundless-system"
+	addonHelmchartFinalizer = "boundless.mirantis.com/helmchart-finalizer"
+	addonManifestFinalizer  = "boundless.mirantis.com/manifest-finalizer"
 )
 
 // AddonReconciler reconciles a Addon object
@@ -75,21 +77,19 @@ func (r *AddonReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 
 		hc := helm.NewHelmChartController(r.Client, logger)
 
-		addonFinalizerName := "boundless.mirantis.com/helmchart-finalizer"
-
 		if instance.ObjectMeta.DeletionTimestamp.IsZero() {
 			// The object is not being deleted, so if it does not have our finalizer,
 			// then lets add the finalizer and update the object. This is equivalent
 			// registering our finalizer.
-			if !controllerutil.ContainsFinalizer(instance, addonFinalizerName) {
-				controllerutil.AddFinalizer(instance, addonFinalizerName)
+			if !controllerutil.ContainsFinalizer(instance, addonHelmchartFinalizer) {
+				controllerutil.AddFinalizer(instance, addonHelmchartFinalizer)
 				if err := r.Update(ctx, instance); err != nil {
 					return ctrl.Result{}, err
 				}
 			}
 		} else {
 			// The object is being deleted
-			if controllerutil.ContainsFinalizer(instance, addonFinalizerName) {
+			if controllerutil.ContainsFinalizer(instance, addonHelmchartFinalizer) {
 				// our finalizer is present, so lets delete the helm chart
 				if err := hc.DeleteHelmChart(chart, instance.Spec.Namespace); err != nil {
 					// if fail to delete the helm chart here, return with error
@@ -98,7 +98,7 @@ func (r *AddonReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 				}
 
 				// remove our finalizer from the list and update it.
-				controllerutil.RemoveFinalizer(instance, addonFinalizerName)
+				controllerutil.RemoveFinalizer(instance, addonHelmchartFinalizer)
 				if err := r.Update(ctx, instance); err != nil {
 					return ctrl.Result{}, err
 				}
@@ -116,21 +116,19 @@ func (r *AddonReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 	case kindManifest:
 		mc := manifest.NewManifestController(r.Client, logger)
 
-		addonFinalizerName := "boundless.mirantis.com/manifest-finalizer"
-
 		if instance.ObjectMeta.DeletionTimestamp.IsZero() {
 			// The object is not being deleted, so if it does not have our finalizer,
 			// then lets add the finalizer and update the object. This is equivalent
 			// registering our finalizer.
-			if !controllerutil.ContainsFinalizer(instance, addonFinalizerName) {
-				controllerutil.AddFinalizer(instance, addonFinalizerName)
+			if !controllerutil.ContainsFinalizer(instance, addonManifestFinalizer) {
+				controllerutil.AddFinalizer(instance, addonManifestFinalizer)
 				if err := r.Update(ctx, instance); err != nil {
 					return ctrl.Result{}, err
 				}
 			}
 		} else {
 			// The object is being deleted
-			if controllerutil.ContainsFinalizer(instance, addonFinalizerName) {
+			if controllerutil.ContainsFinalizer(instance, addonManifestFinalizer) {
 				// our finalizer is present, so lets delete the helm chart
 				if err := mc.DeleteManifest(BoundlessNamespace, instance.Spec.Name, instance.Spec.Manifest.URL); err != nil {
 					// if fail to delete the manifest here, return with error
@@ -139,7 +137,7 @@ func (r *AddonReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 				}
 
 				// remove our finalizer from the list and update it.
-				controllerutil.RemoveFinalizer(instance, addonFinalizerName)
+				controllerutil.RemoveFinalizer(instance, addonManifestFinalizer)
 				if err := r.Update(ctx, instance); err != nil {
 					return ctrl.Result{}, err
 				}