Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport changes for bootstrap config reference rotation into v1.4.x branch #21

Merged
merged 2 commits into from
Dec 14, 2023
Merged

Backport changes for bootstrap config reference rotation into v1.4.x branch #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ce9d0c
Update MachinePool bootstrap dataSecretName when bootstrap config cha…
Jont828 May 23, 2023
ef948e3
Add MachinePool test for switching bootstrap config to another ready/…
AndiDog Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 37 additions & 33 deletions exp/internal/controllers/machinepool_controller_phases.go
View file
Open in desktop
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
Open in desktop
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
Loading