fix(controllers): remove conflicting status update in storeInstallationData (platform9#120)

When r.Status().Update() is called in storeInstallationData, it
overwrites the local scope.Config with the server's response. But also the
.Status update only updates the .status of the object.

Which means, any finalizers added to the object before this but yet to
be updated into the k8s data store, are over-written (effectively
reseting to what it was before the reconcile loop).

This causes the .Patch invocation in the defer-ed func (in .Reconcile)
to not see any new finalizers:
- both before and after, there are no changes to finalziers, leaving the
finalizer change unsaved

Additionally, Status().Update bumps the ResourceVersion, causing a conflict
in the defer-patch's patchStatus step when the resource is updated.

As a result, remove the Status().Update call and let the outer
reconcile's defer-patch handle both status and metadata updates
atomically.

Also fix test assertions:
- k8sinstallerconfig_controller_test.go: check for "uninstall" key in the
  separate uninstallation secret, not the installation secret
- byomachine_controller_test.go: set UninstallationSecret in the test status
  patch, as the reconciler requires both secrets when Ready=true

Author: indradhanush

controllers/infrastructure/byomachine_controller_test.go

@@ -463,6 +463,12 @@ var _ = Describe("Controllers/ByomachineController", func() {
 								Name:       "K8sInstallationSecret",
 								APIVersion: "v1",
 							},
+							UninstallationSecret: &corev1.ObjectReference{
+								Kind:       "Secret",
+								Namespace:  defaultNamespace,
+								Name:       "K8sUninstallationSecret",
+								APIVersion: "v1",
+							},
 						}
 						Expect(ph.Patch(ctx, k8sInstallerConfig, patch.WithStatusObservedGeneration{})).Should(Succeed())
 

controllers/infrastructure/k8sinstallerconfig_controller.go

@@ -247,10 +247,9 @@ func (r *K8sInstallerConfigReconciler) storeInstallationData(ctx context.Context
 	scope.Config.Status.Ready = true
 	logger.Info("created installation and uninstallation secrets")
 
-	// Persist the status update
-	if err := r.Status().Update(ctx, scope.Config); err != nil {
-		return errors.Wrapf(err, "failed to update K8sInstallerConfig status with installation/uninstallation secret references")
-	}
+	// Status fields are set in-memory; the outer reconcile's defer-patch persists
+	// them alongside the finalizer. Using r.Status().Update here would overwrite
+	// the local object's ResourceVersion and cause a conflict in the defer-patch.
 
 	return nil
 }

controllers/infrastructure/k8sinstallerconfig_controller_test.go

@@ -287,12 +287,21 @@ var _ = Describe("Controllers/K8sInstallerConfigController", func() {
 					Namespace: k8sinstallerConfig.Namespace}})
 			Expect(err).NotTo(HaveOccurred())
 
-			createdSecret := &corev1.Secret{}
-			err = k8sClientUncached.Get(ctx, installerSecretLookupKey, createdSecret)
+			installSecret := &corev1.Secret{}
+			err = k8sClientUncached.Get(ctx, installerSecretLookupKey, installSecret)
 			Expect(err).ToNot(HaveOccurred())
-			_, exists := createdSecret.Data["install"]
+			_, exists := installSecret.Data["install"]
 			Expect(exists).To(BeTrue())
-			_, exists = createdSecret.Data["uninstall"]
+
+			// The controller creates a separate secret for the uninstall script.
+			uninstallSecretLookupKey := types.NamespacedName{
+				Name:      "byoh-uninstall-" + k8sinstallerConfig.Name,
+				Namespace: k8sinstallerConfig.Namespace,
+			}
+			uninstallSecret := &corev1.Secret{}
+			err = k8sClientUncached.Get(ctx, uninstallSecretLookupKey, uninstallSecret)
+			Expect(err).ToNot(HaveOccurred())
+			_, exists = uninstallSecret.Data["uninstall"]
 			Expect(exists).To(BeTrue())
 		})