diff --git a/controllers/controllers_suite_test.go b/controllers/controllers_suite_test.go index 54f351ef6..a897b50c5 100644 --- a/controllers/controllers_suite_test.go +++ b/controllers/controllers_suite_test.go @@ -38,7 +38,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics" infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" + "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks" + robotmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/robot" + sshmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/ssh" hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client" + fakehcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client/fake" + "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/mockedsshclient" "github.com/syself/cluster-api-provider-hetzner/test/helpers" ) @@ -63,6 +68,74 @@ func TestControllers(t *testing.T) { RunSpecs(t, "Controller Suite") } +type ControllerResetter struct { + HetznerClusterReconciler *HetznerClusterReconciler + HCloudMachineReconciler *HCloudMachineReconciler + HCloudMachineTemplateReconciler *HCloudMachineTemplateReconciler + HetznerBareMetalHostReconciler *HetznerBareMetalHostReconciler + HetznerBareMetalMachineReconciler *HetznerBareMetalMachineReconciler + HCloudRemediationReconciler *HCloudRemediationReconciler + HetznerBareMetalRemediationReconciler *HetznerBareMetalRemediationReconciler +} + +var _ helpers.Resetter = &ControllerResetter{} + +func (r *ControllerResetter) Reset(namespace string, testEnv *helpers.TestEnvironment, t FullGinkgoTInterface) { + rescueSSHClient := &sshmock.Client{} + rescueSSHClient.Test(t) + + osSSHClientAfterInstallImage := &sshmock.Client{} + osSSHClientAfterInstallImage.Test(t) + + osSSHClientAfterCloudInit := &sshmock.Client{} + osSSHClientAfterCloudInit.Test(t) + + robotClient := &robotmock.Client{} + robotClient.Test(t) + + hcloudSSHClient := &sshmock.Client{} + hcloudSSHClient.Test(t) + + hcloudClientFactory := fakehcloudclient.NewHCloudClientFactory() + + robotClientFactory := mocks.NewRobotFactory(robotClient) + baremetalSSHClientFactory := mocks.NewSSHFactory(rescueSSHClient, + osSSHClientAfterInstallImage, osSSHClientAfterCloudInit) + + // Reset clients used by the test code + testEnv.BaremetalSSHClientFactory = mocks.NewSSHFactory(rescueSSHClient, + osSSHClientAfterInstallImage, osSSHClientAfterCloudInit) + testEnv.HCloudSSHClientFactory = mockedsshclient.NewSSHFactory(hcloudSSHClient) + testEnv.RescueSSHClient = rescueSSHClient + testEnv.OSSSHClientAfterInstallImage = osSSHClientAfterInstallImage + testEnv.OSSSHClientAfterCloudInit = osSSHClientAfterCloudInit + testEnv.RobotClientFactory = robotClientFactory + testEnv.RobotClient = robotClient + + // Reset clients used by Reconcile() and the namespace + r.HetznerClusterReconciler.HCloudClientFactory = hcloudClientFactory + r.HetznerClusterReconciler.Namespace = namespace + + r.HCloudMachineReconciler.HCloudClientFactory = hcloudClientFactory + r.HCloudMachineReconciler.SSHClientFactory = baremetalSSHClientFactory + r.HCloudMachineReconciler.Namespace = namespace + + r.HCloudMachineTemplateReconciler.HCloudClientFactory = hcloudClientFactory + r.HCloudMachineTemplateReconciler.Namespace = namespace + + r.HetznerBareMetalHostReconciler.RobotClientFactory = robotClientFactory + r.HetznerBareMetalHostReconciler.SSHClientFactory = baremetalSSHClientFactory + r.HetznerBareMetalHostReconciler.Namespace = namespace + + r.HCloudRemediationReconciler.HCloudClientFactory = hcloudClientFactory + r.HCloudRemediationReconciler.Namespace = namespace + + r.HetznerBareMetalMachineReconciler.HCloudClientFactory = hcloudClientFactory + r.HetznerBareMetalMachineReconciler.Namespace = namespace + + r.HetznerBareMetalRemediationReconciler.Namespace = namespace +} + var _ = BeforeSuite(func() { utilruntime.Must(infrav1.AddToScheme(scheme.Scheme)) utilruntime.Must(clusterv1.AddToScheme(scheme.Scheme)) @@ -71,51 +144,62 @@ var _ = BeforeSuite(func() { hcloudClient = testEnv.HCloudClientFactory.NewClient("") wg.Add(1) - Expect((&HetznerClusterReconciler{ + resetter := ControllerResetter{} + + hetznerClusterReconciler := &HetznerClusterReconciler{ Client: testEnv.Manager.GetClient(), APIReader: testEnv.Manager.GetAPIReader(), RateLimitWaitTime: 5 * time.Minute, - HCloudClientFactory: testEnv.HCloudClientFactory, TargetClusterManagersWaitGroup: &wg, - }).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) - - Expect((&HCloudMachineReconciler{ - Client: testEnv.Manager.GetClient(), - APIReader: testEnv.Manager.GetAPIReader(), - HCloudClientFactory: testEnv.HCloudClientFactory, - SSHClientFactory: testEnv.BaremetalSSHClientFactory, - }).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) - - Expect((&HCloudMachineTemplateReconciler{ - Client: testEnv.Manager.GetClient(), - APIReader: testEnv.Manager.GetAPIReader(), - HCloudClientFactory: testEnv.HCloudClientFactory, - }).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) - - Expect((&HetznerBareMetalHostReconciler{ - Client: testEnv.Manager.GetClient(), - APIReader: testEnv.Manager.GetAPIReader(), - RobotClientFactory: testEnv.RobotClientFactory, - SSHClientFactory: testEnv.BaremetalSSHClientFactory, - PreProvisionCommand: "dummy-pre-provision-command", - }).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) - - Expect((&HetznerBareMetalMachineReconciler{ - Client: testEnv.Manager.GetClient(), - APIReader: testEnv.Manager.GetAPIReader(), - HCloudClientFactory: testEnv.HCloudClientFactory, - }).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) - - Expect((&HCloudRemediationReconciler{ - Client: testEnv.Manager.GetClient(), - APIReader: testEnv.Manager.GetAPIReader(), - RateLimitWaitTime: 5 * time.Minute, - HCloudClientFactory: testEnv.HCloudClientFactory, - }).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) - - Expect((&HetznerBareMetalRemediationReconciler{ + } + Expect(hetznerClusterReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + resetter.HetznerClusterReconciler = hetznerClusterReconciler + + hcloudMachineReconciler := &HCloudMachineReconciler{ + Client: testEnv.Manager.GetClient(), + APIReader: testEnv.Manager.GetAPIReader(), + } + Expect(hcloudMachineReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + resetter.HCloudMachineReconciler = hcloudMachineReconciler + + hcloudMachineTemplateReconciler := &HCloudMachineTemplateReconciler{ + Client: testEnv.Manager.GetClient(), + APIReader: testEnv.Manager.GetAPIReader(), + } + Expect(hcloudMachineTemplateReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + resetter.HCloudMachineTemplateReconciler = hcloudMachineTemplateReconciler + + hetznerBareMetalHostReconciler := &HetznerBareMetalHostReconciler{ + Client: testEnv.Manager.GetClient(), + APIReader: testEnv.Manager.GetAPIReader(), + PreProvisionCommand: "dummy-pre-provision-command", + SSHAfterInstallImage: true, + } + Expect(hetznerBareMetalHostReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + resetter.HetznerBareMetalHostReconciler = hetznerBareMetalHostReconciler + + hetznerBareMetalMachineReconciler := &HetznerBareMetalMachineReconciler{ + Client: testEnv.Manager.GetClient(), + APIReader: testEnv.Manager.GetAPIReader(), + } + Expect(hetznerBareMetalMachineReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + resetter.HetznerBareMetalMachineReconciler = hetznerBareMetalMachineReconciler + + hcloudRemediationReconciler := &HCloudRemediationReconciler{ + Client: testEnv.Manager.GetClient(), + APIReader: testEnv.Manager.GetAPIReader(), + RateLimitWaitTime: 5 * time.Minute, + } + Expect(hcloudRemediationReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + resetter.HCloudRemediationReconciler = hcloudRemediationReconciler + + hetznerBareMetalRemediationReconciler := &HetznerBareMetalRemediationReconciler{ Client: testEnv.Manager.GetClient(), - }).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + } + Expect(hetznerBareMetalRemediationReconciler.SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed()) + resetter.HetznerBareMetalRemediationReconciler = hetznerBareMetalRemediationReconciler + + testEnv.Resetter = &resetter go func() { defer GinkgoRecover() diff --git a/controllers/hcloudmachine_controller.go b/controllers/hcloudmachine_controller.go index c5ac5dea7..a791abc06 100644 --- a/controllers/hcloudmachine_controller.go +++ b/controllers/hcloudmachine_controller.go @@ -62,6 +62,9 @@ type HCloudMachineReconciler struct { SSHClientFactory sshclient.Factory WatchFilterValue string ImageURLCommand string + + // Reconcile only this namespace. Only needed for testing + Namespace string } //+kubebuilder:rbac:groups="",resources=events,verbs=get;list;watch;create;update;patch @@ -75,6 +78,11 @@ type HCloudMachineReconciler struct { func (r *HCloudMachineReconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, reterr error) { log := ctrl.LoggerFrom(ctx) + if r.Namespace != "" && req.Namespace != r.Namespace { + // Just for testing, skip reconciling objects from finished tests. + return ctrl.Result{}, nil + } + // Fetch the HCloudMachine instance. hcloudMachine := &infrav1.HCloudMachine{} err := r.Get(ctx, req.NamespacedName, hcloudMachine) diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go index c4b501cd1..978f9ffb4 100644 --- a/controllers/hcloudmachine_controller_test.go +++ b/controllers/hcloudmachine_controller_test.go @@ -243,7 +243,7 @@ var _ = Describe("HCloudMachineReconciler", func() { hcloudClient.Reset() var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-reconciler") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-reconciler") Expect(err).NotTo(HaveOccurred()) capiCluster = &clusterv1.Cluster{ @@ -365,7 +365,7 @@ var _ = Describe("HCloudMachineReconciler", func() { }, timeout).Should(BeTrue()) }) - It("creates the HCloud machine in Hetzner 1 (flaky)", func() { + It("creates the HCloud machine in Hetzner 1", func() { By("checking that no servers exist") Eventually(func() bool { @@ -655,7 +655,7 @@ var _ = Describe("Hetzner secret", func() { BeforeEach(func() { hcloudClient.Reset() var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation") Expect(err).NotTo(HaveOccurred()) hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test") @@ -807,7 +807,7 @@ var _ = Describe("HCloudMachine validation", func() { hcloudClient.Reset() var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation") Expect(err).NotTo(HaveOccurred()) hcloudMachine = &infrav1.HCloudMachine{ diff --git a/controllers/hcloudmachinetemplate_controller.go b/controllers/hcloudmachinetemplate_controller.go index 08a089b73..e5a035970 100644 --- a/controllers/hcloudmachinetemplate_controller.go +++ b/controllers/hcloudmachinetemplate_controller.go @@ -49,6 +49,9 @@ type HCloudMachineTemplateReconciler struct { APIReader client.Reader HCloudClientFactory hcloudclient.Factory WatchFilterValue string + + // Reconcile only this namespace. Only needed for testing + Namespace string } // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=hcloudmachinetemplates,verbs=get;list;watch;create;update;patch;delete @@ -58,6 +61,11 @@ type HCloudMachineTemplateReconciler struct { func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req reconcile.Request) (_ reconcile.Result, reterr error) { log := ctrl.LoggerFrom(ctx) + if r.Namespace != "" && req.Namespace != r.Namespace { + // Just for testing, skip reconciling objects from finished tests. + return ctrl.Result{}, nil + } + machineTemplate := &infrav1.HCloudMachineTemplate{} if err := r.Get(ctx, req.NamespacedName, machineTemplate); err != nil { return reconcile.Result{}, client.IgnoreNotFound(err) diff --git a/controllers/hcloudmachinetemplate_controller_test.go b/controllers/hcloudmachinetemplate_controller_test.go index e07f5e6a5..82c1a55fb 100644 --- a/controllers/hcloudmachinetemplate_controller_test.go +++ b/controllers/hcloudmachinetemplate_controller_test.go @@ -40,7 +40,7 @@ var _ = Describe("HCloudMachineTemplateReconciler", func() { BeforeEach(func() { hcloudClient.Reset() var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachinetemplate-reconciler") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachinetemplate-reconciler") Expect(err).NotTo(HaveOccurred()) hetznerSecret = getDefaultHetznerSecret(testNs.Name) @@ -253,7 +253,7 @@ var _ = Describe("HCloudMachineTemplateReconciler", func() { ) BeforeEach(func() { var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation") Expect(err).NotTo(HaveOccurred()) hcloudMachineTemplate = &infrav1.HCloudMachineTemplate{ diff --git a/controllers/hcloudremediation_controller.go b/controllers/hcloudremediation_controller.go index dc2af1734..912eed7c1 100644 --- a/controllers/hcloudremediation_controller.go +++ b/controllers/hcloudremediation_controller.go @@ -49,6 +49,9 @@ type HCloudRemediationReconciler struct { APIReader client.Reader HCloudClientFactory hcloudclient.Factory WatchFilterValue string + + // Reconcile only this namespace. Only needed for testing + Namespace string } //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=hcloudremediations,verbs=get;list;watch;create;update;patch;delete @@ -60,6 +63,11 @@ type HCloudRemediationReconciler struct { func (r *HCloudRemediationReconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, reterr error) { log := ctrl.LoggerFrom(ctx) + if r.Namespace != "" && req.Namespace != r.Namespace { + // Just for testing, skip reconciling objects from finished tests. + return ctrl.Result{}, nil + } + hcloudRemediation := &infrav1.HCloudRemediation{} err := r.Get(ctx, req.NamespacedName, hcloudRemediation) if err != nil { diff --git a/controllers/hcloudremediation_controller_test.go b/controllers/hcloudremediation_controller_test.go index 65c8bc7cd..3561d3a6a 100644 --- a/controllers/hcloudremediation_controller_test.go +++ b/controllers/hcloudremediation_controller_test.go @@ -57,7 +57,7 @@ var _ = Describe("HCloudRemediationReconciler", func() { BeforeEach(func() { hcloudClient.Reset() var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachinetemplate-reconciler") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachinetemplate-reconciler") Expect(err).NotTo(HaveOccurred()) hetznerSecret = getDefaultHetznerSecret(testNs.Name) @@ -204,7 +204,7 @@ var _ = Describe("HCloudRemediationReconciler", func() { }) }) - It("checks that no remediation is tried if HCloud server does not exist anymore (flaky)", func() { + It("checks that no remediation is tried if HCloud server does not exist anymore", func() { By("ensuring if hcloudMachine is provisioned") Eventually(func() error { if err := testEnv.Get(ctx, hcloudMachineKey, hcloudMachine); err != nil { @@ -212,7 +212,7 @@ var _ = Describe("HCloudRemediationReconciler", func() { } if !hcloudMachine.Status.Ready { - return fmt.Errorf("hcloudMachine.Status.Ready is not true (yet) (flaky)") + return fmt.Errorf("hcloudMachine.Status.Ready is not true (yet)") } return nil }, timeout).ShouldNot(HaveOccurred()) diff --git a/controllers/hetznerbaremetalhost_controller.go b/controllers/hetznerbaremetalhost_controller.go index 74d2ca0d3..66634878c 100644 --- a/controllers/hetznerbaremetalhost_controller.go +++ b/controllers/hetznerbaremetalhost_controller.go @@ -63,6 +63,9 @@ type HetznerBareMetalHostReconciler struct { PreProvisionCommand string SSHAfterInstallImage bool ImageURLCommand string + + // Reconcile only this namespace. Only needed for testing + Namespace string } //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=hetznerbaremetalhosts,verbs=get;list;watch;create;update;patch;delete @@ -73,6 +76,11 @@ type HetznerBareMetalHostReconciler struct { func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, reterr error) { log := ctrl.LoggerFrom(ctx) + if r.Namespace != "" && req.Namespace != r.Namespace { + // Just for testing, skip reconciling objects from finished tests. + return ctrl.Result{}, nil + } + start := time.Now() defer func() { // check duration of reconcile. Warn if it took too long. @@ -116,6 +124,12 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl // Use uncached APIReader err := r.APIReader.Get(ctx, client.ObjectKeyFromObject(bmHost), apiserverHost) if err != nil { + if apierrors.IsNotFound(err) { + // resource was deleted. No need to reconcile again. + reterr = nil + res = reconcile.Result{} + return + } reterr = errors.Join(reterr, fmt.Errorf("failed get HetznerBareMetalHost via uncached APIReader: %w", err)) return diff --git a/controllers/hetznerbaremetalhost_controller_test.go b/controllers/hetznerbaremetalhost_controller_test.go index c01881b71..133b363fe 100644 --- a/controllers/hetznerbaremetalhost_controller_test.go +++ b/controllers/hetznerbaremetalhost_controller_test.go @@ -86,7 +86,7 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() { BeforeEach(func() { var err error - testNs, err = testEnv.CreateNamespace(ctx, "baremetalhost-reconciler") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "baremetalhost-reconciler") Expect(err).NotTo(HaveOccurred()) hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test") @@ -615,7 +615,7 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() { BeforeEach(func() { var err error - testNs, err = testEnv.CreateNamespace(ctx, "baremetalmachine-reconciler") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "baremetalmachine-reconciler") Expect(err).NotTo(HaveOccurred()) hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test") diff --git a/controllers/hetznerbaremetalmachine_controller.go b/controllers/hetznerbaremetalmachine_controller.go index 084ad06fa..14367f86e 100644 --- a/controllers/hetznerbaremetalmachine_controller.go +++ b/controllers/hetznerbaremetalmachine_controller.go @@ -53,6 +53,9 @@ type HetznerBareMetalMachineReconciler struct { RateLimitWaitTime time.Duration HCloudClientFactory hcloudclient.Factory WatchFilterValue string + + // Reconcile only this namespace. Only needed for testing + Namespace string } //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=hetznerbaremetalmachines,verbs=get;list;watch;create;update;patch;delete @@ -63,6 +66,11 @@ type HetznerBareMetalMachineReconciler struct { func (r *HetznerBareMetalMachineReconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, reterr error) { log := ctrl.LoggerFrom(ctx) + if r.Namespace != "" && req.Namespace != r.Namespace { + // Just for testing, skip reconciling objects from finished tests. + return ctrl.Result{}, nil + } + // Fetch the Hetzner bare metal instance. hbmMachine := &infrav1.HetznerBareMetalMachine{} err := r.Get(ctx, req.NamespacedName, hbmMachine) diff --git a/controllers/hetznerbaremetalmachine_controller_test.go b/controllers/hetznerbaremetalmachine_controller_test.go index 8dfc1afa9..c864d3b8e 100644 --- a/controllers/hetznerbaremetalmachine_controller_test.go +++ b/controllers/hetznerbaremetalmachine_controller_test.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "fmt" "testing" "time" @@ -74,7 +75,7 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() { BeforeEach(func() { var err error - testNs, err = testEnv.CreateNamespace(ctx, "baremetalmachine-reconciler") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "baremetalmachine-reconciler") Expect(err).NotTo(HaveOccurred()) hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test") @@ -389,17 +390,83 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() { }, timeout).Should(BeTrue()) }) - It("deletes successfully", func() { - By("deleting bm machine") + It("hbmm deletes successfully and host gets deprovisioned", func() { + By("Waiting until host is provisioned") + + Eventually(func() bool { + return isPresentAndTrue(key, bmMachine, infrav1.HostReadyCondition) + }, timeout).Should(BeTrue()) + + err := testEnv.Get(ctx, hostKey, host) + Expect(err).To(BeNil()) + Expect(host.Spec.Status.ProvisioningState).To(Equal(infrav1.StateProvisioned)) + + By("deleting hbmm") Expect(testEnv.Delete(ctx, bmMachine)).To(Succeed()) + Eventually(func() error { + err := testEnv.Get(ctx, key, bmMachine) + if apierrors.IsNotFound(err) { + return nil + } + return err + }, timeout, time.Second).Should(Succeed()) + + By("making sure the host has been deprovisioned") + Eventually(func() bool { - if err := testEnv.Get(ctx, key, bmMachine); apierrors.IsNotFound(err) { - return true + if err := testEnv.Get(ctx, hostKey, host); err != nil { + return false } - return false + return host.Spec.Status.ProvisioningState == infrav1.StateNone }, timeout, time.Second).Should(BeTrue()) + }) + + It("hbmm deletes successfully and host gets deprovisioned, even if state is 'ensure-provisioned'", func() { + By("checking that the host is ready") + Eventually(func() bool { + return isPresentAndTrue(key, bmMachine, infrav1.HostReadyCondition) + }, timeout).Should(BeTrue()) + + osSSHClient.On("GetHostName").Unset() + + osSSHClient.On("GetHostName").Return(sshclient.Output{ + StdOut: "some-unexpected-hostname", + StdErr: "", + Err: nil, + }) + + err := testEnv.Get(ctx, hostKey, host) + Expect(err).To(BeNil()) + Expect(host.Spec.Status.ProvisioningState).To(Equal(infrav1.StateProvisioned)) + + By("Setting State to 'ensure-provisioned'") + host.Spec.Status.ProvisioningState = infrav1.StateEnsureProvisioned + err = testEnv.Update(ctx, host) + Expect(err).To(BeNil()) + + Eventually(func() error { + err := testEnv.Get(ctx, hostKey, host) + if err != nil { + return err + } + if host.Spec.Status.ProvisioningState != infrav1.StateEnsureProvisioned { + return fmt.Errorf("ProvisioningState=%s", host.Spec.Status.ProvisioningState) + } + return nil + }, timeout, time.Second).Should(Succeed()) + + By("deleting bm machine") + Expect(testEnv.Delete(ctx, bmMachine)).To(Succeed()) + + Eventually(func() error { + err := testEnv.Get(ctx, key, bmMachine) + if apierrors.IsNotFound(err) { + return nil + } + return err + }, timeout, time.Second).Should(Succeed()) By("making sure the host has been deprovisioned") @@ -443,14 +510,17 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() { It("checks the hetznerBareMetalMachine status running phase", func() { By("making sure that machine is in running state") - Eventually(func() bool { + Eventually(func() error { if err := testEnv.Get(ctx, key, bmMachine); err != nil { - return false + return err } - - testEnv.GetLogger().Info("status of host and hetznerBareMetalMachine", "hetznerBareMetalMachine phase", bmMachine.Status.Phase, "host state", host.Spec.Status.ProvisioningState) - return bmMachine.Status.Phase == clusterv1.MachinePhaseRunning - }, timeout, time.Second).Should(BeTrue()) + testEnv.GetLogger().Info("status of host and hetznerBareMetalMachine", "hetznerBareMetalMachine phase", bmMachine.Status.Phase, + "hostState", host.Spec.Status.ProvisioningState) + if bmMachine.Status.Phase != clusterv1.MachinePhaseRunning { + return fmt.Errorf("bmMachine.Status.Phase should be MachinePhaseRunning, but is: %q", bmMachine.Status.Phase) + } + return nil + }, timeout, time.Second).Should(Succeed()) }) It("checks that HostReady condition is True for hetznerBareMetalMachine", func() { @@ -721,7 +791,7 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() { ) BeforeEach(func() { var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation") Expect(err).NotTo(HaveOccurred()) hbmmt = &infrav1.HetznerBareMetalMachineTemplate{ diff --git a/controllers/hetznerbaremetalremediation_controller.go b/controllers/hetznerbaremetalremediation_controller.go index cfd012cf8..72f3b210a 100644 --- a/controllers/hetznerbaremetalremediation_controller.go +++ b/controllers/hetznerbaremetalremediation_controller.go @@ -41,6 +41,9 @@ import ( type HetznerBareMetalRemediationReconciler struct { client.Client WatchFilterValue string + + // Reconcile only this namespace. Only needed for testing + Namespace string } //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=hetznerbaremetalremediations,verbs=get;list;watch;create;update;patch;delete @@ -52,6 +55,11 @@ type HetznerBareMetalRemediationReconciler struct { func (r *HetznerBareMetalRemediationReconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, reterr error) { log := ctrl.LoggerFrom(ctx) + if r.Namespace != "" && req.Namespace != r.Namespace { + // Just for testing, skip reconciling objects from finished tests. + return ctrl.Result{}, nil + } + // Fetch the Hetzner bare metal host instance. bareMetalRemediation := &infrav1.HetznerBareMetalRemediation{} err := r.Get(ctx, req.NamespacedName, bareMetalRemediation) diff --git a/controllers/hetznerbaremetalremediation_controller_test.go b/controllers/hetznerbaremetalremediation_controller_test.go index 206d671d6..c5a2cae3b 100644 --- a/controllers/hetznerbaremetalremediation_controller_test.go +++ b/controllers/hetznerbaremetalremediation_controller_test.go @@ -69,7 +69,7 @@ var _ = Describe("HetznerBareMetalRemediationReconciler", func() { BeforeEach(func() { var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachinetemplate-reconciler") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachinetemplate-reconciler") Expect(err).NotTo(HaveOccurred()) capiCluster = &clusterv1.Cluster{ @@ -312,7 +312,6 @@ var _ = Describe("HetznerBareMetalRemediationReconciler", func() { return false } - testEnv.GetLogger().Info("Provisioning state of host", "state", host.Spec.Status.ProvisioningState, "conditions", host.Spec.Status.Conditions) return host.Spec.Status.ProvisioningState == infrav1.StateProvisioned }, timeout).Should(BeTrue()) }) diff --git a/controllers/hetznercluster_controller.go b/controllers/hetznercluster_controller.go index a66f971fb..13d27b43e 100644 --- a/controllers/hetznercluster_controller.go +++ b/controllers/hetznercluster_controller.go @@ -79,6 +79,9 @@ type HetznerClusterReconciler struct { TargetClusterManagersWaitGroup *sync.WaitGroup WatchFilterValue string DisableCSRApproval bool + + // Reconcile only this namespace. Only needed for testing + Namespace string } //+kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch @@ -90,6 +93,11 @@ type HetznerClusterReconciler struct { func (r *HetznerClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, reterr error) { log := ctrl.LoggerFrom(ctx) + if r.Namespace != "" && req.Namespace != r.Namespace { + // Just for testing, skip reconciling objects from finished tests. + return ctrl.Result{}, nil + } + // Fetch the HetznerCluster instance hetznerCluster := &infrav1.HetznerCluster{} err := r.Get(ctx, req.NamespacedName, hetznerCluster) diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go index 0359f0eb7..d8c408603 100644 --- a/controllers/hetznercluster_controller_test.go +++ b/controllers/hetznercluster_controller_test.go @@ -271,7 +271,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() { ) BeforeEach(func() { hcloudClient.Reset() - testNs, err = testEnv.CreateNamespace(ctx, "cluster-tests") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "cluster-tests") Expect(err).NotTo(HaveOccurred()) namespace = testNs.Name @@ -334,7 +334,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() { }) Context("load balancer", func() { - It("should create load balancer and update it accordingly (flaky)", func() { + It("should create load balancer and update it accordingly", func() { Expect(testEnv.Create(ctx, instance)).To(Succeed()) Eventually(func() bool { @@ -530,7 +530,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() { }, timeout, time.Second).Should(BeTrue()) }) - It("should take over an existing load balancer with correct name (flaky)", func() { + It("should take over an existing load balancer with correct name", func() { By("creating load balancer manually") opts := hcloud.LoadBalancerCreateOpts{ @@ -576,7 +576,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() { "message", c.Message, ) return false - }, 2*timeout, time.Second).Should(BeTrue()) // flaky ? + }, 2*timeout, time.Second).Should(BeTrue()) By("checking that load balancer has label set") @@ -625,7 +625,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() { return false }, timeout, time.Second).Should(BeTrue()) - By("deleting the cluster and load balancer and testing that owned label is gone (flaky)") + By("deleting the cluster and load balancer and testing that owned label is gone") Expect(testEnv.Delete(ctx, instance)) @@ -633,7 +633,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() { loadBalancers, err := hcloudClient.ListLoadBalancers(ctx, hcloud.LoadBalancerListOpts{Name: lbName}) // there should always be one load balancer, if not, then this is a problem where we can immediately return Expect(err).To(BeNil()) - Expect(loadBalancers).To(HaveLen(1)) // flaky + Expect(loadBalancers).To(HaveLen(1)) _, found := loadBalancers[0].Labels[instance.ClusterTagKey()] return found @@ -965,7 +965,7 @@ var _ = Describe("Hetzner secret", func() { BeforeEach(func() { hcloudClient.Reset() var err error - testNs, err = testEnv.CreateNamespace(ctx, "hetzner-secret") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hetzner-secret") Expect(err).NotTo(HaveOccurred()) hetznerClusterName = utils.GenerateName(nil, "hetzner-cluster-test") @@ -1064,7 +1064,7 @@ var _ = Describe("HetznerCluster validation", func() { BeforeEach(func() { hcloudClient.Reset() var err error - testNs, err = testEnv.CreateNamespace(ctx, "hcloudmachine-validation") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "hcloudmachine-validation") Expect(err).NotTo(HaveOccurred()) }) AfterEach(func() { diff --git a/pkg/services/hcloud/server/server_suite_test.go b/pkg/services/hcloud/server/server_suite_test.go index cb8109dac..8828dc6c3 100644 --- a/pkg/services/hcloud/server/server_suite_test.go +++ b/pkg/services/hcloud/server/server_suite_test.go @@ -33,7 +33,9 @@ import ( infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" "github.com/syself/cluster-api-provider-hetzner/pkg/scope" + sshmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/ssh" hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client" + "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/mockedsshclient" "github.com/syself/cluster-api-provider-hetzner/test/helpers" ) @@ -203,13 +205,27 @@ func TestServer(t *testing.T) { RunSpecs(t, "Server Suite") } +type Resetter struct{} + +var _ helpers.Resetter = &Resetter{} + +func (r *Resetter) Reset(_ string, testEnv *helpers.TestEnvironment, t FullGinkgoTInterface) { + rescueSSHClient := &sshmock.Client{} + rescueSSHClient.Test(t) + testEnv.RescueSSHClient = rescueSSHClient + + testEnv.HCloudSSHClient = &sshmock.Client{} + testEnv.HCloudSSHClient.Test(t) + testEnv.HCloudSSHClientFactory = mockedsshclient.NewSSHFactory(testEnv.HCloudSSHClient) +} + var _ = BeforeSuite(func() { utilruntime.Must(corev1.AddToScheme(scheme.Scheme)) utilruntime.Must(infrav1.AddToScheme(scheme.Scheme)) utilruntime.Must(clusterv1.AddToScheme(scheme.Scheme)) testEnv = helpers.NewTestEnvironment() - + testEnv.Resetter = &Resetter{} go func() { defer GinkgoRecover() Expect(testEnv.StartManager(ctx)).To(Succeed()) diff --git a/pkg/services/hcloud/server/server_test.go b/pkg/services/hcloud/server/server_test.go index f2519b495..3cf2720f6 100644 --- a/pkg/services/hcloud/server/server_test.go +++ b/pkg/services/hcloud/server/server_test.go @@ -101,6 +101,7 @@ var _ = DescribeTable("createLabels", ClusterScope: scope.ClusterScope{ HetznerCluster: &hetznerCluster, }, + SSHClientFactory: testEnv.HCloudSSHClientFactory, }, } Expect(service.createLabels()).To(Equal(tc.expectedOutput)) @@ -362,6 +363,7 @@ var _ = Describe("getSSHKeys", func() { Namespace: "default", }, }, + SSHClientFactory: testEnv.HCloudSSHClientFactory, }, } }) @@ -587,7 +589,7 @@ var _ = Describe("Reconcile", func() { BeforeEach(func() { hcloudClient = mocks.NewClient(GinkgoT()) - testNs, err = testEnv.CreateNamespace(ctx, "server-reconcile") + testNs, err = testEnv.ResetAndCreateNamespace(ctx, "server-reconcile") Expect(err).To(BeNil()) clusterScope, err := scope.NewClusterScope(scope.ClusterScopeParams{ @@ -675,7 +677,7 @@ var _ = Describe("Reconcile", func() { }, }, }, - SSHClientFactory: testEnv.BaremetalSSHClientFactory, + SSHClientFactory: testEnv.HCloudSSHClientFactory, }, } }) @@ -951,7 +953,7 @@ var _ = Describe("Reconcile", func() { Status: hcloud.ServerStatusRunning, }, nil).Once() - testEnv.RescueSSHClient.On("Reboot").Return(sshclient.Output{ + testEnv.HCloudSSHClient.On("Reboot").Return(sshclient.Output{ Err: nil, StdOut: "ok", StdErr: "", @@ -969,12 +971,12 @@ var _ = Describe("Reconcile", func() { Name: "hcloudmachinenameWithRescueEnabled", Status: hcloud.ServerStatusRunning, }, nil).Once() - testEnv.RescueSSHClient.On("GetHostName").Return(sshclient.Output{ + testEnv.HCloudSSHClient.On("GetHostName").Return(sshclient.Output{ StdOut: "rescue", StdErr: "", Err: nil, }) - startImageURLCommandMock := testEnv.RescueSSHClient.On("StartImageURLCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything, "hcloudmachinename", []string{"sda"}).Return(0, "", nil) + startImageURLCommandMock := testEnv.HCloudSSHClient.On("StartImageURLCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything, "hcloudmachinename", []string{"sda"}).Return(0, "", nil) _, err = service.Reconcile(ctx) Expect(err).To(BeNil()) Expect(service.scope.HCloudMachine.Status.FailureReason).To(BeNil()) @@ -984,12 +986,12 @@ var _ = Describe("Reconcile", func() { startImageURLCommandMock.Parent.AssertNumberOfCalls(GinkgoT(), "StartImageURLCommand", 1) By("reconcile again --------------------------------------------------------") - testEnv.RescueSSHClient.On("GetHostName").Return(sshclient.Output{ + testEnv.HCloudSSHClient.On("GetHostName").Return(sshclient.Output{ StdOut: "rescue", StdErr: "", Err: nil, }) - testEnv.RescueSSHClient.On("StateOfImageURLCommand").Return(sshclient.ImageURLCommandStateFinishedSuccessfully, "output-of-image-url-command", nil) + testEnv.HCloudSSHClient.On("StateOfImageURLCommand").Return(sshclient.ImageURLCommandStateFinishedSuccessfully, "output-of-image-url-command", nil) hcloudClient.On("GetServer", mock.Anything, mock.Anything).Return(&hcloud.Server{ ID: 1, Name: "hcloudmachinenameWithRescueEnabled", diff --git a/test/helpers/envtest.go b/test/helpers/envtest.go index 17015fcee..7debc7acb 100644 --- a/test/helpers/envtest.go +++ b/test/helpers/envtest.go @@ -52,14 +52,12 @@ import ( infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" secretutil "github.com/syself/cluster-api-provider-hetzner/pkg/secrets" - "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks" robotmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/robot" sshmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/ssh" robotclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/robot" sshclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/ssh" hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client" - fakeclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client/fake" - "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/mockedsshclient" + fakehcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client/fake" ) func init() { @@ -106,6 +104,19 @@ func init() { } } +// Resetter provides an interface, so that test-suites using TestEnvironment can define their custom +// resetting. Mocks get used in two places: In the real code (like FooReconciler) and during the +// tests (for FooMock.On("....")). The Resetter contains the pointer to the places where mocks need +// to be updated on Reset(). +type Resetter interface { + // Reset resets the objects shared between tests. This avoids that changes done by the first + // test will modify the environment of the second test. Testify Mocks should be initialized with + // fooMock.Test(t), to ensure failures are propagated to the corresponding test. Otherwise + // errors of mocks will panic on usage, and finding the corresponding test to the panic is + // sometimes not trivial. + Reset(namespace string, testEnv *TestEnvironment, t g.FullGinkgoTInterface) +} + // TestEnvironment encapsulates a Kubernetes local test environment. type TestEnvironment struct { ctrl.Manager @@ -115,12 +126,14 @@ type TestEnvironment struct { RobotClientFactory robotclient.Factory BaremetalSSHClientFactory sshclient.Factory HCloudSSHClientFactory sshclient.Factory + HCloudSSHClient *sshmock.Client RescueSSHClient *sshmock.Client OSSSHClientAfterInstallImage *sshmock.Client OSSSHClientAfterCloudInit *sshmock.Client RobotClient *robotmock.Client cancel context.CancelFunc RateLimitWaitTime time.Duration + Resetter Resetter } // NewTestEnvironment creates a new environment spinning up a local api-server. @@ -191,29 +204,15 @@ func NewTestEnvironment() *TestEnvironment { klog.Fatalf("failed to set up webhook with manager for HCloudRemediationTemplate: %s", err) } // Create a fake HCloudClientFactory - hcloudClientFactory := fakeclient.NewHCloudClientFactory() - - rescueSSHClient := &sshmock.Client{} - osSSHClientAfterInstallImage := &sshmock.Client{} - osSSHClientAfterCloudInit := &sshmock.Client{} - - robotClient := &robotmock.Client{} - - hcloudSSHClient := &sshmock.Client{} + hcloudClientFactory := fakehcloudclient.NewHCloudClientFactory() return &TestEnvironment{ - Manager: mgr, - Client: mgr.GetClient(), - Config: mgr.GetConfig(), - HCloudClientFactory: hcloudClientFactory, - BaremetalSSHClientFactory: mocks.NewSSHFactory(rescueSSHClient, osSSHClientAfterInstallImage, osSSHClientAfterCloudInit), - HCloudSSHClientFactory: mockedsshclient.NewSSHFactory(hcloudSSHClient), - RescueSSHClient: rescueSSHClient, - OSSSHClientAfterInstallImage: osSSHClientAfterInstallImage, - OSSSHClientAfterCloudInit: osSSHClientAfterCloudInit, - RobotClientFactory: mocks.NewRobotFactory(robotClient), - RobotClient: robotClient, - RateLimitWaitTime: 5 * time.Minute, + Manager: mgr, + Client: mgr.GetClient(), + Config: mgr.GetConfig(), + HCloudClientFactory: hcloudClientFactory, + + RateLimitWaitTime: 5 * time.Minute, } } @@ -246,8 +245,8 @@ func (t *TestEnvironment) Cleanup(ctx context.Context, objs ...client.Object) er return kerrors.NewAggregate(errs) } -// CreateNamespace creates a namespace. -func (t *TestEnvironment) CreateNamespace(ctx context.Context, generateName string) (*corev1.Namespace, error) { +// ResetAndCreateNamespace creates a namespace. +func (t *TestEnvironment) ResetAndCreateNamespace(ctx context.Context, generateName string) (*corev1.Namespace, error) { ns := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ GenerateName: fmt.Sprintf("%s-", generateName), @@ -260,6 +259,10 @@ func (t *TestEnvironment) CreateNamespace(ctx context.Context, generateName stri return nil, err } + if t.Resetter != nil { + t.Resetter.Reset(ns.Name, t, g.GinkgoT()) + } + return ns, nil }