diff --git a/api/v1alpha4/zz_generated.conversion.go b/api/v1alpha4/zz_generated.conversion.go index 4565fb0ae651..9a9e3d3dc1ab 100644 --- a/api/v1alpha4/zz_generated.conversion.go +++ b/api/v1alpha4/zz_generated.conversion.go @@ -29,7 +29,6 @@ import ( conversion "k8s.io/apimachinery/pkg/conversion" runtime "k8s.io/apimachinery/pkg/runtime" intstr "k8s.io/apimachinery/pkg/util/intstr" - v1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" errors "sigs.k8s.io/cluster-api/errors" ) diff --git a/api/v1alpha4/zz_generated.deepcopy.go b/api/v1alpha4/zz_generated.deepcopy.go index e5877405611b..125f9356bac1 100644 --- a/api/v1alpha4/zz_generated.deepcopy.go +++ b/api/v1alpha4/zz_generated.deepcopy.go @@ -26,7 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" - "sigs.k8s.io/cluster-api/errors" ) diff --git a/exp/api/v1alpha4/zz_generated.conversion.go b/exp/api/v1alpha4/zz_generated.conversion.go index 7fa12480d7bf..3609aa05f248 100644 --- a/exp/api/v1alpha4/zz_generated.conversion.go +++ b/exp/api/v1alpha4/zz_generated.conversion.go @@ -27,7 +27,6 @@ import ( v1 "k8s.io/api/core/v1" conversion "k8s.io/apimachinery/pkg/conversion" runtime "k8s.io/apimachinery/pkg/runtime" - apiv1alpha4 "sigs.k8s.io/cluster-api/api/v1alpha4" apiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" errors "sigs.k8s.io/cluster-api/errors" diff --git a/exp/api/v1alpha4/zz_generated.deepcopy.go b/exp/api/v1alpha4/zz_generated.deepcopy.go index eaac234057a0..82f93a86a8f0 100644 --- a/exp/api/v1alpha4/zz_generated.deepcopy.go +++ b/exp/api/v1alpha4/zz_generated.deepcopy.go @@ -24,7 +24,6 @@ package v1alpha4 import ( "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" - apiv1alpha4 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/errors" ) diff --git a/internal/controllers/machine/machine_controller_test.go b/internal/controllers/machine/machine_controller_test.go index 3d9f7c38a729..ae494f459ac8 100644 --- a/internal/controllers/machine/machine_controller_test.go +++ b/internal/controllers/machine/machine_controller_test.go @@ -17,6 +17,8 @@ limitations under the License. package machine import ( + "context" + "fmt" "testing" "time" @@ -27,9 +29,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -1856,6 +1860,193 @@ func TestNodeToMachine(t *testing.T) { } } +type fakeClientWithNodeDeletionErr struct { + client.Client +} + +func (fc fakeClientWithNodeDeletionErr) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + gvk, err := apiutil.GVKForObject(obj, fakeScheme) + if err == nil && gvk.Kind == "Node" { + return fmt.Errorf("fake error") + } + return fc.Client.Delete(ctx, obj, opts...) +} + +func TestNodeDeletion(t *testing.T) { + g := NewWithT(t) + + deletionTime := metav1.Now().Add(-1 * time.Second) + + testCluster := clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: metav1.NamespaceDefault, + }, + } + + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: corev1.NodeSpec{ProviderID: "test://id-1"}, + } + + testMachine := clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{ + clusterv1.MachineControlPlaneLabelName: "", + }, + Annotations: map[string]string{ + "machine.cluster.x-k8s.io/exclude-node-draining": "", + }, + Finalizers: []string{clusterv1.MachineFinalizer}, + DeletionTimestamp: &metav1.Time{Time: deletionTime}, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{ + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "GenericInfrastructureMachine", + Name: "infra-config1", + }, + Bootstrap: clusterv1.Bootstrap{DataSecretName: pointer.StringPtr("data")}, + }, + Status: clusterv1.MachineStatus{ + NodeRef: &corev1.ObjectReference{ + Name: "test", + }, + }, + } + + cpmachine1 := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cp1", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{ + clusterv1.ClusterLabelName: "test-cluster", + clusterv1.MachineControlPlaneLabelName: "", + }, + Finalizers: []string{clusterv1.MachineFinalizer}, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{}, + Bootstrap: clusterv1.Bootstrap{DataSecretName: pointer.StringPtr("data")}, + }, + Status: clusterv1.MachineStatus{ + NodeRef: &corev1.ObjectReference{ + Name: "cp1", + }, + }, + } + + testCases := []struct { + name string + deletionTimeout *metav1.Duration + resultErr bool + clusterDeleted bool + expectNodeDeletion bool + createFakeClient func(...client.Object) client.Client + }{ + { + name: "should return no error when deletion is successful", + deletionTimeout: &metav1.Duration{Duration: time.Second}, + resultErr: false, + expectNodeDeletion: true, + createFakeClient: func(initObjs ...client.Object) client.Client { + return fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + }, + }, + { + name: "should return an error when timeout is not expired and node deletion fails", + deletionTimeout: &metav1.Duration{Duration: time.Hour}, + resultErr: true, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + { + name: "should return an error when timeout is infinite and node deletion fails", + deletionTimeout: &metav1.Duration{Duration: 0}, // should lead to infinite timeout + resultErr: true, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + { + name: "should not return an error when timeout is expired and node deletion fails", + deletionTimeout: &metav1.Duration{Duration: time.Millisecond}, + resultErr: false, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + { + name: "should not delete the node or return an error when the cluster is marked for deletion", + deletionTimeout: nil, // should lead to infinite timeout + resultErr: false, + clusterDeleted: true, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + m := testMachine.DeepCopy() + m.Spec.NodeDeletionTimeout = tc.deletionTimeout + + fakeClient := tc.createFakeClient(node, m, cpmachine1) + tracker := remote.NewTestClusterCacheTracker(ctrl.Log, fakeClient, fakeScheme, client.ObjectKeyFromObject(&testCluster)) + + r := &Reconciler{ + Client: fakeClient, + Tracker: tracker, + recorder: record.NewFakeRecorder(10), + nodeDeletionRetryTimeout: 10 * time.Millisecond, + } + + cluster := testCluster.DeepCopy() + if tc.clusterDeleted { + cluster.DeletionTimestamp = &metav1.Time{Time: deletionTime.Add(time.Hour)} + } + + _, err := r.reconcileDelete(context.Background(), cluster, m) + + if tc.resultErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + if tc.expectNodeDeletion { + n := &corev1.Node{} + g.Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(node), n)).NotTo(Succeed()) + } + } + }) + } +} + // adds a condition list to an external object. func addConditionsToExternal(u *unstructured.Unstructured, newConditions clusterv1.Conditions) { existingConditions := clusterv1.Conditions{}