Merge pull request kubernetes-sigs#286 from vishesh92/fix-affinity-group-naming

Fix affinity group naming

Author: k8s-ci-robot

controllers/cloudstackmachine_controller.go

@@ -19,11 +19,12 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"k8s.io/utils/pointer"
 	"math/rand"
 	"reflect"
 	"regexp"
 
+	"k8s.io/utils/pointer"
+
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -135,18 +136,32 @@ func (r *CloudStackMachineReconciliationRunner) ConsiderAffinity() (ctrl.Result,
 		r.ReconciliationSubject.Spec.Affinity == "" { // No managed affinity.
 		return ctrl.Result{}, nil
 	}
+	var agName string
+	var err error
 
-	agName, err := utils.GenerateAffinityGroupName(*r.ReconciliationSubject, r.CAPIMachine)
-	if err != nil {
-		return ctrl.Result{}, err
+	if r.ReconciliationSubject.Spec.AffinityGroupRef != nil {
+		agName = r.ReconciliationSubject.Spec.AffinityGroupRef.Name
+	} else {
+		agName, err = utils.GenerateAffinityGroupName(*r.ReconciliationSubject, r.CAPIMachine, r.CAPICluster)
+		if err != nil {
+			return ctrl.Result{}, 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.FailureDomain)(); r.ShouldReturn(res, err) {
+	res, err := r.GetOrCreateAffinityGroup(
+		agName, r.ReconciliationSubject.Spec.Affinity, r.AffinityGroup, r.FailureDomain)()
+	if r.ShouldReturn(res, err) {
 		return res, err
 	}
+	// Set affinity group reference.
+	r.ReconciliationSubject.Spec.AffinityGroupRef = &corev1.ObjectReference{
+		Kind:      r.AffinityGroup.Kind,
+		UID:       r.AffinityGroup.UID,
+		Name:      r.AffinityGroup.Name,
+		Namespace: r.AffinityGroup.Namespace,
+	}
 	if !r.AffinityGroup.Status.Ready {
 		return r.RequeueWithMessage("Required affinity group not ready.")
 	}

controllers/utils/affinity_group.go

@@ -18,9 +18,10 @@ package utils
 
 import (
 	"fmt"
+	"strings"
+
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
-	"strings"
 
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -88,12 +89,19 @@ func (r *ReconciliationRunner) GetOrCreateAffinityGroup(
 }
 
 // The computed affinity group name relevant to this machine.
-func GenerateAffinityGroupName(csm infrav1.CloudStackMachine, capiMachine *clusterv1.Machine) (string, error) {
+func GenerateAffinityGroupName(csm infrav1.CloudStackMachine, capiMachine *clusterv1.Machine, capiCluster *clusterv1.Cluster) (string, error) {
 	managerOwnerRef := GetManagementOwnerRef(capiMachine)
 	if managerOwnerRef == nil {
 		return "", errors.Errorf("could not find owner UID for %s/%s", csm.Namespace, csm.Name)
 	}
 	titleCaser := cases.Title(language.English)
-	return fmt.Sprintf("%sAffinity-%s-%s-%s",
-		titleCaser.String(csm.Spec.Affinity), managerOwnerRef.Name, managerOwnerRef.UID, csm.Spec.FailureDomainName), nil
+
+	// If the machine's owner is KubeadmControlPlane or EtcdadmCluster, then we don't consider the name and UID of the
+	// owner, since there will only be one of each of those per cluster.
+	if managerOwnerRef.Kind == "KubeadmControlPlane" || managerOwnerRef.Kind == "EtcdadmCluster" {
+		return fmt.Sprintf("%s-%s-%sAffinity-%s-%s",
+			capiCluster.Name, capiCluster.UID, titleCaser.String(csm.Spec.Affinity), managerOwnerRef.Kind, csm.Spec.FailureDomainName), nil
+	}
+	return fmt.Sprintf("%s-%s-%sAffinity-%s-%s-%s",
+		capiCluster.Name, capiCluster.UID, titleCaser.String(csm.Spec.Affinity), managerOwnerRef.Name, managerOwnerRef.UID, csm.Spec.FailureDomainName), nil
 }

controllers/utils/utils.go

@@ -128,10 +128,10 @@ func fetchOwnerRef(refList []meta.OwnerReference, kind string) *meta.OwnerRefere
 func GetManagementOwnerRef(capiMachine *clusterv1.Machine) *meta.OwnerReference {
 	if util.IsControlPlaneMachine(capiMachine) {
 		return fetchOwnerRef(capiMachine.OwnerReferences, "KubeadmControlPlane")
-	} else if ref := fetchOwnerRef(capiMachine.OwnerReferences, "MachineSet"); ref != nil {
+	} else if ref := fetchOwnerRef(capiMachine.OwnerReferences, "EtcdadmCluster"); ref != nil {
 		return ref
 	}
-	return fetchOwnerRef(capiMachine.OwnerReferences, "EtcdadmCluster")
+	return fetchOwnerRef(capiMachine.OwnerReferences, "MachineSet")
 }
 
 // GetOwnerOfKind returns the Cluster object owning the current resource of passed kind.