Close to being able to add/delete failure domains.

Author: Joshua Reed

api/v1beta2/cloudstackcluster_webhook.go

@@ -18,7 +18,6 @@ package v1beta2
 
 import (
 	"fmt"
-	"reflect"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -94,12 +93,12 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
 	}
 	oldSpec := oldCluster.Spec
 
-	// No spec fields may be updated.
 	errorList := field.ErrorList(nil)
-	if !reflect.DeepEqual(oldSpec.FailureDomains, spec.FailureDomains) {
-		errorList = append(errorList, field.Forbidden(
-			field.NewPath("spec", "FailureDomains"), "FailureDomains and sub-attributes may not be modified after creation"))
+
+	if err := ValidateFailureDomainUpdates(oldSpec.FailureDomains, spec.FailureDomains); err != nil {
+		errorList = append(errorList, err)
 	}
+
 	if oldSpec.ControlPlaneEndpoint.Host != "" { // Need to allow one time endpoint setting via CAPC cluster controller.
 		errorList = webhookutil.EnsureStringFieldsAreEqual(
 			spec.ControlPlaneEndpoint.Host, oldSpec.ControlPlaneEndpoint.Host, "controlplaneendpoint.host", errorList)
@@ -111,6 +110,43 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
 	return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
 }
 
+// ValidateFailureDomainUpdates verifies that at least one failure domain has not been deleted, and
+// failure domains that are held over have not been modified.
+func ValidateFailureDomainUpdates(oldFDs, newFDs []CloudStackFailureDomainSpec) *field.Error {
+	newFDsByName := map[string]CloudStackFailureDomainSpec{}
+	for _, newFD := range newFDs {
+		newFDsByName[newFD.Name] = newFD
+	}
+
+	atLeastOneRemains := false
+	for _, oldFD := range oldFDs {
+		if newFD, present := newFDsByName[oldFD.Name]; present {
+			atLeastOneRemains = true
+			if !FailureDomainsEqual(newFD, oldFD) {
+				return field.Forbidden(field.NewPath("spec", "FailureDomains"),
+					fmt.Sprintf("Cannot change FailureDomain %s", oldFD.Name))
+			}
+		}
+	}
+	if !atLeastOneRemains {
+		return field.Forbidden(field.NewPath("spec", "FailureDomains"), "At least one FailureDomain must be unchanged on udpate.")
+	}
+	return nil
+}
+
+// FailureDomainsEqual is a manual deep equal on failure domains.
+func FailureDomainsEqual(fd1, fd2 CloudStackFailureDomainSpec) bool {
+	return fd1.Name == fd2.Name &&
+		fd1.ACSEndpoint == fd2.ACSEndpoint &&
+		fd1.Account == fd2.Account &&
+		fd1.Domain == fd2.Domain &&
+		fd1.Zone.Name == fd2.Zone.Name &&
+		fd1.Zone.ID == fd2.Zone.ID &&
+		fd1.Zone.Network.Name == fd2.Zone.Network.Name &&
+		fd1.Zone.Network.ID == fd2.Zone.Network.ID &&
+		fd1.Zone.Network.Type == fd2.Zone.Network.Type
+}
+
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
 func (r *CloudStackCluster) ValidateDelete() error {
 	cloudstackclusterlog.V(1).Info("entered validate delete webhook", "api resource name", r.Name)

controllers/cloudstackcluster_controller.go

@@ -90,6 +90,7 @@ func (r *CloudStackClusterReconciliationRunner) Reconcile() (res ctrl.Result, re
 		r.SetFailureDomainsStatusMap,
 		r.CreateFailureDomains(r.ReconciliationSubject.Spec.FailureDomains),
 		r.GetFailureDomains(r.FailureDomains),
+		r.RemoveExtraneousFailureDomains(r.FailureDomains),
 		r.VerifyFailureDomainCRDs,
 		r.SetReady)
 }

controllers/utils/failuredomains.go

@@ -72,6 +72,30 @@ func (r *ReconciliationRunner) GetFailureDomains(fds *infrav1.CloudStackFailureD
 	}
 }
 
+// RemoveExtraneousFailureDomains deletes failure domains no longer listed under the CloudStackCluster's spec.
+func (r *ReconciliationRunner) RemoveExtraneousFailureDomains(fds *infrav1.CloudStackFailureDomainList) CloudStackReconcilerMethod {
+	return func() (ctrl.Result, error) {
+		// Toss together a precense map.
+		fdPresenceByName := map[string]bool{}
+		for _, fdSpec := range r.CSCluster.Spec.FailureDomains {
+			name := fdSpec.Name
+			if !strings.HasSuffix(name, "-"+r.CAPICluster.ClusterName) { // Add cluster name suffix if missing.
+				name = name + "-" + r.CAPICluster.Name
+			}
+			fdPresenceByName[name] = true
+		}
+		// Send a deletion request for each FailureDomain no speced for.
+		for _, fd := range fds.Items {
+			if _, present := fdPresenceByName[fd.Name]; !present {
+				if err := r.K8sClient.Delete(r.RequestCtx, &fd); err != nil {
+					return ctrl.Result{}, errors.Wrap(err, "failed to delete obsolete failure domain")
+				}
+			}
+		}
+		return ctrl.Result{}, nil
+	}
+}
+
 // GetFailureDomainsAndRequeueIfMissing gets CloudStackFailureDomains owned by a CloudStackCluster and requeues if none are found.
 func (r *ReconciliationRunner) GetFailureDomainsAndRequeueIfMissing(fds *infrav1.CloudStackFailureDomainList) CloudStackReconcilerMethod {
 	return func() (ctrl.Result, error) {

pkg/cloud/instance.go

@@ -319,7 +319,8 @@ func (c *client) DestroyVMInstance(csMachine *infrav1.CloudStackMachine) error {
 		return err
 	}
 
-	return errors.New("VM deletion in progress")
+	return nil
+	//return errors.New("VM deletion in progress")
 }
 
 func (c *client) listVMInstanceDatadiskVolumeIDs(instanceID string) ([]string, error) {