From 3f5332271a6a35090b76501b5e569fe648fada52 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 1 Oct 2024 05:56:41 +0000 Subject: [PATCH] fix tests --- api/v1alpha2/linodemachine_types.go | 2 +- controller/linodemachine_controller.go | 7 +- .../linodemachine_controller_helpers_test.go | 91 +------------------ controller/linodemachine_controller_test.go | 86 +++++++----------- 4 files changed, 40 insertions(+), 146 deletions(-) diff --git a/api/v1alpha2/linodemachine_types.go b/api/v1alpha2/linodemachine_types.go index f904418e..29eee204 100644 --- a/api/v1alpha2/linodemachine_types.go +++ b/api/v1alpha2/linodemachine_types.go @@ -162,8 +162,8 @@ type LinodeMachineStatus struct { Addresses []clusterv1.MachineAddress `json:"addresses,omitempty"` // CloudinitMetadataSupport determines whether to use cloud-init or not. - // +kubebuilder:default=true // +optional + // +kubebuilder:default=true CloudinitMetadataSupport bool `json:"cloudinitMetadataSupport,omitempty"` // InstanceState is the state of the Linode instance for this machine. diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 5a180a3b..9dd9fc4a 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -214,7 +214,7 @@ func (r *LinodeMachineReconciler) reconcile(ctx context.Context, logger logr.Log // Make sure bootstrap data is available and populated. if machineScope.Machine.Spec.Bootstrap.DataSecretName == nil { logger.Info("Bootstrap data secret is not yet available") - conditions.MarkFalse(machineScope.LinodeMachine, ConditionPreflightCreated, WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "") + conditions.MarkFalse(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured, WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "") return ctrl.Result{}, nil } @@ -242,7 +242,7 @@ func (r *LinodeMachineReconciler) reconcileCreate( return ctrl.Result{}, err } - if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured) { + if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured) && machineScope.LinodeMachine.Spec.ProviderID == nil { res, err := r.reconcilePreflightMetadataSupportConfigure(ctx, logger, machineScope) if err != nil || !res.IsZero() { return res, err @@ -293,7 +293,7 @@ func (r *LinodeMachineReconciler) reconcilePreflightMetadataSupportConfigure(ctx logger.Error(err, fmt.Sprintf("Failed to fetch region %s", machineScope.LinodeMachine.Spec.Region)) return retryIfTransient(err) } - regionMetadataSupport := slices.Contains(region.Capabilities, "Metadata") + regionMetadataSupport := slices.Contains(region.Capabilities, linodego.CapabilityMetadata) imageName := reconciler.DefaultMachineControllerLinodeImage if machineScope.LinodeMachine.Spec.Image != "" { imageName = machineScope.LinodeMachine.Spec.Image @@ -304,6 +304,7 @@ func (r *LinodeMachineReconciler) reconcilePreflightMetadataSupportConfigure(ctx return retryIfTransient(err) } imageMetadataSupport := slices.Contains(image.Capabilities, "cloud-init") + machineScope.LinodeMachine.Status.CloudinitMetadataSupport = true if !imageMetadataSupport || !regionMetadataSupport { logger.Info("cloud-init metadata support not available", "imageMetadataSupport", imageMetadataSupport, "regionMetadataSupport", regionMetadataSupport) machineScope.LinodeMachine.Status.CloudinitMetadataSupport = false diff --git a/controller/linodemachine_controller_helpers_test.go b/controller/linodemachine_controller_helpers_test.go index 1a5fc9ef..88c38a55 100644 --- a/controller/linodemachine_controller_helpers_test.go +++ b/controller/linodemachine_controller_helpers_test.go @@ -103,7 +103,7 @@ func TestSetUserData(t *testing.T) { Namespace: "default", }, Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-ord", Image: "linode/ubuntu22.04"}, - Status: infrav1alpha2.LinodeMachineStatus{}, + Status: infrav1alpha2.LinodeMachineStatus{CloudinitMetadataSupport: true}, }}, createConfig: &linodego.InstanceCreateOptions{}, wantConfig: &linodego.InstanceCreateOptions{Metadata: &linodego.InstanceMetadataOptions{ @@ -119,12 +119,6 @@ func TestSetUserData(t *testing.T) { *obj = cred return nil }) - mockClient.EXPECT().GetRegion(gomock.Any(), "us-ord").Return(&linodego.Region{ - Capabilities: []string{"Metadata"}, - }, nil) - mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(&linodego.Image{ - Capabilities: []string{"cloud-init"}, - }, nil) }, }, { @@ -143,7 +137,7 @@ func TestSetUserData(t *testing.T) { Namespace: "default", }, Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-east", Image: "linode/ubuntu22.04", Type: "g6-standard-1"}, - Status: infrav1alpha2.LinodeMachineStatus{}, + Status: infrav1alpha2.LinodeMachineStatus{CloudinitMetadataSupport: false}, }}, createConfig: &linodego.InstanceCreateOptions{}, wantConfig: &linodego.InstanceCreateOptions{StackScriptID: 1234, StackScriptData: map[string]string{ @@ -160,10 +154,6 @@ func TestSetUserData(t *testing.T) { *obj = cred return nil }) - mockClient.EXPECT().GetRegion(gomock.Any(), "us-east").Return(&linodego.Region{ - Capabilities: []string{"Metadata"}, - }, nil) - mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(&linodego.Image{}, nil) mockClient.EXPECT().ListStackscripts(gomock.Any(), &linodego.ListOptions{Filter: "{\"label\":\"CAPL-dev\"}"}).Return([]linodego.Stackscript{{ Label: "CAPI Test 1", ID: 1234, @@ -228,77 +218,6 @@ func TestSetUserData(t *testing.T) { }, expectedError: fmt.Errorf("bootstrap data secret is nil for LinodeMachine default/test-cluster"), }, - { - name: "Error - SetUserData failed to get regions", - machineScope: &scope.MachineScope{Machine: &v1beta1.Machine{ - Spec: v1beta1.MachineSpec{ - ClusterName: "", - Bootstrap: v1beta1.Bootstrap{ - DataSecretName: ptr.To("test-data"), - }, - InfrastructureRef: corev1.ObjectReference{}, - }, - }, LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-ord", Image: "linode/ubuntu22.04"}, - Status: infrav1alpha2.LinodeMachineStatus{}, - }}, - createConfig: &linodego.InstanceCreateOptions{}, - wantConfig: &linodego.InstanceCreateOptions{}, - expects: func(mockClient *mock.MockLinodeClient, kMock *mock.MockK8sClient) { - kMock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { - cred := corev1.Secret{ - Data: map[string][]byte{ - "value": []byte("hello"), - }, - } - *obj = cred - return nil - }) - mockClient.EXPECT().GetRegion(gomock.Any(), "us-ord").Return(nil, fmt.Errorf("cannot find region")) - }, - expectedError: fmt.Errorf("cannot find region"), - }, - { - name: "Error - SetUserData failed to get images", - machineScope: &scope.MachineScope{Machine: &v1beta1.Machine{ - Spec: v1beta1.MachineSpec{ - ClusterName: "", - Bootstrap: v1beta1.Bootstrap{ - DataSecretName: ptr.To("test-data"), - }, - InfrastructureRef: corev1.ObjectReference{}, - }, - }, LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-ord", Image: "linode/ubuntu22.04"}, - Status: infrav1alpha2.LinodeMachineStatus{}, - }}, - createConfig: &linodego.InstanceCreateOptions{}, - wantConfig: &linodego.InstanceCreateOptions{}, - expects: func(mockClient *mock.MockLinodeClient, kMock *mock.MockK8sClient) { - kMock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { - cred := corev1.Secret{ - Data: map[string][]byte{ - "value": []byte("hello"), - }, - } - *obj = cred - return nil - }) - mockClient.EXPECT().GetRegion(gomock.Any(), "us-ord").Return(&linodego.Region{ - Capabilities: []string{"Metadata"}, - }, nil) - mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(nil, fmt.Errorf("cannot find image")) - }, - expectedError: fmt.Errorf("cannot find image"), - }, { name: "Error - SetUserData failed to get stackscripts", machineScope: &scope.MachineScope{Machine: &v1beta1.Machine{ @@ -315,7 +234,7 @@ func TestSetUserData(t *testing.T) { Namespace: "default", }, Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-east", Image: "linode/ubuntu22.04", Type: "g6-standard-1"}, - Status: infrav1alpha2.LinodeMachineStatus{}, + Status: infrav1alpha2.LinodeMachineStatus{CloudinitMetadataSupport: false}, }}, createConfig: &linodego.InstanceCreateOptions{}, wantConfig: &linodego.InstanceCreateOptions{StackScriptID: 1234, StackScriptData: map[string]string{ @@ -332,10 +251,6 @@ func TestSetUserData(t *testing.T) { *obj = cred return nil }) - mockClient.EXPECT().GetRegion(gomock.Any(), "us-east").Return(&linodego.Region{ - Capabilities: []string{"Metadata"}, - }, nil) - mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(&linodego.Image{}, nil) mockClient.EXPECT().ListStackscripts(gomock.Any(), &linodego.ListOptions{Filter: "{\"label\":\"CAPL-dev\"}"}).Return(nil, fmt.Errorf("failed to get stackscripts")) }, expectedError: fmt.Errorf("ensure stackscript: failed to get stackscript with label CAPL-dev: failed to get stackscripts"), diff --git a/controller/linodemachine_controller_test.go b/controller/linodemachine_controller_test.go index 89f242a8..33510844 100644 --- a/controller/linodemachine_controller_test.go +++ b/controller/linodemachine_controller_test.go @@ -203,6 +203,7 @@ var _ = Describe("create", Label("machine", "create"), func() { _, err = reconciler.reconcileCreate(ctx, logger, &mScope) Expect(err).NotTo(HaveOccurred()) + Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightMetadataSupportConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightCreated)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightBootTriggered)).To(BeTrue()) @@ -267,6 +268,7 @@ var _ = Describe("create", Label("machine", "create"), func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("time is up")) + Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightMetadataSupportConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightCreated)).To(BeFalse()) Expect(conditions.Get(&linodeMachine, ConditionPreflightCreated).Severity).To(Equal(clusterv1.ConditionSeverityError)) Expect(conditions.Get(&linodeMachine, ConditionPreflightCreated).Message).To(ContainSubstring("time is up")) @@ -442,6 +444,7 @@ var _ = Describe("create", Label("machine", "create"), func() { _, err = reconciler.reconcileCreate(ctx, logger, &mScope) Expect(err).NotTo(HaveOccurred()) + Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightMetadataSupportConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightCreated)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightBootTriggered)).To(BeTrue()) @@ -532,6 +535,7 @@ var _ = Describe("create", Label("machine", "create"), func() { Expect(res.RequeueAfter).To(Equal(rutil.DefaultMachineControllerWaitForRunningDelay)) Expect(err).ToNot(HaveOccurred()) + Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightMetadataSupportConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightCreated)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightConfigured)).To(BeFalse()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightAdditionalDisksCreated)).To(BeFalse()) @@ -624,6 +628,7 @@ var _ = Describe("create", Label("machine", "create"), func() { _, err = reconciler.reconcileCreate(ctx, logger, &mScope) Expect(err).NotTo(HaveOccurred()) + Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightMetadataSupportConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightCreated)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightConfigured)).To(BeTrue()) Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightBootTriggered)).To(BeTrue()) @@ -921,6 +926,21 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc ctlrSuite.Run( OneOf( Path( + Call("machine is not created because of too many requests", func(ctx context.Context, mck Mock) { + }), + Path(Result("create requeues when failing to create instance config", func(ctx context.Context, mck Mock) { + getRegion := mck.LinodeClient.EXPECT(). + GetRegion(ctx, gomock.Any()). + Return(&linodego.Region{Capabilities: []string{"Metadata"}}, nil) + mck.LinodeClient.EXPECT(). + GetImage(ctx, gomock.Any()). + After(getRegion). + Return(nil, &linodego.Error{Code: http.StatusTooManyRequests}) + res, err := reconciler.reconcile(ctx, mck.Logger(), mScope) + Expect(err).NotTo(HaveOccurred()) + Expect(res.RequeueAfter).To(Equal(rutil.DefaultLinodeTooManyRequestsErrorRetryDelay)) + Expect(mck.Logs()).To(ContainSubstring("Failed to fetch image")) + })), Call("machine is not created because there was an error creating instance", func(ctx context.Context, mck Mock) { }), OneOf( @@ -958,16 +978,11 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc Name: "test-missing-fw", Namespace: namespace, } + linodeMachine.Status.CloudinitMetadataSupport = true + conditions.MarkTrue(mScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured) }), OneOf( Path(Result("create fails when failing to get referenced firewall", func(ctx context.Context, mck Mock) { - getRegion := mck.LinodeClient.EXPECT(). - GetRegion(ctx, gomock.Any()). - Return(&linodego.Region{Capabilities: []string{"Metadata"}}, nil) - mck.LinodeClient.EXPECT(). - GetImage(ctx, gomock.Any()). - After(getRegion). - Return(&linodego.Image{Capabilities: []string{"cloud-init"}}, nil) _, err := reconciler.reconcile(ctx, mck.Logger(), mScope) Expect(err).To(HaveOccurred()) Expect(mck.Logs()).To(ContainSubstring("nil firewallID")) @@ -977,29 +992,11 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc Path( Call("machine is not created because there were too many requests", func(ctx context.Context, mck Mock) { linodeMachine.Spec.FirewallRef = nil + linodeMachine.Status.CloudinitMetadataSupport = true }), OneOf( - Path(Result("create requeues when failing to create instance config", func(ctx context.Context, mck Mock) { - mck.LinodeClient.EXPECT(). - GetRegion(ctx, gomock.Any()). - Return(&linodego.Region{Capabilities: []string{"Metadata"}}, nil) - mck.LinodeClient.EXPECT(). - GetImage(ctx, gomock.Any()). - Return(nil, &linodego.Error{Code: http.StatusTooManyRequests}) - res, err := reconciler.reconcile(ctx, mck.Logger(), mScope) - Expect(err).NotTo(HaveOccurred()) - Expect(res.RequeueAfter).To(Equal(rutil.DefaultLinodeTooManyRequestsErrorRetryDelay)) - Expect(mck.Logs()).To(ContainSubstring("Failed to create Linode machine InstanceCreateOptions")) - })), Path(Result("create requeues when failing to create instance", func(ctx context.Context, mck Mock) { - mck.LinodeClient.EXPECT(). - GetRegion(ctx, gomock.Any()). - Return(&linodego.Region{Capabilities: []string{"Metadata"}}, nil) - getImage := mck.LinodeClient.EXPECT(). - GetImage(ctx, gomock.Any()). - Return(&linodego.Image{Capabilities: []string{"cloud-init"}}, nil) mck.LinodeClient.EXPECT().CreateInstance(gomock.Any(), gomock.Any()). - After(getImage). Return(nil, &linodego.Error{Code: http.StatusTooManyRequests}) mck.LinodeClient.EXPECT(). OnAfterResponse(gomock.Any()). @@ -1011,15 +1008,8 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc })), Path(Result("create requeues when failing to update instance config", func(ctx context.Context, mck Mock) { linodeMachine.Spec.Configuration = &infrav1alpha2.InstanceConfiguration{Kernel: "test"} - mck.LinodeClient.EXPECT(). - GetRegion(ctx, gomock.Any()). - Return(&linodego.Region{Capabilities: []string{"Metadata"}}, nil) - getImage := mck.LinodeClient.EXPECT(). - GetImage(ctx, gomock.Any()). - Return(&linodego.Image{Capabilities: []string{"cloud-init"}}, nil) createInst := mck.LinodeClient.EXPECT(). CreateInstance(ctx, gomock.Any()). - After(getImage). Return(&linodego.Instance{ ID: 123, IPv4: []*net.IP{ptr.To(net.IPv4(192, 168, 0, 2))}, @@ -1529,6 +1519,9 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup") Name: "test-fw", }, }, + Status: infrav1alpha2.LinodeMachineStatus{ + CloudinitMetadataSupport: true, + }, } lpgReconciler = &LinodePlacementGroupReconciler{ @@ -1556,13 +1549,6 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup") It("creates a instance in a PlacementGroup with a firewall", func(ctx SpecContext) { mockLinodeClient := mock.NewMockLinodeClient(mockCtrl) - getRegion := mockLinodeClient.EXPECT(). - GetRegion(ctx, gomock.Any()). - Return(&linodego.Region{Capabilities: []string{linodego.CapabilityMetadata, infrav1alpha2.LinodePlacementGroupCapability}}, nil) - mockLinodeClient.EXPECT(). - GetImage(ctx, gomock.Any()). - After(getRegion). - Return(&linodego.Image{Capabilities: []string{"cloud-init"}}, nil) helper, err := patch.NewHelper(&linodePlacementGroup, k8sClient) Expect(err).NotTo(HaveOccurred()) @@ -1720,15 +1706,11 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() { }, }, }, + Status: infrav1alpha2.LinodeMachineStatus{ + CloudinitMetadataSupport: true, + }, } mockLinodeClient := mock.NewMockLinodeClient(mockCtrl) - getRegion := mockLinodeClient.EXPECT(). - GetRegion(ctx, gomock.Any()). - Return(&linodego.Region{Capabilities: []string{linodego.CapabilityMetadata, infrav1alpha2.LinodePlacementGroupCapability}}, nil) - mockLinodeClient.EXPECT(). - GetImage(ctx, gomock.Any()). - After(getRegion). - Return(&linodego.Image{Capabilities: []string{"cloud-init"}}, nil) mockLinodeClient.EXPECT(). ListVPCs(ctx, gomock.Any()). Return([]linodego.VPC{}, nil) @@ -1802,15 +1784,11 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() { }, }, }, + Status: infrav1alpha2.LinodeMachineStatus{ + CloudinitMetadataSupport: true, + }, } mockLinodeClient := mock.NewMockLinodeClient(mockCtrl) - getRegion := mockLinodeClient.EXPECT(). - GetRegion(ctx, gomock.Any()). - Return(&linodego.Region{Capabilities: []string{linodego.CapabilityMetadata, infrav1alpha2.LinodePlacementGroupCapability}}, nil) - mockLinodeClient.EXPECT(). - GetImage(ctx, gomock.Any()). - After(getRegion). - Return(&linodego.Image{Capabilities: []string{"cloud-init"}}, nil) mockLinodeClient.EXPECT(). ListVPCs(ctx, gomock.Any()). Return([]linodego.VPC{}, nil)