diff --git a/bootstrap/eks/controllers/eksconfig_controller.go b/bootstrap/eks/controllers/eksconfig_controller.go index 7f57921651..2d4b0b98bf 100644 --- a/bootstrap/eks/controllers/eksconfig_controller.go +++ b/bootstrap/eks/controllers/eksconfig_controller.go @@ -19,6 +19,7 @@ package controllers import ( "bytes" "context" + "time" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -74,20 +75,23 @@ func (r *EKSConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( log.Error(err, "Failed to get config") return ctrl.Result{}, err } + log = log.WithValues("EKSConfig", config.GetName()) // check owner references and look up owning Machine object configOwner, err := bsutil.GetConfigOwner(ctx, r.Client, config) if apierrors.IsNotFound(err) { // no error here, requeue until we find an owner - return ctrl.Result{}, nil + log.Debug("eksconfig failed to look up owner reference, re-queueing") + return ctrl.Result{RequeueAfter: time.Minute}, nil } if err != nil { - log.Error(err, "Failed to get owner") + log.Error(err, "eksconfig failed to get owner") return ctrl.Result{}, err } if configOwner == nil { // no error, requeue until we find an owner - return ctrl.Result{}, nil + log.Debug("eksconfig has no owner reference set, re-queueing") + return ctrl.Result{RequeueAfter: time.Minute}, nil } log = log.WithValues(configOwner.GetKind(), configOwner.GetName()) @@ -95,12 +99,12 @@ func (r *EKSConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( cluster, err := util.GetClusterByName(ctx, r.Client, configOwner.GetNamespace(), configOwner.ClusterName()) if err != nil { if errors.Is(err, util.ErrNoCluster) { - log.Info("EKSConfig does not belong to a cluster yet, re-queuing until it's partof a cluster") - return ctrl.Result{}, nil + log.Info("EKSConfig does not belong to a cluster yet, re-queuing until it's part of a cluster") + return ctrl.Result{RequeueAfter: time.Minute}, nil } if apierrors.IsNotFound(err) { log.Info("Cluster does not exist yet, re-queueing until it is created") - return ctrl.Result{}, nil + return ctrl.Result{RequeueAfter: time.Minute}, nil } log.Error(err, "Could not get cluster with metadata") return ctrl.Result{}, err @@ -138,13 +142,14 @@ func (r *EKSConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } }() - return r.joinWorker(ctx, cluster, config) + return r.joinWorker(ctx, cluster, config, configOwner) } -func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1.Cluster, config *eksbootstrapv1.EKSConfig) (ctrl.Result, error) { +func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1.Cluster, config *eksbootstrapv1.EKSConfig, configOwner *bsutil.ConfigOwner) (ctrl.Result, error) { log := logger.FromContext(ctx) - if config.Status.DataSecretName != nil { + // only need to reconcile the secret for Machine kinds once, but MachinePools need updates for new launch templates + if config.Status.DataSecretName != nil && configOwner.GetKind() == "Machine" { secretKey := client.ObjectKey{Namespace: config.Namespace, Name: *config.Status.DataSecretName} log = log.WithValues("data-secret-name", secretKey.Name) existingSecret := &corev1.Secret{} @@ -289,7 +294,7 @@ func (r *EKSConfigReconciler) storeBootstrapData(ctx context.Context, cluster *c Namespace: config.Namespace, }, secret); err != nil { if apierrors.IsNotFound(err) { - if err := r.createBootstrapSecret(ctx, cluster, config, data); err != nil { + if secret, err = r.createBootstrapSecret(ctx, cluster, config, data); err != nil { return errors.Wrap(err, "failed to create bootstrap data secret for EKSConfig") } log.Info("created bootstrap data secret for EKSConfig", "secret", klog.KObj(secret)) @@ -382,7 +387,7 @@ func (r *EKSConfigReconciler) ClusterToEKSConfigs(o client.Object) []ctrl.Reques } // Create the Secret containing bootstrap userdata. -func (r *EKSConfigReconciler) createBootstrapSecret(ctx context.Context, cluster *clusterv1.Cluster, config *eksbootstrapv1.EKSConfig, data []byte) error { +func (r *EKSConfigReconciler) createBootstrapSecret(ctx context.Context, cluster *clusterv1.Cluster, config *eksbootstrapv1.EKSConfig, data []byte) (*corev1.Secret, error) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: config.Name, @@ -405,11 +410,14 @@ func (r *EKSConfigReconciler) createBootstrapSecret(ctx context.Context, cluster }, Type: clusterv1.ClusterSecretType, } - return r.Client.Create(ctx, secret) + return secret, r.Client.Create(ctx, secret) } // Update the userdata in the bootstrap Secret. func (r *EKSConfigReconciler) updateBootstrapSecret(ctx context.Context, secret *corev1.Secret, data []byte) (bool, error) { + if secret.Data == nil { + secret.Data = make(map[string][]byte) + } if !bytes.Equal(secret.Data["value"], data) { secret.Data["value"] = data return true, r.Client.Update(ctx, secret) diff --git a/bootstrap/eks/controllers/eksconfig_controller_reconciler_test.go b/bootstrap/eks/controllers/eksconfig_controller_reconciler_test.go index c51b17dd6a..213c52dc90 100644 --- a/bootstrap/eks/controllers/eksconfig_controller_reconciler_test.go +++ b/bootstrap/eks/controllers/eksconfig_controller_reconciler_test.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/cluster-api-provider-aws/v2/bootstrap/eks/internal/userdata" ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/exp/api/v1beta1" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" ) @@ -55,7 +56,7 @@ func TestEKSConfigReconciler(t *testing.T) { } t.Logf(fmt.Sprintf("Calling reconcile on cluster '%s' and config '%s' should requeue", cluster.Name, config.Name)) g.Eventually(func(gomega Gomega) { - result, err := reconciler.joinWorker(ctx, cluster, config) + result, err := reconciler.joinWorker(ctx, cluster, config, configOwner("Machine")) gomega.Expect(err).NotTo(HaveOccurred()) gomega.Expect(result.Requeue).To(BeFalse()) }).Should(Succeed()) @@ -74,16 +75,26 @@ func TestEKSConfigReconciler(t *testing.T) { g.Expect(string(secret.Data["value"])).To(Equal(string(expectedUserData))) }) - t.Run("Should reconcile an EKSConfig and update data Secret", func(t *testing.T) { g := NewWithT(t) amcp := newAMCP("test-cluster") cluster := newCluster(amcp.Name) - machine := newMachine(cluster, "test-machine") - config := newEKSConfig(machine) + mp := newMachinePool(cluster, "test-machine") + config := newEKSConfig(nil) + config.ObjectMeta.Name = mp.Name + config.ObjectMeta.UID = types.UID(fmt.Sprintf("%s uid", mp.Name)) + config.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ + { + Kind: "MachinePool", + APIVersion: v1beta1.GroupVersion.String(), + Name: mp.Name, + UID: types.UID(fmt.Sprintf("%s uid", mp.Name)), + }, + } + config.Status.DataSecretName = &mp.Name t.Logf(dump("amcp", amcp)) t.Logf(dump("config", config)) - t.Logf(dump("machine", machine)) + t.Logf(dump("machinepool", mp)) t.Logf(dump("cluster", cluster)) oldUserData, err := newUserData(cluster.Name, map[string]string{"test-arg": "test-value"}) g.Expect(err).To(BeNil()) @@ -100,7 +111,7 @@ func TestEKSConfigReconciler(t *testing.T) { } t.Logf(fmt.Sprintf("Calling reconcile on cluster '%s' and config '%s' should requeue", cluster.Name, config.Name)) g.Eventually(func(gomega Gomega) { - result, err := reconciler.joinWorker(ctx, cluster, config) + result, err := reconciler.joinWorker(ctx, cluster, config, configOwner("MachinePool")) gomega.Expect(err).NotTo(HaveOccurred()) gomega.Expect(result.Requeue).To(BeFalse()) }).Should(Succeed()) @@ -125,7 +136,7 @@ func TestEKSConfigReconciler(t *testing.T) { } t.Logf(dump("config", config)) g.Eventually(func(gomega Gomega) { - result, err := reconciler.joinWorker(ctx, cluster, config) + result, err := reconciler.joinWorker(ctx, cluster, config, configOwner("MachinePool")) gomega.Expect(err).NotTo(HaveOccurred()) gomega.Expect(result.Requeue).To(BeFalse()) }).Should(Succeed()) @@ -141,6 +152,57 @@ func TestEKSConfigReconciler(t *testing.T) { gomega.Expect(string(secret.Data["value"])).To(Equal(string(expectedUserData))) }).Should(Succeed()) }) + + t.Run("Should reconcile an EKSConfig and not update data if secret exists and config owner is Machine kind", func(t *testing.T) { + g := NewWithT(t) + amcp := newAMCP("test-cluster") + cluster := newCluster(amcp.Name) + machine := newMachine(cluster, "test-machine") + config := newEKSConfig(machine) + t.Logf(dump("amcp", amcp)) + t.Logf(dump("config", config)) + t.Logf(dump("machine", machine)) + t.Logf(dump("cluster", cluster)) + expectedUserData, err := newUserData(cluster.Name, map[string]string{"test-arg": "test-value"}) + g.Expect(err).To(BeNil()) + g.Expect(testEnv.Client.Create(ctx, amcp)).To(Succeed()) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: machine.Name, + }, + } + g.Expect(testEnv.Client.Create(ctx, secret)).To(Succeed()) + + amcpList := &ekscontrolplanev1.AWSManagedControlPlaneList{} + testEnv.Client.List(ctx, amcpList) + t.Logf(dump("stored-amcps", amcpList)) + + reconciler := EKSConfigReconciler{ + Client: testEnv.Client, + } + t.Logf(fmt.Sprintf("Calling reconcile on cluster '%s' and config '%s' should requeue", cluster.Name, config.Name)) + g.Eventually(func(gomega Gomega) { + result, err := reconciler.joinWorker(ctx, cluster, config, configOwner("Machine")) + gomega.Expect(err).NotTo(HaveOccurred()) + gomega.Expect(result.Requeue).To(BeFalse()) + }).Should(Succeed()) + + t.Logf(fmt.Sprintf("Secret '%s' should exist and be out of date", config.Name)) + secretList := &corev1.SecretList{} + testEnv.Client.List(ctx, secretList) + t.Logf(dump("secrets", secretList)) + + secret = &corev1.Secret{} + g.Eventually(func(gomega Gomega) { + gomega.Expect(testEnv.Client.Get(ctx, client.ObjectKey{ + Name: config.Name, + Namespace: "default", + }, secret)).To(Succeed()) + gomega.Expect(string(secret.Data["value"])).To(Not(Equal(string(expectedUserData)))) + }).Should(Succeed()) + }) } // newCluster return a CAPI cluster object. @@ -204,6 +266,40 @@ func newMachine(cluster *clusterv1.Cluster, name string) *clusterv1.Machine { return machine } +// newMachinePool returns a CAPI machine object; if cluster is not nil, the MachinePool is linked to the cluster as well. +func newMachinePool(cluster *clusterv1.Cluster, name string) *v1beta1.MachinePool { + generatedName := fmt.Sprintf("%s-%s", name, util.RandomString(5)) + mp := &v1beta1.MachinePool{ + TypeMeta: metav1.TypeMeta{ + Kind: "MachinePool", + APIVersion: v1beta1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: generatedName, + }, + Spec: v1beta1.MachinePoolSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + ConfigRef: &corev1.ObjectReference{ + Kind: "EKSConfig", + APIVersion: eksbootstrapv1.GroupVersion.String(), + }, + }, + }, + }, + }, + } + if cluster != nil { + mp.Spec.ClusterName = cluster.Name + mp.ObjectMeta.Labels = map[string]string{ + clusterv1.ClusterLabelName: cluster.Name, + } + } + return mp +} + // newEKSConfig return an EKSConfig object; if machine is not nil, the EKSConfig is linked to the machine as well. func newEKSConfig(machine *clusterv1.Machine) *eksbootstrapv1.EKSConfig { config := &eksbootstrapv1.EKSConfig{ @@ -219,6 +315,7 @@ func newEKSConfig(machine *clusterv1.Machine) *eksbootstrapv1.EKSConfig { "test-arg": "test-value", }, }, + Status: eksbootstrapv1.EKSConfigStatus{}, } if machine != nil { config.ObjectMeta.Name = machine.Name @@ -231,6 +328,7 @@ func newEKSConfig(machine *clusterv1.Machine) *eksbootstrapv1.EKSConfig { UID: types.UID(fmt.Sprintf("%s uid", machine.Name)), }, } + config.Status.DataSecretName = &machine.Name machine.Spec.Bootstrap.ConfigRef.Name = config.Name machine.Spec.Bootstrap.ConfigRef.Namespace = config.Namespace } diff --git a/bootstrap/eks/controllers/eksconfig_controller_test.go b/bootstrap/eks/controllers/eksconfig_controller_test.go index dfbb0a7b53..ebfd820dcd 100644 --- a/bootstrap/eks/controllers/eksconfig_controller_test.go +++ b/bootstrap/eks/controllers/eksconfig_controller_test.go @@ -21,9 +21,11 @@ import ( "testing" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/reconcile" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + bsutil "sigs.k8s.io/cluster-api/bootstrap/util" ) func TestEKSConfigReconcilerReturnEarlyIfClusterInfraNotReady(t *testing.T) { @@ -42,7 +44,7 @@ func TestEKSConfigReconcilerReturnEarlyIfClusterInfraNotReady(t *testing.T) { } g.Eventually(func(gomega Gomega) { - result, err := reconciler.joinWorker(context.Background(), cluster, config) + result, err := reconciler.joinWorker(context.Background(), cluster, config, configOwner("Machine")) gomega.Expect(result).To(Equal(reconcile.Result{})) gomega.Expect(err).NotTo(HaveOccurred()) }).Should(Succeed()) @@ -64,8 +66,16 @@ func TestEKSConfigReconcilerReturnEarlyIfClusterControlPlaneNotInitialized(t *te } g.Eventually(func(gomega Gomega) { - result, err := reconciler.joinWorker(context.Background(), cluster, config) + result, err := reconciler.joinWorker(context.Background(), cluster, config, configOwner("Machine")) gomega.Expect(result).To(Equal(reconcile.Result{})) gomega.Expect(err).NotTo(HaveOccurred()) }).Should(Succeed()) } + +func configOwner(kind string) *bsutil.ConfigOwner { + unstructuredOwner := unstructured.Unstructured{ + Object: map[string]interface{}{"kind": kind}, + } + configOwner := bsutil.ConfigOwner{Unstructured: &unstructuredOwner} + return &configOwner +}