Skip to content

Commit dd26db7

Browse files
tchinmai7tchinmai7
committed
Fix tests
1 parent 9396f55 commit dd26db7

14 files changed

+37
-153
lines changed

‎internal/controller/linodecluster_controller.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"errors"
2222
"fmt"
2323
"net/http"
24+
"sigs.k8s.io/cluster-api/util/patch"
2425
"time"
2526

2627
"github.com/go-logr/logr"
@@ -118,6 +119,7 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
118119
}
119120

120121
func (r *LinodeClusterReconciler) reconcilePause(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) error {
122+
// Pausing a cluster pauses the VPC as well.
121123
// First thing to do is handle a paused Cluster. Paused clusters shouldn't be deleted.
122124
isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, clusterScope.Client, clusterScope.Cluster, clusterScope.LinodeCluster)
123125
if err == nil && !isPaused && !conditionChanged {
@@ -160,7 +162,11 @@ func (r *LinodeClusterReconciler) reconcilePause(ctx context.Context, clusterSco
160162
delete(annotations, clusterv1.PausedAnnotation)
161163
}
162164
linodeVPC.SetAnnotations(annotations)
163-
return clusterScope.PatchHelper.Patch(ctx, &linodeVPC)
165+
vpcPatchHelper, err := patch.NewHelper(&linodeVPC, clusterScope.Client)
166+
if err != nil {
167+
return fmt.Errorf("failed to build patch helper for linode VPC object: %w", err)
168+
}
169+
return vpcPatchHelper.Patch(ctx, &linodeVPC)
164170
}
165171

166172
//nolint:cyclop // can't make it simpler with existing API

‎internal/controller/linodecluster_controller_test.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package controller
1919
import (
2020
"context"
2121
"errors"
22-
2322
"github.com/go-logr/logr"
2423
"github.com/linode/linodego"
2524
"go.uber.org/mock/gomock"
@@ -65,7 +64,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
6564
ObjectMeta: metadata,
6665
Spec: infrav1alpha2.LinodeClusterSpec{
6766
Region: "us-ord",
68-
VPCRef: &corev1.ObjectReference{Name: "vpctest"},
67+
VPCRef: &corev1.ObjectReference{Name: "vpctest", Namespace: defaultNamespace},
6968
},
7069
}
7170
linodeVPC := infrav1alpha2.LinodeVPC{
@@ -124,15 +123,10 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
124123
OneOf(
125124
Path(Result("", func(ctx context.Context, mck Mock) {
126125
reconciler.Client = k8sClient
127-
err = cScope.Client.Get(ctx, clusterKey, cScope.LinodeCluster)
128-
Expect(err).NotTo(HaveOccurred())
129-
126+
// first for pause reconciliation
130127
_, err := reconciler.reconcile(ctx, cScope, mck.Logger())
131128
Expect(err).NotTo(HaveOccurred())
132-
133-
err = cScope.Client.Get(ctx, clusterKey, cScope.LinodeCluster)
134-
Expect(err).NotTo(HaveOccurred())
135-
// pause is done, now real thing
129+
// second for real
136130
_, err = reconciler.reconcile(ctx, cScope, mck.Logger())
137131
Expect(err).NotTo(HaveOccurred())
138132
Expect(rec.ConditionTrue(&linodeCluster, ConditionPreflightLinodeVPCReady)).To(BeFalse())
@@ -145,8 +139,13 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
145139
}),
146140
Result("", func(ctx context.Context, mck Mock) {
147141
reconciler.Client = k8sClient
142+
// first reconcile is for pause
148143
_, err := reconciler.reconcile(ctx, cScope, mck.Logger())
149144
Expect(err).NotTo(HaveOccurred())
145+
146+
// second reconcile is for real
147+
_, err = reconciler.reconcile(ctx, cScope, mck.Logger())
148+
Expect(err).NotTo(HaveOccurred())
150149
Expect(rec.ConditionTrue(&linodeCluster, ConditionPreflightLinodeNBFirewallReady)).To(BeFalse())
151150
}),
152151
),
@@ -329,10 +328,11 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
329328
clusterKey := client.ObjectKeyFromObject(&linodeCluster)
330329
Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed())
331330
Expect(linodeCluster.Status.Ready).To(BeTrue())
332-
Expect(linodeCluster.Status.Conditions).To(HaveLen(3))
333-
Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(string(clusterv1.ReadyCondition)))
334-
Expect(linodeCluster.Status.Conditions[1].Type).To(Equal(ConditionPreflightLinodeNBFirewallReady))
335-
Expect(linodeCluster.Status.Conditions[2].Type).To(Equal(ConditionPreflightLinodeVPCReady))
331+
Expect(linodeCluster.Status.Conditions).To(HaveLen(4))
332+
Expect(conditions.Get(&linodeCluster, clusterv1.PausedV1Beta2Condition).Status).To(Equal(metav1.ConditionFalse))
333+
Expect(conditions.Get(&linodeCluster, string(clusterv1.ReadyCondition)).Status).To(Equal(metav1.ConditionTrue))
334+
Expect(conditions.Get(&linodeCluster, ConditionPreflightLinodeNBFirewallReady)).NotTo(BeNil())
335+
Expect(conditions.Get(&linodeCluster, ConditionPreflightLinodeVPCReady)).NotTo(BeNil())
336336
By("checking NB id")
337337
Expect(linodeCluster.Spec.Network.NodeBalancerID).To(Equal(&nodebalancerID))
338338

@@ -440,7 +440,7 @@ var _ = Describe("cluster-lifecycle-dns", Ordered, Label("cluster", "cluster-lif
440440
_, err := reconciler.reconcile(ctx, cScope, logr.Logger{})
441441
Expect(err).NotTo(HaveOccurred())
442442

443-
// One more for pause
443+
// Once more for pause
444444
_, err = reconciler.reconcile(ctx, cScope, logr.Logger{})
445445
Expect(err).NotTo(HaveOccurred())
446446
By("checking ready conditions")

‎internal/controller/linodefirewall_controller.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3333
kutil "sigs.k8s.io/cluster-api/util"
3434
conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2"
35-
"sigs.k8s.io/cluster-api/util/paused"
3635
"sigs.k8s.io/cluster-api/util/predicates"
3736
ctrl "sigs.k8s.io/controller-runtime"
3837
"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -138,12 +137,6 @@ func (r *LinodeFirewallReconciler) reconcile(
138137
return ctrl.Result{}, err
139138
}
140139

141-
// Pause
142-
// We don't have much to do, but simply requeue without an error if we are paused or if we were recently unpaused
143-
if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, fwScope.Client, nil, fwScope.LinodeFirewall); err != nil || isPaused || conditionChanged {
144-
return ctrl.Result{}, err
145-
}
146-
147140
// Delete
148141
if !fwScope.LinodeFirewall.ObjectMeta.DeletionTimestamp.IsZero() {
149142
failureReason = infrav1alpha2.DeleteFirewallError

‎internal/controller/linodefirewall_controller_test.go‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,6 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() {
157157
Path(Result("create requeues", func(ctx context.Context, mck Mock) {
158158
res, err := reconciler.reconcile(ctx, mck.Logger(), &fwScope)
159159
Expect(err).NotTo(HaveOccurred())
160-
// first one is for pause
161-
res, err = reconciler.reconcile(ctx, mck.Logger(), &fwScope)
162-
Expect(err).NotTo(HaveOccurred())
163160
Expect(res.RequeueAfter).To(Equal(rec.DefaultFWControllerReconcilerDelay))
164161
Expect(mck.Logs()).To(ContainSubstring("re-queuing Firewall create"))
165162
})),
@@ -197,9 +194,6 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() {
197194
Result("success", func(ctx context.Context, mck Mock) {
198195
_, err := reconciler.reconcile(ctx, mck.Logger(), &fwScope)
199196
Expect(err).NotTo(HaveOccurred())
200-
// once more after pause
201-
_, err = reconciler.reconcile(ctx, mck.Logger(), &fwScope)
202-
Expect(err).NotTo(HaveOccurred())
203197
Expect(k8sClient.Get(ctx, fwObjectKey, &linodeFW)).To(Succeed())
204198
Expect(*linodeFW.Spec.FirewallID).To(Equal(1))
205199
Expect(mck.Logs()).NotTo(ContainSubstring("failed to create Firewall"))
@@ -214,18 +208,14 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() {
214208
}),
215209
OneOf(
216210
Path(Result("update requeues for update rules error", func(ctx context.Context, mck Mock) {
217-
mck.LinodeClient.EXPECT().UpdateFirewallRules(ctx, 1, gomock.Any()).Return(nil, &linodego.Error{Code: http.StatusInternalServerError})
218-
res, err := reconciler.reconcile(ctx, mck.Logger(), &fwScope)
219-
Expect(err).NotTo(HaveOccurred())
220-
221211
conditions.Set(fwScope.LinodeFirewall, metav1.Condition{
222212
Type: string(clusterv1.ReadyCondition),
223213
Status: metav1.ConditionFalse,
224214
Reason: "test",
225215
Message: "test",
226216
})
227-
// after pause is done, do the real reconcile
228-
res, err = reconciler.reconcile(ctx, mck.Logger(), &fwScope)
217+
mck.LinodeClient.EXPECT().UpdateFirewallRules(ctx, 1, gomock.Any()).Return(nil, &linodego.Error{Code: http.StatusInternalServerError})
218+
res, err := reconciler.reconcile(ctx, mck.Logger(), &fwScope)
229219
Expect(err).NotTo(HaveOccurred())
230220
Expect(res.RequeueAfter).To(Equal(rec.DefaultFWControllerReconcilerDelay))
231221
Expect(mck.Logs()).To(ContainSubstring("re-queuing Firewall update"))
@@ -290,18 +280,12 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() {
290280
Path(Result("deletes are requeued", func(ctx context.Context, mck Mock) {
291281
res, err := reconciler.reconcile(ctx, mck.Logger(), &fwScope)
292282
Expect(err).NotTo(HaveOccurred())
293-
// Now do it after the pause is done
294-
res, err = reconciler.reconcile(ctx, mck.Logger(), &fwScope)
295-
Expect(err).NotTo(HaveOccurred())
296283
Expect(res.RequeueAfter).To(Equal(rec.DefaultFWControllerReconcilerDelay))
297284
Expect(mck.Logs()).To(ContainSubstring("failed to delete Firewall"))
298285
})),
299286
Path(Result("timeout error", func(ctx context.Context, mck Mock) {
300287
reconciler.ReconcileTimeout = time.Nanosecond
301288
res, err := reconciler.reconcile(ctx, mck.Logger(), &fwScope)
302-
Expect(err).NotTo(HaveOccurred())
303-
// Now pause is done
304-
res, err = reconciler.reconcile(ctx, mck.Logger(), &fwScope)
305289
Expect(err).To(HaveOccurred())
306290
Expect(res.RequeueAfter).To(Equal(time.Duration(0)))
307291
})),

‎internal/controller/linodemachine_controller.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"errors"
2222
"fmt"
2323
"net/http"
24+
"sigs.k8s.io/cluster-api/util/patch"
2425
"slices"
2526
"strings"
2627
"time"
@@ -171,7 +172,7 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
171172
}
172173

173174
func (r *LinodeMachineReconciler) reconcilePause(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) error {
174-
// Pause
175+
// Pausing a machine Pauses the firewall referred by the machine
175176
isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, machineScope.Client, machineScope.Cluster, machineScope.LinodeMachine)
176177

177178
if err == nil && !isPaused && !conditionChanged {
@@ -212,7 +213,11 @@ func (r *LinodeMachineReconciler) reconcilePause(ctx context.Context, logger log
212213
delete(annotations, clusterv1.PausedAnnotation)
213214
}
214215
linodeFW.SetAnnotations(annotations)
215-
return machineScope.PatchHelper.Patch(ctx, &linodeFW)
216+
fwPatchHelper, err := patch.NewHelper(&linodeFW, machineScope.Client)
217+
if err != nil {
218+
return fmt.Errorf("failed to create patch helper for firewalls: %w", err)
219+
}
220+
return fwPatchHelper.Patch(ctx, &linodeFW)
216221
}
217222

218223
func (r *LinodeMachineReconciler) reconcile(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (res ctrl.Result, err error) {

‎internal/controller/linodemachine_controller_test.go‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,7 +1928,6 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
19281928

19291929
It("creates a instance in a PlacementGroup with a firewall", func(ctx SpecContext) {
19301930
mockLinodeClient := mock.NewMockLinodeClient(mockCtrl)
1931-
19321931
helper, err := patch.NewHelper(&linodePlacementGroup, k8sClient)
19331932
Expect(err).NotTo(HaveOccurred())
19341933

@@ -1939,16 +1938,6 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
19391938
LinodePlacementGroup: &linodePlacementGroup,
19401939
})
19411940

1942-
Expect(err).NotTo(HaveOccurred())
1943-
1944-
// gotta do it again for pause
1945-
_, err = lpgReconciler.reconcile(ctx, logger, &scope.PlacementGroupScope{
1946-
PatchHelper: helper,
1947-
Client: k8sClient,
1948-
LinodeClient: mockLinodeClient,
1949-
LinodePlacementGroup: &linodePlacementGroup,
1950-
})
1951-
19521941
Expect(err).NotTo(HaveOccurred())
19531942
mScope := scope.MachineScope{
19541943
Client: k8sClient,
@@ -2125,14 +2114,6 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() {
21252114
LinodeVPC: &linodeVPC,
21262115
})
21272116

2128-
Expect(err).NotTo(HaveOccurred())
2129-
// reconcile again for the real thing
2130-
_, err = lvpcReconciler.reconcile(ctx, logger, &scope.VPCScope{
2131-
PatchHelper: helper,
2132-
Client: k8sClient,
2133-
LinodeClient: mockLinodeClient,
2134-
LinodeVPC: &linodeVPC,
2135-
})
21362117
Expect(err).NotTo(HaveOccurred())
21372118

21382119
mScope := scope.MachineScope{
@@ -2209,15 +2190,6 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() {
22092190
LinodeVPC: &linodeVPC,
22102191
})
22112192

2212-
Expect(err).NotTo(HaveOccurred())
2213-
// second one is for real - first just does the pause
2214-
_, err = lvpcReconciler.reconcile(ctx, logger, &scope.VPCScope{
2215-
PatchHelper: helper,
2216-
Client: k8sClient,
2217-
LinodeClient: mockLinodeClient,
2218-
LinodeVPC: &linodeVPC,
2219-
})
2220-
22212193
Expect(err).NotTo(HaveOccurred())
22222194

22232195
mScope := scope.MachineScope{

‎internal/controller/linodeobjectstoragebucket_controller.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3131
kutil "sigs.k8s.io/cluster-api/util"
3232
conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2"
33-
"sigs.k8s.io/cluster-api/util/paused"
3433
"sigs.k8s.io/cluster-api/util/predicates"
3534
ctrl "sigs.k8s.io/controller-runtime"
3635
"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -117,12 +116,6 @@ func (r *LinodeObjectStorageBucketReconciler) reconcile(ctx context.Context, bSc
117116
}
118117
}()
119118

120-
// Pause
121-
// We don't have much to do, but simply requeue without an error if we are paused or if we were recently unpaused
122-
if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, bScope.Client, nil, bScope.Bucket); err != nil || isPaused || conditionChanged {
123-
return ctrl.Result{}, err
124-
}
125-
126119
if err := r.reconcileApply(ctx, bScope); err != nil {
127120
return res, err
128121
}
@@ -188,7 +181,7 @@ func (r *LinodeObjectStorageBucketReconciler) SetupWithManager(mgr ctrl.Manager,
188181
WithOptions(options).
189182
Owns(&corev1.Secret{}).
190183
WithEventFilter(predicate.And(
191-
predicates.ResourceHasFilterLabel(mgr.GetScheme(), mgr.GetLogger(), r.WatchFilterValue),
184+
predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), mgr.GetLogger(), r.WatchFilterValue),
192185
predicate.GenerationChangedPredicate{},
193186
)).
194187
Watches(

‎internal/controller/linodeobjectstoragebucket_controller_test.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,12 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
9999
bScope.LinodeClient = mck.LinodeClient
100100
_, err := reconciler.reconcile(ctx, &bScope)
101101
Expect(err).NotTo(HaveOccurred())
102-
// second one is the real thing
103-
_, err = reconciler.reconcile(ctx, &bScope)
104-
Expect(err).NotTo(HaveOccurred())
105102

106103
By("status")
107104
Expect(k8sClient.Get(ctx, objectKey, &obj)).To(Succeed())
108105
Expect(obj.Status.Ready).To(BeTrue())
109106
Expect(obj.Status.FailureMessage).To(BeNil())
110-
Expect(obj.Status.Conditions).To(HaveLen(2))
107+
Expect(obj.Status.Conditions).To(HaveLen(1))
111108
readyCond := conditions.Get(&obj, string(clusterv1.ReadyCondition))
112109
Expect(readyCond).NotTo(BeNil())
113110
Expect(*obj.Status.Hostname).To(Equal("hostname"))
@@ -145,8 +142,6 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
145142
bScope.LinodeClient = mck.LinodeClient
146143
_, err := reconciler.reconcile(ctx, &bScope)
147144
Expect(err).NotTo(BeNil())
148-
// pause is done, now retry
149-
_, err = reconciler.reconcile(ctx, &bScope)
150145
Expect(err.Error()).To(ContainSubstring("get bucket error"))
151146
}),
152147
),

‎internal/controller/linodeobjectstoragekey_controller.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3434
kutil "sigs.k8s.io/cluster-api/util"
3535
conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2"
36-
"sigs.k8s.io/cluster-api/util/paused"
3736
"sigs.k8s.io/cluster-api/util/predicates"
3837
ctrl "sigs.k8s.io/controller-runtime"
3938
"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -130,12 +129,6 @@ func (r *LinodeObjectStorageKeyReconciler) reconcile(ctx context.Context, keySco
130129
return res, err
131130
}
132131

133-
// Pause
134-
// We don't have much to do, but simply requeue without an error if we are paused or if we were recently unpaused
135-
if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, keyScope.Client, nil, keyScope.Key); err != nil || isPaused || conditionChanged {
136-
return ctrl.Result{}, err
137-
}
138-
139132
if !keyScope.Key.DeletionTimestamp.IsZero() {
140133
return res, r.reconcileDelete(ctx, keyScope)
141134
}
@@ -319,7 +312,7 @@ func (r *LinodeObjectStorageKeyReconciler) SetupWithManager(mgr ctrl.Manager, op
319312
WithOptions(options).
320313
Owns(&corev1.Secret{}).
321314
WithEventFilter(predicate.And(
322-
predicates.ResourceHasFilterLabel(mgr.GetScheme(), mgr.GetLogger(), r.WatchFilterValue),
315+
predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), mgr.GetLogger(), r.WatchFilterValue),
323316
predicate.GenerationChangedPredicate{},
324317
)).
325318
Watches(

0 commit comments

Comments
 (0)