Skip to content

Commit

Permalink
handle nil firewall ID if there is an error creating the Firewall ref…
Browse files Browse the repository at this point in the history
…erenced
  • Loading branch information
AshleyDumaine committed Sep 5, 2024
1 parent ca72308 commit 9919e8d
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
11 changes: 8 additions & 3 deletions controller/linodemachine_controller_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,18 +312,23 @@ func getFirewallID(ctx context.Context, machineScope *scope.MachineScope, logger

logger = logger.WithValues("firewallName", name, "firewallNamespace", namespace)

linodeFirewall := infrav1alpha2.LinodeFirewall{
linodeFirewall := &infrav1alpha2.LinodeFirewall{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Name: name,
},
}
objectKey := client.ObjectKeyFromObject(&linodeFirewall)
err := machineScope.Client.Get(ctx, objectKey, &linodeFirewall)
objectKey := client.ObjectKeyFromObject(linodeFirewall)
err := machineScope.Client.Get(ctx, objectKey, linodeFirewall)
if err != nil {
logger.Error(err, "Failed to fetch LinodeFirewall")
return -1, err
}
if linodeFirewall.Spec.FirewallID == nil {
err = errors.New("nil firewallID")
logger.Error(err, "Failed to fetch LinodeFirewall")
return -1, err
}

return *linodeFirewall.Spec.FirewallID, nil
}
Expand Down
34 changes: 34 additions & 0 deletions controller/linodemachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,16 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc
Namespace: namespace,
OwnerReferences: ownerRefs,
}
missingFW := &infrav1alpha2.LinodeFirewall{
ObjectMeta: metav1.ObjectMeta{
Name: "test-missing-fw",
Namespace: namespace,
},
Spec: infrav1alpha2.LinodeFirewallSpec{
FirewallID: nil,
Enabled: true,
},
}
linodeMachine := &infrav1alpha2.LinodeMachine{
ObjectMeta: metadata,
Spec: infrav1alpha2.LinodeMachineSpec{
Expand Down Expand Up @@ -869,6 +879,7 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc
},
}
mScope.Machine = machine
_ = k8sClient.Create(ctx, missingFW)
Expect(k8sClient.Create(ctx, linodeCluster)).To(Succeed())
Expect(k8sClient.Create(ctx, linodeMachine)).To(Succeed())
_ = k8sClient.Create(ctx, secret)
Expand Down Expand Up @@ -920,8 +931,31 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc
})),
),
),
Path(
Call("machine is not created because it fails to get referenced firewall", func(ctx context.Context, mck Mock) {
linodeMachine.Spec.FirewallRef = &corev1.ObjectReference{
Name: "test-missing-fw",
Namespace: namespace,
}
}),
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"))
})),
),
),
Path(
Call("machine is not created because there were too many requests", func(ctx context.Context, mck Mock) {
linodeMachine.Spec.FirewallRef = nil
}),
OneOf(
Path(Result("create requeues when failing to create instance config", func(ctx context.Context, mck Mock) {
Expand Down

0 comments on commit 9919e8d

Please sign in to comment.