Skip to content

Commit

Permalink
Merge pull request #21 from giantswarm/machine-pool-bootstrap-config-…
Browse files Browse the repository at this point in the history 
…ref-rotation-based-on-1.4

Backport changes for bootstrap config reference rotation into v1.4.x branch
  • Loading branch information
@AndiDog
AndiDog committed Dec 14, 2023
2 parents 7030a83 + ef948e3 commit 3fce432
Show file tree
Hide file tree
Showing 2 changed files with 249 additions and 38 deletions.
70 changes: 37 additions & 33 deletions exp/internal/controllers/machinepool_controller_phases.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,48 +197,52 @@ func (r *MachinePoolReconciler) reconcileBootstrap(ctx context.Context, cluster
return ctrl.Result{}, nil
}
bootstrapConfig = bootstrapReconcileResult.Result
}

// If the bootstrap data secret is populated, set ready and return.
if m.Spec.Template.Spec.Bootstrap.DataSecretName != nil {
m.Status.BootstrapReady = true
conditions.MarkTrue(m, clusterv1.BootstrapReadyCondition)
return ctrl.Result{}, nil
}
// If the bootstrap config is being deleted, return early.
if !bootstrapConfig.GetDeletionTimestamp().IsZero() {
return ctrl.Result{}, nil
}

// If the bootstrap config is being deleted, return early.
if !bootstrapConfig.GetDeletionTimestamp().IsZero() {
return ctrl.Result{}, nil
}
// Determine if the bootstrap provider is ready.
ready, err := external.IsReady(bootstrapConfig)
if err != nil {
return ctrl.Result{}, err
}

// Determine if the bootstrap provider is ready.
ready, err := external.IsReady(bootstrapConfig)
if err != nil {
return ctrl.Result{}, err
}
// Report a summary of current status of the bootstrap object defined for this machine pool.
conditions.SetMirror(m, clusterv1.BootstrapReadyCondition,
conditions.UnstructuredGetter(bootstrapConfig),
conditions.WithFallbackValue(ready, clusterv1.WaitingForDataSecretFallbackReason, clusterv1.ConditionSeverityInfo, ""),
)

// Report a summary of current status of the bootstrap object defined for this machine pool.
conditions.SetMirror(m, clusterv1.BootstrapReadyCondition,
conditions.UnstructuredGetter(bootstrapConfig),
conditions.WithFallbackValue(ready, clusterv1.WaitingForDataSecretFallbackReason, clusterv1.ConditionSeverityInfo, ""),
)
if !ready {
log.V(2).Info("Bootstrap provider is not ready, requeuing")
m.Status.BootstrapReady = ready
return ctrl.Result{RequeueAfter: externalReadyWait}, nil
}

if !ready {
log.V(2).Info("Bootstrap provider is not ready, requeuing")
return ctrl.Result{RequeueAfter: externalReadyWait}, nil
// Get and set the name of the secret containing the bootstrap data.
secretName, _, err := unstructured.NestedString(bootstrapConfig.Object, "status", "dataSecretName")
if err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to retrieve dataSecretName from bootstrap provider for MachinePool %q in namespace %q", m.Name, m.Namespace)
} else if secretName == "" {
return ctrl.Result{}, errors.Errorf("retrieved empty dataSecretName from bootstrap provider for MachinePool %q in namespace %q", m.Name, m.Namespace)
}

m.Spec.Template.Spec.Bootstrap.DataSecretName = pointer.String(secretName)
m.Status.BootstrapReady = true
return ctrl.Result{}, nil
}

// Get and set the name of the secret containing the bootstrap data.
secretName, _, err := unstructured.NestedString(bootstrapConfig.Object, "status", "dataSecretName")
if err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to retrieve dataSecretName from bootstrap provider for MachinePool %q in namespace %q", m.Name, m.Namespace)
} else if secretName == "" {
return ctrl.Result{}, errors.Errorf("retrieved empty dataSecretName from bootstrap provider for MachinePool %q in namespace %q", m.Name, m.Namespace)
// If dataSecretName is set without a ConfigRef, this means the user brought their own bootstrap data.
if m.Spec.Template.Spec.Bootstrap.DataSecretName != nil {
m.Status.BootstrapReady = true
conditions.MarkTrue(m, clusterv1.BootstrapReadyCondition)
return ctrl.Result{}, nil
}

m.Spec.Template.Spec.Bootstrap.DataSecretName = pointer.String(secretName)
m.Status.BootstrapReady = true
return ctrl.Result{}, nil
// This should never happen because the MachinePool webhook would not allow neither ConfigRef nor DataSecretName to be set.
return ctrl.Result{}, errors.Errorf("neither .spec.bootstrap.configRef nor .spec.bootstrap.dataSecretName are set for MachinePool %q in namespace %q", m.Name, m.Namespace)
}

// reconcileInfrastructure reconciles the Spec.InfrastructureRef object on a MachinePool.
Expand Down
217 changes: 212 additions & 5 deletions exp/internal/controllers/machinepool_controller_phases_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,173 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
r.reconcilePhase(machinepool)
g.Expect(machinepool.Status.GetTypedPhase()).To(Equal(expv1.MachinePoolPhaseDeleting))
})

t.Run("Should keep `Running` when MachinePool bootstrap config is changed to another ready one", func(t *testing.T) {
g := NewWithT(t)

defaultKubeconfigSecret = kubeconfig.GenerateSecret(defaultCluster, kubeconfig.FromEnvTestConfig(env.Config, defaultCluster))
machinePool := defaultMachinePool.DeepCopy()
bootstrapConfig := defaultBootstrap.DeepCopy()
infraConfig := defaultInfra.DeepCopy()

// Set bootstrap ready.
err := unstructured.SetNestedField(bootstrapConfig.Object, true, "status", "ready")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(bootstrapConfig.Object, "secret-data", "status", "dataSecretName")
g.Expect(err).ToNot(HaveOccurred())

// Set infra ready.
err = unstructured.SetNestedStringSlice(infraConfig.Object, []string{"test://id-1"}, "spec", "providerIDList")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(infraConfig.Object, true, "status", "ready")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(infraConfig.Object, []interface{}{
map[string]interface{}{
"type": "InternalIP",
"address": "10.0.0.1",
},
map[string]interface{}{
"type": "InternalIP",
"address": "10.0.0.2",
},
}, "addresses")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(infraConfig.Object, int64(1), "status", "replicas")
g.Expect(err).ToNot(HaveOccurred())

// Set NodeRef.
machinePool.Status.NodeRefs = []corev1.ObjectReference{{Kind: "Node", Name: "machinepool-test-node"}}

// Set replicas to fully reconciled
machinePool.Spec.ProviderIDList = []string{"test://id-1"}
machinePool.Status.ReadyReplicas = 1
machinePool.Status.Replicas = 1

r := &MachinePoolReconciler{
Client: fake.NewClientBuilder().WithObjects(defaultCluster, defaultKubeconfigSecret, machinePool, bootstrapConfig, infraConfig, builder.TestBootstrapConfigCRD, builder.TestInfrastructureMachineTemplateCRD).Build(),
}

res, err := r.reconcile(ctx, defaultCluster, machinePool)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(res.Requeue).To(BeFalse())

r.reconcilePhase(machinePool)
g.Expect(machinePool.Status.GetTypedPhase()).To(Equal(expv1.MachinePoolPhaseRunning))
g.Expect(*machinePool.Spec.Template.Spec.Bootstrap.DataSecretName).To(Equal("secret-data"))

// Change bootstrap reference.
newBootstrapConfig := defaultBootstrap.DeepCopy()
newBootstrapConfig.SetName("bootstrap-config2")
err = unstructured.SetNestedField(newBootstrapConfig.Object, true, "status", "ready")
g.Expect(err).ToNot(HaveOccurred())
err = unstructured.SetNestedField(newBootstrapConfig.Object, "secret-data-new", "status", "dataSecretName")
g.Expect(err).ToNot(HaveOccurred())
err = r.Client.Create(ctx, newBootstrapConfig)
g.Expect(err).ToNot(HaveOccurred())
machinePool.Spec.Template.Spec.Bootstrap.ConfigRef.Name = newBootstrapConfig.GetName()

// Reconcile again. The new bootstrap config should be used.
res, err = r.reconcile(ctx, defaultCluster, machinePool)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(res.Requeue).To(BeFalse())

r.reconcilePhase(machinePool)
g.Expect(*machinePool.Spec.Template.Spec.Bootstrap.DataSecretName).To(Equal("secret-data-new"))
g.Expect(machinePool.Status.BootstrapReady).To(BeTrue())
g.Expect(machinePool.Status.GetTypedPhase()).To(Equal(expv1.MachinePoolPhaseRunning))
})

t.Run("Should keep `Running` when MachinePool bootstrap config is changed to a non-ready one", func(t *testing.T) {
g := NewWithT(t)

defaultKubeconfigSecret = kubeconfig.GenerateSecret(defaultCluster, kubeconfig.FromEnvTestConfig(env.Config, defaultCluster))
machinePool := defaultMachinePool.DeepCopy()
bootstrapConfig := defaultBootstrap.DeepCopy()
infraConfig := defaultInfra.DeepCopy()

// Set bootstrap ready
err := unstructured.SetNestedField(bootstrapConfig.Object, true, "status", "ready")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(bootstrapConfig.Object, "secret-data", "status", "dataSecretName")
g.Expect(err).ToNot(HaveOccurred())

// Set infra ready
err = unstructured.SetNestedStringSlice(infraConfig.Object, []string{"test://id-1"}, "spec", "providerIDList")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(infraConfig.Object, true, "status", "ready")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(infraConfig.Object, []interface{}{
map[string]interface{}{
"type": "InternalIP",
"address": "10.0.0.1",
},
map[string]interface{}{
"type": "InternalIP",
"address": "10.0.0.2",
},
}, "addresses")
g.Expect(err).ToNot(HaveOccurred())

err = unstructured.SetNestedField(infraConfig.Object, int64(1), "status", "replicas")
g.Expect(err).ToNot(HaveOccurred())

// Set NodeRef
machinePool.Status.NodeRefs = []corev1.ObjectReference{{Kind: "Node", Name: "machinepool-test-node"}}

// Set replicas to fully reconciled
machinePool.Spec.ProviderIDList = []string{"test://id-1"}
machinePool.Status.ReadyReplicas = 1
machinePool.Status.Replicas = 1

r := &MachinePoolReconciler{
Client: fake.NewClientBuilder().WithObjects(defaultCluster, defaultKubeconfigSecret, machinePool, bootstrapConfig, infraConfig, builder.TestBootstrapConfigCRD, builder.TestInfrastructureMachineTemplateCRD).Build(),
}

res, err := r.reconcile(ctx, defaultCluster, machinePool)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(res.Requeue).To(BeFalse())

r.reconcilePhase(machinePool)
g.Expect(machinePool.Status.GetTypedPhase()).To(Equal(expv1.MachinePoolPhaseRunning))
g.Expect(*machinePool.Spec.Template.Spec.Bootstrap.DataSecretName).To(Equal("secret-data"))

// Change bootstrap reference
newBootstrapConfig := defaultBootstrap.DeepCopy()
newBootstrapConfig.SetName("bootstrap-config2")
err = unstructured.SetNestedField(newBootstrapConfig.Object, false, "status", "ready")
g.Expect(err).ToNot(HaveOccurred())
// Fill the `dataSecretName` so we can check if the machine pool uses the non-ready secret immediately or,
// as it should, not yet
err = unstructured.SetNestedField(newBootstrapConfig.Object, "secret-data-new", "status", "dataSecretName")
g.Expect(err).ToNot(HaveOccurred())
err = r.Client.Create(ctx, newBootstrapConfig)
g.Expect(err).ToNot(HaveOccurred())
machinePool.Spec.Template.Spec.Bootstrap.ConfigRef.Name = newBootstrapConfig.GetName()

// Reconcile again. The new bootstrap config should be used
res, err = r.reconcile(ctx, defaultCluster, machinePool)
g.Expect(err).ToNot(HaveOccurred())

// Controller should wait until bootstrap provider reports ready bootstrap config
g.Expect(res.Requeue).To(BeFalse())

r.reconcilePhase(machinePool)

// The old secret should still be used, as the new bootstrap config is not marked ready
g.Expect(*machinePool.Spec.Template.Spec.Bootstrap.DataSecretName).To(Equal("secret-data"))
g.Expect(machinePool.Status.BootstrapReady).To(BeFalse())

// There is no phase defined for "changing to new bootstrap config", so it should still be `Running` the
// old configuration
g.Expect(machinePool.Status.GetTypedPhase()).To(Equal(expv1.MachinePoolPhaseRunning))
})
}

func TestReconcileMachinePoolBootstrap(t *testing.T) {
Expand Down Expand Up @@ -605,7 +772,7 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
expectError: true,
},
{
name: "existing machinepool, bootstrap data should not change",
name: "existing machinepool with config ref, update data secret name",
bootstrapConfig: map[string]interface{}{
"kind": builder.TestBootstrapConfigKind,
"apiVersion": builder.BootstrapGroupVersion.String(),
Expand Down Expand Up @@ -643,13 +810,52 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
},
},
expectError: false,
expected: func(g *WithT, m *expv1.MachinePool) {
g.Expect(m.Status.BootstrapReady).To(BeTrue())
g.Expect(*m.Spec.Template.Spec.Bootstrap.DataSecretName).To(Equal("secret-data"))
},
},
{
name: "existing machinepool without config ref, do not update data secret name",
bootstrapConfig: map[string]interface{}{
"kind": builder.TestBootstrapConfigKind,
"apiVersion": builder.BootstrapGroupVersion.String(),
"metadata": map[string]interface{}{
"name": "bootstrap-config1",
"namespace": metav1.NamespaceDefault,
},
"spec": map[string]interface{}{},
"status": map[string]interface{}{
"ready": true,
"dataSecretName": "secret-data",
},
},
machinepool: &expv1.MachinePool{
ObjectMeta: metav1.ObjectMeta{
Name: "bootstrap-test-existing",
Namespace: metav1.NamespaceDefault,
},
Spec: expv1.MachinePoolSpec{
Template: clusterv1.MachineTemplateSpec{
Spec: clusterv1.MachineSpec{
Bootstrap: clusterv1.Bootstrap{
DataSecretName: pointer.String("data"),
},
},
},
},
Status: expv1.MachinePoolStatus{
BootstrapReady: true,
},
},
expectError: false,
expected: func(g *WithT, m *expv1.MachinePool) {
g.Expect(m.Status.BootstrapReady).To(BeTrue())
g.Expect(*m.Spec.Template.Spec.Bootstrap.DataSecretName).To(Equal("data"))
},
},
{
name: "existing machinepool, bootstrap provider is to not ready",
name: "existing machinepool, bootstrap provider is not ready",
bootstrapConfig: map[string]interface{}{
"kind": builder.TestBootstrapConfigKind,
"apiVersion": builder.BootstrapGroupVersion.String(),
Expand Down Expand Up @@ -683,12 +889,13 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
},
},
Status: expv1.MachinePoolStatus{
BootstrapReady: true,
BootstrapReady: false,
},
},
expectError: false,
expectError: false,
expectResult: ctrl.Result{RequeueAfter: externalReadyWait},
expected: func(g *WithT, m *expv1.MachinePool) {
g.Expect(m.Status.BootstrapReady).To(BeTrue())
g.Expect(m.Status.BootstrapReady).To(BeFalse())
},
},
}
Expand Down

0 comments on commit 3fce432

Please sign in to comment.