Skip to content

Commit

Permalink
fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@rahulait
rahulait committed Oct 1, 2024
1 parent f76cff3 commit 3f53322
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 146 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha2/linodemachine_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 4 additions & 3 deletions controller/linodemachine_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")

Check warning on line 217 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L217 

Added line #L217 was not covered by tests
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Check warning on line 294 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L293-L294 

Added lines #L293 - L294 were not covered by tests
}
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
Expand All @@ -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

Check warning on line 310 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L309-L310 

Added lines #L309 - L310 were not covered by tests
Expand Down
91 changes: 3 additions & 88 deletions controller/linodemachine_controller_helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
},
},
{
Expand All @@ -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{
Expand All @@ -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,
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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"),
Expand Down
86 changes: 32 additions & 54 deletions controller/linodemachine_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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"))
Expand Down Expand 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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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"))
Expand All @@ -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()).
Expand All @@ -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))},
Expand Down Expand Up @@ -1529,6 +1519,9 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
Name: "test-fw",
},
},
Status: infrav1alpha2.LinodeMachineStatus{
CloudinitMetadataSupport: true,
},
}

lpgReconciler = &LinodePlacementGroupReconciler{
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 3f53322

Please sign in to comment.