Skip to content

Commit

Permalink
Merge pull request #11211 from goushicui/cleanbootstrap
Browse files Browse the repository at this point in the history 
🌱 MachineSet controller: delete Bootstrap object when creating InfraMachine object failed
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jan 20, 2025
2 parents c1ec7aa + 05b8f5b commit 02e03de
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 2 deletions.
11 changes: 9 additions & 2 deletions internal/controllers/machineset/machineset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -768,10 +768,17 @@ func (r *Reconciler) syncReplicas(ctx context.Context, s *scope) (ctrl.Result, e
},
})
if err != nil {
var deleteErr error
if bootstrapRef != nil {
// Cleanup the bootstrap resource if we can't create the InfraMachine; or we might risk to leak it.
if err := r.Client.Delete(ctx, util.ObjectReferenceToUnstructured(*bootstrapRef)); err != nil && !apierrors.IsNotFound(err) {
deleteErr = errors.Wrapf(err, "failed to cleanup %s %s after %s creation failed", bootstrapRef.Kind, klog.KRef(bootstrapRef.Namespace, bootstrapRef.Name), (&ms.Spec.Template.Spec.InfrastructureRef).Kind)
}
}
conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.InfrastructureTemplateCloningFailedReason, clusterv1.ConditionSeverityError, err.Error())
return ctrl.Result{}, errors.Wrapf(err, "failed to clone infrastructure machine from %s %s while creating a machine",
return ctrl.Result{}, kerrors.NewAggregate([]error{errors.Wrapf(err, "failed to clone infrastructure machine from %s %s while creating a machine",
ms.Spec.Template.Spec.InfrastructureRef.Kind,
klog.KRef(ms.Spec.Template.Spec.InfrastructureRef.Namespace, ms.Spec.Template.Spec.InfrastructureRef.Name))
klog.KRef(ms.Spec.Template.Spec.InfrastructureRef.Namespace, ms.Spec.Template.Spec.InfrastructureRef.Name)), deleteErr})
}
log = log.WithValues(infraRef.Kind, klog.KRef(infraRef.Namespace, infraRef.Name))
machine.Spec.InfrastructureRef = *infraRef
Expand Down
155 changes: 155 additions & 0 deletions internal/controllers/machineset/machineset_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package machineset

import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand All @@ -26,11 +28,13 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/client-go/tools/record"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

Expand Down Expand Up @@ -2346,6 +2350,157 @@ func TestMachineSetReconciler_syncReplicas(t *testing.T) {
})
}

func TestMachineSetReconciler_syncReplicas_WithErrors(t *testing.T) {
t.Run("should hold off on sync replicas when create Infrastructure of machine failed ", func(t *testing.T) {
g := NewWithT(t)
fakeClient := fake.NewClientBuilder().WithObjects().WithInterceptorFuncs(interceptor.Funcs{
Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
// simulate scenarios where infra object creation fails
if obj.GetObjectKind().GroupVersionKind().Kind == "GenericInfrastructureMachine" {
return fmt.Errorf("inject error for create machineInfrastructure")
}
return client.Create(ctx, obj, opts...)
},
}).Build()

r := &Reconciler{
Client: fakeClient,
}
testCluster := &clusterv1.Cluster{}
testCluster.Namespace = "default"
testCluster.Name = "test-cluster"
version := "v1.14.2"
duration10m := &metav1.Duration{Duration: 10 * time.Minute}
machineSet := &clusterv1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Name: "machineset1",
Namespace: metav1.NamespaceDefault,
Finalizers: []string{"block-deletion"},
},
Spec: clusterv1.MachineSetSpec{
Replicas: ptr.To[int32](1),
ClusterName: "test-cluster",
Template: clusterv1.MachineTemplateSpec{
Spec: clusterv1.MachineSpec{
ClusterName: testCluster.Name,
Version: &version,
Bootstrap: clusterv1.Bootstrap{
ConfigRef: &corev1.ObjectReference{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "GenericBootstrapConfigTemplate",
Name: "ms-template",
Namespace: metav1.NamespaceDefault,
},
},
InfrastructureRef: corev1.ObjectReference{
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
Kind: "GenericInfrastructureMachineTemplate",
Name: "ms-template",
Namespace: metav1.NamespaceDefault,
},
NodeDrainTimeout: duration10m,
NodeDeletionTimeout: duration10m,
NodeVolumeDetachTimeout: duration10m,
},
},
},
}

// Create bootstrap template resource.
bootstrapResource := map[string]interface{}{
"kind": "GenericBootstrapConfig",
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"precedence": "GenericBootstrapConfig",
},
},
}
bootstrapTmpl := &unstructured.Unstructured{
Object: map[string]interface{}{
"spec": map[string]interface{}{
"template": bootstrapResource,
},
},
}
bootstrapTmpl.SetKind("GenericBootstrapConfigTemplate")
bootstrapTmpl.SetAPIVersion("bootstrap.cluster.x-k8s.io/v1beta1")
bootstrapTmpl.SetName("ms-template")
bootstrapTmpl.SetNamespace(metav1.NamespaceDefault)
g.Expect(r.Client.Create(context.TODO(), bootstrapTmpl)).To(Succeed())

// Create infrastructure template resource.
infraResource := map[string]interface{}{
"kind": "GenericInfrastructureMachine",
"apiVersion": "infrastructure.cluster.x-k8s.io/v1beta1",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"precedence": "GenericInfrastructureMachineTemplate",
},
},
"spec": map[string]interface{}{
"size": "3xlarge",
},
}
infraTmpl := &unstructured.Unstructured{
Object: map[string]interface{}{
"spec": map[string]interface{}{
"template": infraResource,
},
},
}
infraTmpl.SetKind("GenericInfrastructureMachineTemplate")
infraTmpl.SetAPIVersion("infrastructure.cluster.x-k8s.io/v1beta1")
infraTmpl.SetName("ms-template")
infraTmpl.SetNamespace(metav1.NamespaceDefault)
g.Expect(r.Client.Create(context.TODO(), infraTmpl)).To(Succeed())

s := &scope{
cluster: testCluster,
machineSet: machineSet,
machines: []*clusterv1.Machine{},
getAndAdoptMachinesForMachineSetSucceeded: true,
}
_, err := r.syncReplicas(ctx, s)
g.Expect(err).To(HaveOccurred())

// Verify the proper condition is set on the MachineSet.
condition := clusterv1.MachinesCreatedCondition
g.Expect(conditions.Has(machineSet, condition)).To(BeTrue(), "MachineSet should have the %s condition set", condition)

machinesCreatedCondition := conditions.Get(machineSet, condition)
g.Expect(machinesCreatedCondition.Status).
To(Equal(corev1.ConditionFalse), "%s condition status should be %s", condition, corev1.ConditionFalse)
g.Expect(machinesCreatedCondition.Reason).
To(Equal(clusterv1.InfrastructureTemplateCloningFailedReason), "%s condition reason should be %s", condition, clusterv1.InfrastructureTemplateCloningFailedReason)

// Verify no new Machines are created.
machineList := &clusterv1.MachineList{}
g.Expect(r.Client.List(ctx, machineList)).To(Succeed())
g.Expect(machineList.Items).To(BeEmpty(), "There should not be any machines")

// Verify no boostrap object created
bootstrapList := &unstructured.UnstructuredList{}
bootstrapList.SetGroupVersionKind(schema.GroupVersionKind{
Group: bootstrapTmpl.GetObjectKind().GroupVersionKind().Group,
Version: bootstrapTmpl.GetObjectKind().GroupVersionKind().Version,
Kind: strings.TrimSuffix(bootstrapTmpl.GetObjectKind().GroupVersionKind().Kind, clusterv1.TemplateSuffix),
})
g.Expect(r.Client.List(ctx, bootstrapList)).To(Succeed())
g.Expect(bootstrapList.Items).To(BeEmpty(), "There should not be any bootstrap object")

// Verify no infra object created
infraList := &unstructured.UnstructuredList{}
infraList.SetGroupVersionKind(schema.GroupVersionKind{
Group: infraTmpl.GetObjectKind().GroupVersionKind().Group,
Version: infraTmpl.GetObjectKind().GroupVersionKind().Version,
Kind: strings.TrimSuffix(infraTmpl.GetObjectKind().GroupVersionKind().Kind, clusterv1.TemplateSuffix),
})
g.Expect(r.Client.List(ctx, infraList)).To(Succeed())
g.Expect(infraList.Items).To(BeEmpty(), "There should not be any infra object")
})
}

func TestComputeDesiredMachine(t *testing.T) {
duration5s := &metav1.Duration{Duration: 5 * time.Second}
duration10s := &metav1.Duration{Duration: 10 * time.Second}
Expand Down

0 comments on commit 02e03de

Please sign in to comment.