merge from upstream

Author: wanyufe

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"
@@ -95,13 +94,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)
@@ -113,6 +111,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)

api/v1beta2/cloudstackfailuredomain_types.go

@@ -23,6 +23,7 @@ import (
 
 const (
 	FailureDomainFinalizer = "cloudstackfailuredomain.infrastructure.cluster.x-k8s.io"
+	FailureDomainLabelName = "cloudstackfailuredomain.infrastructure.cluster.x-k8s.io/name"
 	NetworkTypeIsolated    = "Isolated"
 	NetworkTypeShared      = "Shared"
 )

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/cloudstackfailuredomain_controller.go

@@ -20,7 +20,9 @@ import (
 	"context"
 
 	"github.com/pkg/errors"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta2"
@@ -105,21 +107,49 @@ func (r *CloudStackFailureDomainReconciliationRunner) Reconcile() (retRes ctrl.R
 }
 
 // ReconcileDelete on the ReconciliationRunner attempts to delete the reconciliation subject.
-func (r *CloudStackFailureDomainReconciliationRunner) ReconcileDelete() (retRes ctrl.Result, retErr error) {
+func (r *CloudStackFailureDomainReconciliationRunner) ReconcileDelete() (ctrl.Result, error) {
 	r.Log.Info("Deleting CloudStackFailureDomain")
-	// Address Isolated Networks.
-	if r.ReconciliationSubject.Spec.Zone.Network.Type == infrav1.NetworkTypeIsolated {
-		netName := r.ReconciliationSubject.Spec.Zone.Network.Name
-		if res, err := r.GetObjectByName(r.IsoNetMetaName(netName), r.IsoNet)(); r.ShouldReturn(res, err) {
-			return res, err
-		}
-		if r.IsoNet.Name != "" {
-			if err := r.K8sClient.Delete(r.RequestCtx, r.IsoNet); err != nil {
-				return ctrl.Result{}, err
+
+	return r.RunReconciliationStages(
+		r.ClearMachines,
+		r.DeleteOwnedObjects(
+			infrav1.GroupVersion.WithKind("CloudStackAffinityGroup"),
+			infrav1.GroupVersion.WithKind("CloudStackIsolatedNetwork")),
+		r.CheckOwnedObjectsDeleted(
+			infrav1.GroupVersion.WithKind("CloudStackAffinityGroup"),
+			infrav1.GroupVersion.WithKind("CloudStackIsolatedNetwork")),
+		r.RemoveFinalizer,
+	)
+}
+
+// ClearMachines checks for any machines in failure domain, deletes the CAPI machine for any still in FailureDomain,
+// and requeus until all CloudStack machines are cleared from the FailureDomain.
+func (r *CloudStackFailureDomainReconciliationRunner) ClearMachines() (ctrl.Result, error) {
+	machines := &infrav1.CloudStackMachineList{}
+	if err := r.K8sClient.List(r.RequestCtx, machines, client.MatchingLabels{infrav1.FailureDomainLabelName: r.ReconciliationSubject.Name}); err != nil {
+		return ctrl.Result{}, err
+	}
+	// Deleted CAPI machines for CloudStack machines found.
+	for _, machine := range machines.Items {
+		for _, ref := range machine.OwnerReferences {
+			if ref.Kind == "Machine" {
+				machine := &clusterv1.Machine{}
+				machine.Name = ref.Name
+				machine.Namespace = r.ReconciliationSubject.Namespace
+				if err := r.K8sClient.Delete(r.RequestCtx, machine); err != nil {
+					return ctrl.Result{}, err
+				}
 			}
-			return r.RequeueWithMessage("Child IsolatedNetwork still present, requeueing.")
 		}
 	}
+	if len(machines.Items) > 0 {
+		return r.RequeueWithMessage("FailureDomain still has machine in it.")
+	}
+	return ctrl.Result{}, nil
+}
+
+// RemoveFinalizer just removes the finalizer from the failure domain.
+func (r *CloudStackFailureDomainReconciliationRunner) RemoveFinalizer() (ctrl.Result, error) {
 	controllerutil.RemoveFinalizer(r.ReconciliationSubject, infrav1.FailureDomainFinalizer)
 	return ctrl.Result{}, nil
 }

controllers/cloudstackmachine_controller.go

@@ -121,20 +121,23 @@ func (r *CloudStackMachineReconciliationRunner) ConsiderAffinity() (ctrl.Result,
 		r.Log.Info("getting affinity group name", err)
 	}
 
+	// Set failure domain name and owners.
 	r.AffinityGroup.Spec.FailureDomainName = r.ReconciliationSubject.Spec.FailureDomainName
-	if res, err := r.GetOrCreateAffinityGroup(agName, r.ReconciliationSubject.Spec.Affinity, r.AffinityGroup)(); r.ShouldReturn(res, err) {
+	if res, err := r.GetOrCreateAffinityGroup(
+		agName, r.ReconciliationSubject.Spec.Affinity, r.AffinityGroup, r.FailureDomain)(); r.ShouldReturn(res, err) {
 		return res, err
 	}
 	if !r.AffinityGroup.Status.Ready {
 		return r.RequeueWithMessage("Required affinity group not ready.")
 	}
+
 	return ctrl.Result{}, nil
 }
 
 // SetFailureDomainOnCSMachine sets the failure domain the machine should launch in.
 func (r *CloudStackMachineReconciliationRunner) SetFailureDomainOnCSMachine() (retRes ctrl.Result, reterr error) {
-	if r.ReconciliationSubject.Spec.FailureDomainName == "" { // Needs random FD, but not yet set.
-		name := ""
+	if r.ReconciliationSubject.Spec.FailureDomainName == "" {
+		var name string
 		if r.CAPIMachine.Spec.FailureDomain != nil &&
 			(util.IsControlPlaneMachine(r.CAPIMachine) || // Is control plane machine -- CAPI will specify.
 				*r.CAPIMachine.Spec.FailureDomain != "") { // Or potentially another machine controller specified.
@@ -148,6 +151,7 @@ func (r *CloudStackMachineReconciliationRunner) SetFailureDomainOnCSMachine() (r
 			name = name + "-" + r.CAPICluster.Name
 		}
 		r.ReconciliationSubject.Spec.FailureDomainName = name
+		r.ReconciliationSubject.Labels[infrav1.FailureDomainLabelName] = r.ReconciliationSubject.Spec.FailureDomainName
 	}
 	return ctrl.Result{}, nil
 }

controllers/utils/affinity_group.go

@@ -28,10 +28,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// GenerateIsolatedNetwork of the passed name that's owned by the ReconciliationSubject.
-func (r *ReconciliationRunner) GetOrCreateAffinityGroup(name string, affinityType string, ag *infrav1.CloudStackAffinityGroup) CloudStackReconcilerMethod {
-	return func() (ctrl.Result, error) {
+// GetOrCreateAffinityGroup of the passed name that's owned by the failure domain of the reconciliation subject and
+// the control plane that manages it.
+func (r *ReconciliationRunner) GetOrCreateAffinityGroup(
+	name string,
+	affinityType string,
+	ag *infrav1.CloudStackAffinityGroup,
+	fd *infrav1.CloudStackFailureDomain) CloudStackReconcilerMethod {
 
+	return func() (ctrl.Result, error) {
 		// Start by attempting a fetch.
 		lowerName := strings.ToLower(name)
 		namespace := r.ReconciliationSubject.GetNamespace()
@@ -56,7 +61,7 @@ func (r *ReconciliationRunner) GetOrCreateAffinityGroup(name string, affinityTyp
 		ag.Spec.Name = name
 		ag.ObjectMeta = r.NewChildObjectMeta(lowerName)
 
-		// Replace owner reference with controller of CAPI and CloudStack machines.
+		// Replace owner reference with controller of CAPI and CloudStack machines and FailureDomain.
 		for _, ref := range r.ReconciliationSubject.GetOwnerReferences() {
 			if strings.EqualFold(ref.Kind, "EtcdadmCluster") ||
 				strings.EqualFold(ref.Kind, "KubeadmControlPlane") ||
@@ -65,6 +70,13 @@ func (r *ReconciliationRunner) GetOrCreateAffinityGroup(name string, affinityTyp
 				break
 			}
 		}
+		ag.OwnerReferences = append(ag.OwnerReferences,
+			metav1.OwnerReference{
+				Name:       fd.Name,
+				Kind:       fd.Kind,
+				APIVersion: fd.APIVersion,
+				UID:        fd.UID,
+			})
 
 		if err := r.K8sClient.Create(r.RequestCtx, ag); err != nil && !ContainsAlreadyExistsSubstring(err) {
 			return r.ReturnWrappedError(err, "creating affinity group CRD")

controllers/utils/base_reconciler.go

@@ -232,11 +232,11 @@ func (r *ReconciliationRunner) CheckOwnedCRDsForReadiness(gvks ...schema.GroupVe
 					} else if !found {
 						return r.RequeueWithMessage(
 							fmt.Sprintf(
-								"Owned object of kind %s with name %s not found, requeueing.",
+								"Owned object of kind %s with name %s not found, requeuing.",
 								gvk.Kind,
 								owned.GetName()))
 					} else {
-						r.Log.Info(fmt.Sprintf("Owned object %s of kind %s not ready, requeueing", name, gvk.Kind))
+						r.Log.Info(fmt.Sprintf("Owned object %s of kind %s not ready, requeuing", name, gvk.Kind))
 						return ctrl.Result{RequeueAfter: RequeueTimeout}, nil
 					}
 				}
@@ -246,6 +246,66 @@ func (r *ReconciliationRunner) CheckOwnedCRDsForReadiness(gvks ...schema.GroupVe
 	}
 }
 
+// CheckOwnedObjectsDeleted queries for the presence of owned objects and requeues if any are still present. Primarily
+// used to prevent deletions of owners before dependents.
+func (r *ReconciliationRunner) DeleteOwnedObjects(gvks ...schema.GroupVersionKind) CloudStackReconcilerMethod {
+	return func() (ctrl.Result, error) {
+		// For each GroupVersionKind...
+		for _, gvk := range gvks {
+			// Query to find objects of this kind.
+			potentiallyOnwedObjs := &unstructured.UnstructuredList{}
+			potentiallyOnwedObjs.SetGroupVersionKind(gvk)
+			err := r.K8sClient.List(r.RequestCtx, potentiallyOnwedObjs)
+			if err != nil {
+				return ctrl.Result{}, errors.Wrapf(err, "requesting owned objects with gvk %s", gvk)
+			}
+
+			// Filter objects not actually owned by reconciliation subject via owner reference UID.
+			for _, pOwned := range potentiallyOnwedObjs.Items {
+				_pOwned := pOwned
+				refs := pOwned.GetOwnerReferences()
+				for _, ref := range refs {
+					if ref.UID == r.ReconciliationSubject.GetUID() {
+						if err := r.K8sClient.Delete(r.RequestCtx, &_pOwned); err != nil {
+							return ctrl.Result{}, err
+						}
+					}
+				}
+			}
+		}
+		return ctrl.Result{}, nil
+	}
+}
+
+// CheckOwnedObjectsDeleted queries for the presence of owned objects and requeues if any are still present. Primarily
+// used to prevent deletions of owners before dependents.
+func (r *ReconciliationRunner) CheckOwnedObjectsDeleted(gvks ...schema.GroupVersionKind) CloudStackReconcilerMethod {
+	return func() (ctrl.Result, error) {
+		// For each GroupVersionKind...
+		for _, gvk := range gvks {
+			// Query to find objects of this kind.
+			potentiallyOnwedObjs := &unstructured.UnstructuredList{}
+			potentiallyOnwedObjs.SetGroupVersionKind(gvk)
+			err := r.K8sClient.List(r.RequestCtx, potentiallyOnwedObjs)
+			if err != nil {
+				return ctrl.Result{}, errors.Wrapf(err, "requesting owned objects with gvk %s", gvk)
+			}
+
+			// Filter objects not actually owned by reconciliation subject via owner reference UID.
+			for _, pOwned := range potentiallyOnwedObjs.Items {
+				refs := pOwned.GetOwnerReferences()
+				for _, ref := range refs {
+					if ref.UID == r.ReconciliationSubject.GetUID() {
+						return r.RequeueWithMessage(
+							fmt.Sprintf("Owned object %s of kind %s not yet deleted", pOwned.GetKind(), pOwned.GetName()))
+					}
+				}
+			}
+		}
+		return ctrl.Result{}, nil
+	}
+}
+
 // RequeueIfCloudStackClusterNotReady requeues the reconciliation request if the CloudStackCluster is not ready.
 func (r *ReconciliationRunner) RequeueIfCloudStackClusterNotReady() (ctrl.Result, error) {
 	if !r.CSCluster.Status.Ready {
@@ -272,9 +332,9 @@ func (r *ReconciliationRunner) PatchChangesBackToAPI() (res ctrl.Result, retErr
 
 // RequeueWithMessage is a convenience method to log requeue message and then return a result with RequeueAfter set.
 func (r *ReconciliationRunner) RequeueWithMessage(msg string, keysAndValues ...interface{}) (ctrl.Result, error) {
-	// Add requeueing to message if not present. Might turn this into a lint check later.
-	if !strings.Contains(strings.ToLower(msg), "requeue") {
-		msg = msg + " Requeueing."
+	// Add requeuing to message if not present. Might turn this into a lint check later.
+	if !strings.Contains(strings.ToLower(msg), "requeu") {
+		msg = msg + " Requeuing."
 	}
 	r.Log.Info(msg, keysAndValues...)
 	return ctrl.Result{RequeueAfter: RequeueTimeout}, nil
@@ -284,7 +344,7 @@ func (r *ReconciliationRunner) RequeueWithMessage(msg string, keysAndValues ...i
 func (r *ReconciliationRunner) RequeueWithMessageStage(msg string, keysAndValues ...interface{}) CloudStackReconcilerMethod {
 	return func() (ctrl.Result, error) {
 		if !strings.Contains(strings.ToLower(msg), "requeue") {
-			msg = msg + " Requeueing."
+			msg = msg + " Requeuing."
 		}
 		r.Log.Info(msg, keysAndValues...)
 		return ctrl.Result{RequeueAfter: RequeueTimeout}, nil
@@ -381,6 +441,7 @@ func (r *ReconciliationRunner) GetReconciliationSubject() (res ctrl.Result, rete
 	r.Log.V(1).Info("Getting reconciliation subject.")
 	err := client.IgnoreNotFound(r.K8sClient.Get(r.RequestCtx, r.Request.NamespacedName, r.ReconciliationSubject))
 	if r.ReconciliationSubject.GetName() == "" { // Resource does not exist. No need to reconcile.
+		r.Log.V(1).Info("Resource not found. Exiting reconciliation.")
 		r.SetReturnEarly()
 	}
 	if err != nil {
@@ -428,7 +489,7 @@ func (r *ReconciliationRunner) NewChildObjectMeta(name string) metav1.ObjectMeta
 		Name:      strings.ToLower(name),
 		Namespace: r.Request.Namespace,
 		Labels: map[string]string{clusterv1.ClusterLabelName: r.CAPICluster.Name,
-			infrav1.CloudStackClusterLabelName: r.CSCluster.ClusterName},
+			infrav1.CloudStackClusterLabelName: r.CSCluster.Name},
 		OwnerReferences: []metav1.OwnerReference{
 			*metav1.NewControllerRef(r.ReconciliationSubject, ownerGVK),
 		},
@@ -438,11 +499,11 @@ func (r *ReconciliationRunner) NewChildObjectMeta(name string) metav1.ObjectMeta
 // RequeueIfMissingBaseCRs checks that the ReconciliationSubject, CAPI Cluster, and CloudStackCluster objects were
 // actually fetched and reques if not. The base reconciliation stages will continue even if not so as to allow deletion.
 func (r *ReconciliationRunner) RequeueIfMissingBaseCRs() (ctrl.Result, error) {
-	r.Log.V(1).Info("Requeueing if missing ReconciliationSubject, CloudStack cluster, or CAPI cluster.")
+	r.Log.V(1).Info("Requeuing if missing ReconciliationSubject, CloudStack cluster, or CAPI cluster.")
 	if r.CSCluster.GetName() == "" {
-		return r.RequeueWithMessage("CloudStackCluster wasn't found. Requeueing.")
+		return r.RequeueWithMessage("CloudStackCluster wasn't found. Requeuing.")
 	} else if r.CAPICluster.GetName() == "" {
-		return r.RequeueWithMessage("CAPI Cluster wasn't found. Requeueing.")
+		return r.RequeueWithMessage("CAPI Cluster wasn't found. Requeuing.")
 	}
 	return ctrl.Result{}, nil
 }