Skip to content

Commit 18d4d23

Browse files
guettliguettli
committed
Add testEnv.Resetter to avoid flaky tests
Background: All tests share the same reconciler and mock objects. This means if test-1 changes the mocks, then test-2 could fail. The method CreateNamespace() was changed to ResetAndCreateNamespace(). Flakiness should be reduced, because every test resets the mocks now.
1 parent 2b851a9 commit 18d4d23

12 files changed

+211
-87
lines changed

‎controllers/controllers_suite_test.go‎

Lines changed: 114 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,12 @@ import (
3838
"sigs.k8s.io/controller-runtime/pkg/metrics"
3939

4040
infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
41+
"github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks"
42+
robotmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/robot"
43+
sshmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/ssh"
4144
hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client"
45+
fakehcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client/fake"
46+
"github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/mockedsshclient"
4247
"github.com/syself/cluster-api-provider-hetzner/test/helpers"
4348
)
4449

@@ -63,6 +68,68 @@ func TestControllers(t *testing.T) {
6368
RunSpecs(t, "Controller Suite")
6469
}
6570

71+
type ControllerResetter struct {
72+
HetznerClusterReconciler *HetznerClusterReconciler
73+
HCloudMachineReconciler *HCloudMachineReconciler
74+
HCloudMachineTemplateReconciler *HCloudMachineTemplateReconciler
75+
HetznerBareMetalHostReconciler *HetznerBareMetalHostReconciler
76+
HetznerBareMetalMachineReconciler *HetznerBareMetalMachineReconciler
77+
HCloudRemediationReconciler *HCloudRemediationReconciler
78+
HetznerBareMetalRemediationReconciler *HetznerBareMetalRemediationReconciler
79+
}
80+
81+
func (r *ControllerResetter) Reset(testEnv *helpers.TestEnvironment, t FullGinkgoTInterface) {
82+
rescueSSHClient := &sshmock.Client{}
83+
rescueSSHClient.Test(t)
84+
85+
osSSHClientAfterInstallImage := &sshmock.Client{}
86+
osSSHClientAfterInstallImage.Test(t)
87+
88+
osSSHClientAfterCloudInit := &sshmock.Client{}
89+
osSSHClientAfterCloudInit.Test(t)
90+
91+
robotClient := &robotmock.Client{}
92+
robotClient.Test(t)
93+
94+
hcloudSSHClient := &sshmock.Client{}
95+
hcloudSSHClient.Test(t)
96+
97+
hcloudClientFactory := fakehcloudclient.NewHCloudClientFactory()
98+
99+
robotClientFactory := mocks.NewRobotFactory(robotClient)
100+
baremetalSSHClientFactory := mocks.NewSSHFactory(rescueSSHClient,
101+
osSSHClientAfterInstallImage, osSSHClientAfterCloudInit)
102+
{
103+
// Reset clients used by the test code
104+
testEnv.BaremetalSSHClientFactory = mocks.NewSSHFactory(rescueSSHClient,
105+
osSSHClientAfterInstallImage, osSSHClientAfterCloudInit)
106+
testEnv.HCloudSSHClientFactory = mockedsshclient.NewSSHFactory(hcloudSSHClient)
107+
testEnv.RescueSSHClient = rescueSSHClient
108+
testEnv.OSSSHClientAfterInstallImage = osSSHClientAfterInstallImage
109+
testEnv.OSSSHClientAfterCloudInit = osSSHClientAfterCloudInit
110+
testEnv.RobotClientFactory = robotClientFactory
111+
testEnv.RobotClient = robotClient
112+
}
113+
114+
{
115+
// Reset clients used by Reconcile()
116+
r.HetznerClusterReconciler.HCloudClientFactory = hcloudClientFactory
117+
118+
r.HCloudMachineReconciler.HCloudClientFactory = hcloudClientFactory
119+
r.HCloudMachineReconciler.SSHClientFactory = baremetalSSHClientFactory
120+
r.HCloudMachineReconciler.HCloudClientFactory = hcloudClientFactory
121+
122+
r.HCloudMachineTemplateReconciler.HCloudClientFactory = hcloudClientFactory
123+
r.HetznerBareMetalHostReconciler.RobotClientFactory = robotClientFactory
124+
r.HetznerBareMetalHostReconciler.SSHClientFactory = baremetalSSHClientFactory
125+
126+
r.HCloudRemediationReconciler.HCloudClientFactory = hcloudClientFactory
127+
128+
r.HetznerBareMetalMachineReconciler.HCloudClientFactory = hcloudClientFactory
129+
130+
}
131+
}
132+
66133
var _ = BeforeSuite(func() {
67134
utilruntime.Must(infrav1.AddToScheme(scheme.Scheme))
68135
utilruntime.Must(clusterv1.AddToScheme(scheme.Scheme))
@@ -71,52 +138,62 @@ var _ = BeforeSuite(func() {
71138
hcloudClient = testEnv.HCloudClientFactory.NewClient("")
72139
wg.Add(1)
73140

74-
Expect((&HetznerClusterReconciler{
141+
resetter := ControllerResetter{}
142+
143+
hetznerClusterReconciler := &HetznerClusterReconciler{
75144
Client: testEnv.Manager.GetClient(),
76145
APIReader: testEnv.Manager.GetAPIReader(),
77146
RateLimitWaitTime: 5 * time.Minute,
78-
HCloudClientFactory: testEnv.HCloudClientFactory,
79147
TargetClusterManagersWaitGroup: &wg,
80-
}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
81-
82-
Expect((&HCloudMachineReconciler{
83-
Client: testEnv.Manager.GetClient(),
84-
APIReader: testEnv.Manager.GetAPIReader(),
85-
HCloudClientFactory: testEnv.HCloudClientFactory,
86-
SSHClientFactory: testEnv.BaremetalSSHClientFactory,
87-
}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
88-
89-
Expect((&HCloudMachineTemplateReconciler{
90-
Client: testEnv.Manager.GetClient(),
91-
APIReader: testEnv.Manager.GetAPIReader(),
92-
HCloudClientFactory: testEnv.HCloudClientFactory,
93-
}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
94-
95-
Expect((&HetznerBareMetalHostReconciler{
148+
}
149+
Expect(hetznerClusterReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
150+
resetter.HetznerClusterReconciler = hetznerClusterReconciler
151+
152+
hcloudMachineReconciler := &HCloudMachineReconciler{
153+
Client: testEnv.Manager.GetClient(),
154+
APIReader: testEnv.Manager.GetAPIReader(),
155+
}
156+
Expect(hcloudMachineReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
157+
resetter.HCloudMachineReconciler = hcloudMachineReconciler
158+
159+
hcloudMachineTemplateReconciler := &HCloudMachineTemplateReconciler{
160+
Client: testEnv.Manager.GetClient(),
161+
APIReader: testEnv.Manager.GetAPIReader(),
162+
}
163+
Expect(hcloudMachineTemplateReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
164+
resetter.HCloudMachineTemplateReconciler = hcloudMachineTemplateReconciler
165+
166+
hetznerBareMetalHostReconciler := &HetznerBareMetalHostReconciler{
96167
Client: testEnv.Manager.GetClient(),
97168
APIReader: testEnv.Manager.GetAPIReader(),
98-
RobotClientFactory: testEnv.RobotClientFactory,
99-
SSHClientFactory: testEnv.BaremetalSSHClientFactory,
100169
PreProvisionCommand: "dummy-pre-provision-command",
101170
SSHAfterInstallImage: true,
102-
}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
103-
104-
Expect((&HetznerBareMetalMachineReconciler{
105-
Client: testEnv.Manager.GetClient(),
106-
APIReader: testEnv.Manager.GetAPIReader(),
107-
HCloudClientFactory: testEnv.HCloudClientFactory,
108-
}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
109-
110-
Expect((&HCloudRemediationReconciler{
111-
Client: testEnv.Manager.GetClient(),
112-
APIReader: testEnv.Manager.GetAPIReader(),
113-
RateLimitWaitTime: 5 * time.Minute,
114-
HCloudClientFactory: testEnv.HCloudClientFactory,
115-
}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
116-
117-
Expect((&HetznerBareMetalRemediationReconciler{
171+
}
172+
Expect(hetznerBareMetalHostReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
173+
resetter.HetznerBareMetalHostReconciler = hetznerBareMetalHostReconciler
174+
175+
hetznerBareMetalMachineReconciler := &HetznerBareMetalMachineReconciler{
176+
Client: testEnv.Manager.GetClient(),
177+
APIReader: testEnv.Manager.GetAPIReader(),
178+
}
179+
Expect(hetznerBareMetalMachineReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
180+
resetter.HetznerBareMetalMachineReconciler = hetznerBareMetalMachineReconciler
181+
182+
hcloudRemediationReconciler := &HCloudRemediationReconciler{
183+
Client: testEnv.Manager.GetClient(),
184+
APIReader: testEnv.Manager.GetAPIReader(),
185+
RateLimitWaitTime: 5 * time.Minute,
186+
}
187+
Expect(hcloudRemediationReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
188+
resetter.HCloudRemediationReconciler = hcloudRemediationReconciler
189+
190+
hetznerBareMetalRemediationReconciler := &HetznerBareMetalRemediationReconciler{
118191
Client: testEnv.Manager.GetClient(),
119-
}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
192+
}
193+
Expect(hetznerBareMetalRemediationReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
194+
resetter.HetznerBareMetalRemediationReconciler = hetznerBareMetalRemediationReconciler
195+
196+
testEnv.Resetter = &resetter
120197

121198
go func() {
122199
defer GinkgoRecover()

‎controllers/hcloudmachine_controller_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ var _ = Describe("HCloudMachineReconciler", func() {
243243
hcloudClient.Reset()
244244

245245
var err error
246-
testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-reconciler")
246+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-reconciler")
247247
Expect(err).NotTo(HaveOccurred())
248248

249249
capiCluster = &clusterv1.Cluster{
@@ -655,7 +655,7 @@ var _ = Describe("Hetzner secret", func() {
655655
BeforeEach(func() {
656656
hcloudClient.Reset()
657657
var err error
658-
testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation")
658+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation")
659659
Expect(err).NotTo(HaveOccurred())
660660

661661
hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test")
@@ -807,7 +807,7 @@ var _ = Describe("HCloudMachine validation", func() {
807807
hcloudClient.Reset()
808808

809809
var err error
810-
testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation")
810+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation")
811811
Expect(err).NotTo(HaveOccurred())
812812

813813
hcloudMachine = &infrav1.HCloudMachine{

‎controllers/hcloudmachinetemplate_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ var _ = Describe("HCloudMachineTemplateReconciler", func() {
4040
BeforeEach(func() {
4141
hcloudClient.Reset()
4242
var err error
43-
testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachinetemplate-reconciler")
43+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachinetemplate-reconciler")
4444
Expect(err).NotTo(HaveOccurred())
4545

4646
hetznerSecret = getDefaultHetznerSecret(testNs.Name)
@@ -253,7 +253,7 @@ var _ = Describe("HCloudMachineTemplateReconciler", func() {
253253
)
254254
BeforeEach(func() {
255255
var err error
256-
testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation")
256+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation")
257257
Expect(err).NotTo(HaveOccurred())
258258

259259
hcloudMachineTemplate = &infrav1.HCloudMachineTemplate{

‎controllers/hcloudremediation_controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ var _ = Describe("HCloudRemediationReconciler", func() {
5757
BeforeEach(func() {
5858
hcloudClient.Reset()
5959
var err error
60-
testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachinetemplate-reconciler")
60+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachinetemplate-reconciler")
6161
Expect(err).NotTo(HaveOccurred())
6262

6363
hetznerSecret = getDefaultHetznerSecret(testNs.Name)

‎controllers/hetznerbaremetalhost_controller.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/google/go-cmp/cmp"
2727
corev1 "k8s.io/api/core/v1"
2828
apierrors "k8s.io/apimachinery/pkg/api/errors"
29+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/types"
3031
"k8s.io/apimachinery/pkg/util/wait"
3132
"k8s.io/klog/v2"
@@ -116,6 +117,12 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl
116117
// Use uncached APIReader
117118
err := r.APIReader.Get(ctx, client.ObjectKeyFromObject(bmHost), apiserverHost)
118119
if err != nil {
120+
if apierrors.IsNotFound(err) {
121+
// resource was deleted. No need to reconcile again.
122+
reterr = nil
123+
res = reconcile.Result{}
124+
return
125+
}
119126
reterr = errors.Join(reterr,
120127
fmt.Errorf("failed get HetznerBareMetalHost via uncached APIReader: %w", err))
121128
return
@@ -185,6 +192,25 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl
185192
return res, nil
186193
}
187194

195+
{
196+
// Stop reconciling, if namspace gets deleted. Used for envTests.
197+
// Related: https://book.kubebuilder.io/reference/envtest.html#testing-considerations
198+
ns := &corev1.Namespace{
199+
ObjectMeta: v1.ObjectMeta{
200+
Name: req.Namespace,
201+
},
202+
}
203+
err = r.Client.Get(ctx, client.ObjectKeyFromObject(ns), ns)
204+
if err != nil {
205+
return ctrl.Result{}, fmt.Errorf("failed to get namespace: %w", err)
206+
}
207+
if !ns.DeletionTimestamp.IsZero() {
208+
// Namespace gets deleted. Stop reconciling this resource
209+
log.Info("Namespace is deleting. Not reconciling")
210+
return ctrl.Result{}, nil
211+
}
212+
}
213+
188214
hetznerCluster := &infrav1.HetznerCluster{}
189215

190216
hetznerClusterName := client.ObjectKey{

‎controllers/hetznerbaremetalhost_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
8686

8787
BeforeEach(func() {
8888
var err error
89-
testNs, err = testEnv.CreateNamespace(ctx, "baremetalhost-reconciler")
89+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "baremetalhost-reconciler")
9090
Expect(err).NotTo(HaveOccurred())
9191

9292
hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test")
@@ -615,7 +615,7 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
615615

616616
BeforeEach(func() {
617617
var err error
618-
testNs, err = testEnv.CreateNamespace(ctx, "baremetalmachine-reconciler")
618+
testNs, err = testEnv.ResetAndCreateNamespace(ctx, "baremetalmachine-reconciler")
619619
Expect(err).NotTo(HaveOccurred())
620620

621621
hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test")

‎controllers/hetznerbaremetalmachine_controller.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ import (
2323
"time"
2424

2525
"github.com/go-logr/logr"
26+
corev1 "k8s.io/api/core/v1"
2627
apierrors "k8s.io/apimachinery/pkg/api/errors"
28+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2729
"k8s.io/apimachinery/pkg/types"
2830
"k8s.io/klog/v2"
2931
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -159,6 +161,25 @@ func (r *HetznerBareMetalMachineReconciler) Reconcile(ctx context.Context, req r
159161
return r.reconcileDelete(ctx, machineScope)
160162
}
161163

164+
{
165+
// Stop reconciling, if namspace gets deleted. Used for envTests.
166+
// Related: https://book.kubebuilder.io/reference/envtest.html#testing-considerations
167+
ns := &corev1.Namespace{
168+
ObjectMeta: v1.ObjectMeta{
169+
Name: req.Namespace,
170+
},
171+
}
172+
err = r.Client.Get(ctx, client.ObjectKeyFromObject(ns), ns)
173+
if err != nil {
174+
return ctrl.Result{}, fmt.Errorf("failed to get namespace: %w", err)
175+
}
176+
if !ns.DeletionTimestamp.IsZero() {
177+
// Namespace gets deleted. Stop reconciling this resource
178+
log.Info("Namespace is deleting. Not reconciling")
179+
return ctrl.Result{}, nil
180+
}
181+
}
182+
162183
return r.reconcileNormal(ctx, machineScope)
163184
}
164185

0 commit comments

Comments
 (0)