Merge pull request #5416 from anmazzotti/use_external_bootstrap_config_reference

Support any bootstrap provider with MachinePools

Author: k8s-ci-robot

config/rbac/role.yaml

@@ -48,8 +48,7 @@ rules:
 - apiGroups:
   - bootstrap.cluster.x-k8s.io
   resources:
-  - kubeadmconfigs
-  - kubeadmconfigs/status
+  - '*'
   verbs:
   - get
   - list

exp/controllers/azuremachinepool_controller.go

@@ -19,19 +19,17 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"time"
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
-	kubeadmv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
+	"sigs.k8s.io/cluster-api/controllers/external"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
@@ -41,7 +39,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
@@ -63,7 +60,7 @@ type (
 		Timeouts                      reconciler.Timeouts
 		WatchFilterValue              string
 		createAzureMachinePoolService azureMachinePoolServiceCreator
-		BootstrapConfigGVK            schema.GroupVersionKind
+		externalTracker               external.ObjectTracker
 		CredentialCache               azure.CredentialCache
 	}
 
@@ -77,21 +74,13 @@ type (
 type azureMachinePoolServiceCreator func(machinePoolScope *scope.MachinePoolScope) (*azureMachinePoolService, error)
 
 // NewAzureMachinePoolReconciler returns a new AzureMachinePoolReconciler instance.
-func NewAzureMachinePoolReconciler(client client.Client, recorder record.EventRecorder, timeouts reconciler.Timeouts, watchFilterValue, bootstrapConfigGVK string, credCache azure.CredentialCache) *AzureMachinePoolReconciler {
-	gvk := schema.FromAPIVersionAndKind(kubeadmv1.GroupVersion.String(), reflect.TypeOf((*kubeadmv1.KubeadmConfig)(nil)).Elem().Name())
-	userGVK, _ := schema.ParseKindArg(bootstrapConfigGVK)
-
-	if userGVK != nil {
-		gvk = *userGVK
-	}
-
+func NewAzureMachinePoolReconciler(client client.Client, recorder record.EventRecorder, timeouts reconciler.Timeouts, watchFilterValue string, credCache azure.CredentialCache) *AzureMachinePoolReconciler {
 	ampr := &AzureMachinePoolReconciler{
-		Client:             client,
-		Recorder:           recorder,
-		Timeouts:           timeouts,
-		WatchFilterValue:   watchFilterValue,
-		BootstrapConfigGVK: gvk,
-		CredentialCache:    credCache,
+		Client:           client,
+		Recorder:         recorder,
+		Timeouts:         timeouts,
+		WatchFilterValue: watchFilterValue,
+		CredentialCache:  credCache,
 	}
 
 	ampr.createAzureMachinePoolService = newAzureMachinePoolService
@@ -127,9 +116,7 @@ func (ampr *AzureMachinePoolReconciler) SetupWithManager(ctx context.Context, mg
 		return errors.Wrap(err, "failed to create mapper for Cluster to AzureMachines")
 	}
 
-	config := &metav1.PartialObjectMetadata{}
-	config.SetGroupVersionKind(ampr.BootstrapConfigGVK)
-	return ctrl.NewControllerManagedBy(mgr).
+	controller, err := ctrl.NewControllerManagedBy(mgr).
 		WithOptions(options.Options).
 		For(&infrav1exp.AzureMachinePool{}).
 		WithEventFilter(predicates.ResourceHasFilterLabel(mgr.GetScheme(), log, ampr.WatchFilterValue)).
@@ -148,12 +135,6 @@ func (ampr *AzureMachinePoolReconciler) SetupWithManager(ctx context.Context, mg
 			&infrav1.AzureManagedControlPlane{},
 			handler.EnqueueRequestsFromMapFunc(azureManagedControlPlaneMapper),
 		).
-		// watch for changes in KubeadmConfig (or any BootstrapConfig) to sync bootstrap token
-		Watches(
-			config,
-			handler.EnqueueRequestsFromMapFunc(BootstrapConfigToInfrastructureMapFunc(ctx, ampr.Client, log)),
-			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
-		).
 		Watches(
 			&infrav1exp.AzureMachinePoolMachine{},
 			handler.EnqueueRequestsFromMapFunc(AzureMachinePoolMachineMapper(mgr.GetScheme(), log)),
@@ -170,13 +151,25 @@ func (ampr *AzureMachinePoolReconciler) SetupWithManager(ctx context.Context, mg
 				infracontroller.ClusterPauseChangeAndInfrastructureReady(mgr.GetScheme(), log),
 				predicates.ResourceHasFilterLabel(mgr.GetScheme(), log, ampr.WatchFilterValue),
 			),
-		).
-		Complete(r)
+		).Build(r)
+	if err != nil {
+		return fmt.Errorf("creating new controller manager: %w", err)
+	}
+
+	predicateLog := ptr.To(ctrl.LoggerFrom(ctx).WithValues("controller", "azuremachinepool"))
+	ampr.externalTracker = external.ObjectTracker{
+		Controller:      controller,
+		Cache:           mgr.GetCache(),
+		Scheme:          mgr.GetScheme(),
+		PredicateLogger: predicateLog,
+	}
+
+	return nil
 }
 
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinepools,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinepools/status,verbs=get;update;patch
-// +kubebuilder:rbac:groups=bootstrap.cluster.x-k8s.io,resources=kubeadmconfigs;kubeadmconfigs/status,verbs=get;list;watch
+// +kubebuilder:rbac:groups=bootstrap.cluster.x-k8s.io,resources=*,verbs=get;list;watch
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinepoolmachines,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinepoolmachines/status,verbs=get
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinepools;machinepools/status,verbs=get;list;watch;update;patch
@@ -295,6 +288,27 @@ func (ampr *AzureMachinePoolReconciler) reconcileNormal(ctx context.Context, mac
 		return reconcile.Result{}, nil
 	}
 
+	// Add a Watch to the referenced Bootstrap Config
+	if machinePoolScope.MachinePool.Spec.Template.Spec.Bootstrap.ConfigRef != nil {
+		ref := machinePoolScope.MachinePool.Spec.Template.Spec.Bootstrap.ConfigRef
+		obj, err := external.Get(ctx, ampr.Client, ref)
+		if err != nil {
+			if apierrors.IsNotFound(errors.Cause(err)) {
+				return reconcile.Result{}, errors.Wrapf(err, "could not find %v %q for MachinePool %q in namespace %q, requeuing while searching for bootstrap ConfigRef",
+					ref.GroupVersionKind(), ref.Name, machinePoolScope.MachinePool.Name, ref.Namespace)
+			}
+			return reconcile.Result{}, err
+		}
+
+		// Ensure we add a watch to the external object, if there isn't one already.
+		if err := ampr.externalTracker.Watch(log, obj,
+			handler.EnqueueRequestsFromMapFunc(BootstrapConfigToInfrastructureMapFunc(ampr.Client, *ampr.externalTracker.PredicateLogger)),
+			predicates.ResourceIsChanged(ampr.Client.Scheme(), *ampr.externalTracker.PredicateLogger)); err != nil {
+			return reconcile.Result{}, errors.Wrapf(err, "could not add a watcher to the object %v %q for MachinePool %q in namespace %q, requeuing",
+				ref.GroupVersionKind(), ref.Name, machinePoolScope.MachinePool.Name, ref.Namespace)
+		}
+	}
+
 	// Make sure bootstrap data is available and populated.
 	if machinePoolScope.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName == nil {
 		log.Info("Bootstrap data secret reference is not yet available")

exp/controllers/azuremachinepool_controller_test.go

@@ -46,7 +46,7 @@ var _ = Describe("AzureMachinePoolReconciler", func() {
 	Context("Reconcile an AzureMachinePool", func() {
 		It("should not error with minimal set up", func() {
 			reconciler := NewAzureMachinePoolReconciler(testEnv, testEnv.GetEventRecorderFor("azuremachinepool-reconciler"),
-				reconciler.Timeouts{}, "", "", testEnv.CredentialCache)
+				reconciler.Timeouts{}, "", testEnv.CredentialCache)
 			By("Calling reconcile")
 			instance := &infrav1exp.AzureMachinePool{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}}
 			result, err := reconciler.Reconcile(context.Background(), ctrl.Request{
@@ -81,7 +81,7 @@ func TestAzureMachinePoolReconcilePaused(t *testing.T) {
 
 	recorder := record.NewFakeRecorder(1)
 
-	reconciler := NewAzureMachinePoolReconciler(c, recorder, reconciler.Timeouts{}, "", "", azure.NewCredentialCache())
+	reconciler := NewAzureMachinePoolReconciler(c, recorder, reconciler.Timeouts{}, "", azure.NewCredentialCache())
 	name := test.RandomName("paused", 10)
 	namespace := "default"
 

exp/controllers/helpers.go

@@ -373,17 +373,29 @@ func MachinePoolMachineHasStateOrVersionChange(logger logr.Logger) predicate.Fun
 }
 
 // BootstrapConfigToInfrastructureMapFunc returns a handler.ToRequestsFunc that watches for <Bootstrap>Config events and returns.
-func BootstrapConfigToInfrastructureMapFunc(_ context.Context, c client.Client, log logr.Logger) handler.MapFunc {
+func BootstrapConfigToInfrastructureMapFunc(c client.Client, log logr.Logger) handler.MapFunc {
 	return func(ctx context.Context, o client.Object) []reconcile.Request {
 		ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultMappingTimeout)
 		defer cancel()
 
+		// Lookup the first MachinePool owner of this BootstrapConfig, if any
+		mpName := ""
+		for _, owner := range o.GetOwnerReferences() {
+			if owner.Kind == "MachinePool" {
+				mpName = owner.Name
+				break
+			}
+		}
+		if mpName == "" {
+			log.V(4).Info("BootstrapConfig has no MachinePool owner")
+			return []reconcile.Request{}
+		}
+
+		// Fetch the MachinePool to validate the Bootstrap.ConfigRef
 		mpKey := client.ObjectKey{
 			Namespace: o.GetNamespace(),
-			Name:      o.GetName(),
+			Name:      mpName,
 		}
-
-		// fetch MachinePool to get reference
 		mp := &expv1.MachinePool{}
 		if err := c.Get(ctx, mpKey, mp); err != nil {
 			if !apierrors.IsNotFound(err) {
@@ -397,7 +409,7 @@ func BootstrapConfigToInfrastructureMapFunc(_ context.Context, c client.Client,
 			log.V(4).Info("fetched MachinePool has no Bootstrap.ConfigRef")
 			return []reconcile.Request{}
 		}
-		sameKind := ref.Kind != o.GetObjectKind().GroupVersionKind().Kind
+		sameKind := ref.Kind == o.GetObjectKind().GroupVersionKind().Kind
 		sameName := ref.Name == o.GetName()
 		sameNamespace := ref.Namespace == o.GetNamespace()
 		if !sameKind || !sameName || !sameNamespace {
@@ -411,12 +423,12 @@ func BootstrapConfigToInfrastructureMapFunc(_ context.Context, c client.Client,
 			return []reconcile.Request{}
 		}
 
-		key := client.ObjectKeyFromObject(o)
-		log.V(4).Info("adding KubeadmConfig to watch", "key", key)
+		// Enqueue a request for an AzureMachinePool with same name of MachinePool
+		log.V(4).Info("Enqueueing AzureMachinePool from Bootstrap Config", "key", client.ObjectKeyFromObject(o), "kind", ref.Kind)
 
 		return []reconcile.Request{
 			{
-				NamespacedName: key,
+				NamespacedName: mpKey,
 			},
 		}
 	}

exp/controllers/helpers_test.go

@@ -21,12 +21,18 @@ import (
 	"testing"
 
 	"github.com/go-logr/logr"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"go.uber.org/mock/gomock"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	kubeadmv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
+	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -41,6 +47,99 @@ var (
 	clusterName = "my-cluster"
 )
 
+var _ = Describe("BootstrapConfigToInfrastructureMapFunc", func() {
+	It("should map bootstrap config to machine pool", func() {
+		ctx := context.Background()
+		scheme := runtime.NewScheme()
+		Expect(kubeadmv1.AddToScheme(scheme)).Should(Succeed())
+		Expect(expv1.AddToScheme(scheme)).Should(Succeed())
+		Expect(clusterv1.AddToScheme(scheme)).Should(Succeed())
+		fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
+		mapFn := BootstrapConfigToInfrastructureMapFunc(fakeClient, ctrl.Log)
+		bootstrapConfig := kubeadmv1.KubeadmConfig{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bootstrap-test",
+				Namespace: "default",
+			},
+		}
+		Expect(fakeClient.Create(ctx, &bootstrapConfig)).Should(Succeed())
+
+		By("doing nothing if the config has no owners")
+		Expect(mapFn(ctx, &bootstrapConfig)).Should(Equal([]ctrl.Request{}))
+
+		By("doing nothing if the config has no MachinePool owner")
+		bootstrapConfig.OwnerReferences = []metav1.OwnerReference{
+			{
+				APIVersion: "cluster.x-k8s.io/v1beta1",
+				Name:       "machine-pool-test",
+				Kind:       "NotAMachinePool",
+				UID:        types.UID("foobar"),
+				Controller: ptr.To(true),
+			},
+		}
+		Expect(fakeClient.Update(ctx, &bootstrapConfig)).Should(Succeed())
+		Expect(mapFn(ctx, &bootstrapConfig)).Should(Equal([]ctrl.Request{}))
+
+		By("doing nothing if the MachinePool is not found")
+		bootstrapConfig.OwnerReferences = []metav1.OwnerReference{
+			{
+				APIVersion: "cluster.x-k8s.io/v1beta1",
+				Name:       "machine-pool-test",
+				Kind:       "MachinePool",
+				UID:        types.UID("foobar"),
+				Controller: ptr.To(true),
+			},
+		}
+		Expect(fakeClient.Update(ctx, &bootstrapConfig)).Should(Succeed())
+		Expect(mapFn(ctx, &bootstrapConfig)).Should(Equal([]ctrl.Request{}))
+
+		By("doing nothing if the MachinePool has no BootstrapConfigRef")
+		machinePool := expv1.MachinePool{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "machine-pool-test",
+				Namespace: "default",
+			},
+			Spec: expv1.MachinePoolSpec{
+				ClusterName: "test-cluster",
+				Template: clusterv1.MachineTemplateSpec{
+					Spec: clusterv1.MachineSpec{
+						ClusterName: "test-cluster",
+					},
+				},
+			},
+		}
+		Expect(fakeClient.Create(ctx, &machinePool)).Should(Succeed())
+		Expect(mapFn(ctx, &bootstrapConfig)).Should(Equal([]ctrl.Request{}))
+
+		By("doing nothing if the MachinePool has a different BootstrapConfigRef Kind")
+		machinePool.Spec.Template.Spec.Bootstrap = clusterv1.Bootstrap{
+			ConfigRef: &corev1.ObjectReference{
+				APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
+				Kind:       "OtherBootstrapConfig",
+				Name:       bootstrapConfig.Name,
+				Namespace:  bootstrapConfig.Namespace,
+			},
+		}
+		Expect(fakeClient.Update(ctx, &machinePool)).Should(Succeed())
+		Expect(mapFn(ctx, &bootstrapConfig)).Should(Equal([]ctrl.Request{}))
+
+		By("doing nothing if the MachinePool has a different BootstrapConfigRef Name")
+		machinePool.Spec.Template.Spec.Bootstrap.ConfigRef.Kind = bootstrapConfig.GetObjectKind().GroupVersionKind().Kind
+		machinePool.Spec.Template.Spec.Bootstrap.ConfigRef.Name = "other-bootstrap-config"
+		Expect(fakeClient.Update(ctx, &machinePool)).Should(Succeed())
+		Expect(mapFn(ctx, &bootstrapConfig)).Should(Equal([]ctrl.Request{}))
+
+		By("enqueueing AzureMachinePool")
+		machinePool.Spec.Template.Spec.Bootstrap.ConfigRef.Name = bootstrapConfig.Name
+		Expect(fakeClient.Update(ctx, &machinePool)).Should(Succeed())
+		Expect(mapFn(ctx, &bootstrapConfig)).Should(Equal([]ctrl.Request{
+			{
+				NamespacedName: client.ObjectKey{Namespace: machinePool.Namespace, Name: machinePool.Name},
+			},
+		}))
+	})
+})
+
 func TestAzureClusterToAzureMachinePoolsMapper(t *testing.T) {
 	g := NewWithT(t)
 	scheme := newScheme(g)

exp/controllers/suite_test.go

@@ -55,7 +55,7 @@ var _ = BeforeSuite(func() {
 	ctx = log.IntoContext(ctx, logr.New(testEnv.Log))
 
 	Expect(NewAzureMachinePoolReconciler(testEnv, testEnv.GetEventRecorderFor("azuremachinepool-reconciler"),
-		reconciler.Timeouts{}, "", "", testEnv.CredentialCache).SetupWithManager(ctx, testEnv.Manager, controllers.Options{Options: controller.Options{MaxConcurrentReconciles: 1}})).To(Succeed())
+		reconciler.Timeouts{}, "", testEnv.CredentialCache).SetupWithManager(ctx, testEnv.Manager, controllers.Options{Options: controller.Options{MaxConcurrentReconciles: 1}})).To(Succeed())
 
 	Expect(NewAzureMachinePoolMachineController(testEnv, testEnv.GetEventRecorderFor("azuremachinepoolmachine-reconciler"),
 		reconciler.Timeouts{}, "", testEnv.CredentialCache).SetupWithManager(ctx, testEnv.Manager, controllers.Options{Options: controller.Options{MaxConcurrentReconciles: 1}})).To(Succeed())