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

🐛 Update MachinePool bootstrap dataSecretName when bootstrap config changes #8667

Merged
merged 1 commit into from
May 25, 2023
Merged

🐛 Update MachinePool bootstrap dataSecretName when bootstrap config changes #8667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
sbueringer marked this conversation as resolved.
Show resolved Hide resolved
}

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)
Jont828 marked this conversation as resolved.
Show resolved Hide resolved
}

// reconcileInfrastructure reconciles the Spec.InfrastructureRef object on a MachinePool.
Expand Down
50 changes: 45 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 @@ -605,7 +605,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 +643,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 +722,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