Skip to content

Commit

Permalink
Merge pull request #909 from syself/refactor-controller-tests
Browse files Browse the repository at this point in the history 
🌱 Refactor controller tests and fake HCloud client
  • Loading branch information
@janiskemper
janiskemper authored Sep 13, 2023
2 parents 0fb5184 + 325b708 commit 25ec6e7
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 111 deletions.
44 changes: 21 additions & 23 deletions controllers/hcloudmachinetemplate_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package controllers

import (
"fmt"
"time"

. "github.com/onsi/ginkgo/v2"
Expand All @@ -34,9 +33,9 @@ import (

var _ = Describe("HCloudMachineTemplateReconciler", func() {
var (
capiCluster *clusterv1.Cluster
infraCluster *infrav1.HetznerCluster
capiCluster *clusterv1.Cluster

hetznerCluster *infrav1.HetznerCluster
machineTemplate *infrav1.HCloudMachineTemplate

testNs *corev1.Namespace
Expand All @@ -53,27 +52,25 @@ var _ = Describe("HCloudMachineTemplateReconciler", func() {

capiCluster = &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test1-",
GenerateName: "test-",
Namespace: testNs.Name,
Finalizers: []string{clusterv1.ClusterFinalizer},
},
Spec: clusterv1.ClusterSpec{
InfrastructureRef: &corev1.ObjectReference{
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
Kind: "HetznerCluster",
Name: "hetzner-test1",
Name: "hetzner-test",
Namespace: testNs.Name,
},
},
Status: clusterv1.ClusterStatus{
InfrastructureReady: true,
},
}

Expect(testEnv.Create(ctx, capiCluster)).To(Succeed())

infraCluster = &infrav1.HetznerCluster{
hetznerCluster = &infrav1.HetznerCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "hetzner-test1",
Name: "hetzner-test",
Namespace: testNs.Name,
OwnerReferences: []metav1.OwnerReference{
{
Expand All @@ -87,13 +84,13 @@ var _ = Describe("HCloudMachineTemplateReconciler", func() {
Spec: getDefaultHetznerClusterSpec(),
}

Expect(testEnv.Create(ctx, infraCluster)).To(Succeed())
Expect(testEnv.Create(ctx, hetznerCluster)).To(Succeed())

machineTemplate = &infrav1.HCloudMachineTemplate{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "hcloud-machine-template-",
Namespace: testNs.Name,
OwnerReferences: infraCluster.OwnerReferences,
OwnerReferences: hetznerCluster.OwnerReferences,
},
Spec: infrav1.HCloudMachineTemplateSpec{
Template: infrav1.HCloudMachineTemplateResource{
Expand All @@ -114,43 +111,44 @@ var _ = Describe("HCloudMachineTemplateReconciler", func() {
})

AfterEach(func() {
Expect(testEnv.Cleanup(ctx, testNs, capiCluster, infraCluster,
Expect(testEnv.Cleanup(ctx, testNs, capiCluster, hetznerCluster,
machineTemplate, hetznerSecret)).To(Succeed())
})

Context("Basic test", func() {
It("sets the capacity in status", func() {
Eventually(func() bool {
if err := testEnv.Get(ctx, key, machineTemplate); err != nil {
fmt.Printf("Did not find machine template: %s\n", err)
testEnv.GetLogger().Error(err, "did not find machine template")
return false
}

fmt.Printf("Capacity: %v\n", machineTemplate.Status.Capacity)

// If capacity is not set (yet), there is nothing to compare
if machineTemplate.Status.Capacity.Cpu() == nil || machineTemplate.Status.Capacity.Memory() == nil {
fmt.Printf("Capacity not set: %v\n", machineTemplate.Status.Capacity)
testEnv.GetLogger().Info("capacity not set", "capacity", machineTemplate.Status.Capacity)
return false
}

// Compare CPU
// compare CPU
expectedCPU, err := machinetemplate.GetCPUQuantityFromInt(fake.DefaultCPUCores)
Expect(err).To(Succeed())

if !expectedCPU.Equal(*machineTemplate.Status.Capacity.Cpu()) {
fmt.Printf("CPU did not equal: Expected: %v. Actual: %v\n", expectedCPU, machineTemplate.Status.Capacity.Cpu())
testEnv.GetLogger().Info("cpu did not equal", "expected", expectedCPU, "actual", machineTemplate.Status.Capacity.Cpu())
return false
}

// Compare memory
// compare memory
expectedMemory, err := machinetemplate.GetMemoryQuantityFromFloat32(fake.DefaultMemoryInGB)
Expect(err).To(Succeed())

if !expectedMemory.Equal(*machineTemplate.Status.Capacity.Memory()) {
fmt.Printf("Memory did not equal: Expected: %v. Actual: %v\n", expectedMemory, machineTemplate.Status.Capacity.Memory())
testEnv.GetLogger().Info("memory did not equal", "expected", expectedMemory, "actual", machineTemplate.Status.Capacity.Memory())
return false
}
return expectedMemory.Equal(*machineTemplate.Status.Capacity.Memory())
}, 10*time.Second, time.Second).Should(BeTrue())

return true
}, timeout, time.Second).Should(BeTrue())
})
})
})
83 changes: 17 additions & 66 deletions controllers/hetznerbaremetalhost_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/syself/hrobot-go/models"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
Expand Down Expand Up @@ -59,8 +60,9 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
host *infrav1.HetznerBareMetalHost
bmMachine *infrav1.HetznerBareMetalMachine
hetznerCluster *infrav1.HetznerCluster
capiCluster *clusterv1.Cluster
capiMachine *clusterv1.Machine

capiCluster *clusterv1.Cluster
capiMachine *clusterv1.Machine

hetznerClusterName string

Expand Down Expand Up @@ -100,15 +102,6 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
Namespace: testNs.Name,
},
},
Status: clusterv1.ClusterStatus{
InfrastructureReady: true,
Conditions: clusterv1.Conditions{
{
Reason: "reason",
Message: "message",
},
},
},
}
Expect(testEnv.Create(ctx, capiCluster)).To(Succeed())

Expand Down Expand Up @@ -164,6 +157,7 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
robotClient.On("DeleteBootRescue", 1).Return(&models.Rescue{Active: false}, nil)
robotClient.On("RebootBMServer", mock.Anything, mock.Anything).Return(&models.ResetPost{}, nil)
robotClient.On("SetBMServerName", 1, mock.Anything).Return(nil, nil)

configureRescueSSHClient(rescueSSHClient)

osSSHClientAfterInstallImage.On("Reboot").Return(sshclient.Output{})
Expand Down Expand Up @@ -209,21 +203,17 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
})

It("creates the host machine", func() {
defer func() {
Expect(testEnv.Delete(ctx, host)).To(Succeed())
}()
Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
}
return true
}, timeout).Should(BeTrue())

Expect(testEnv.Delete(ctx, host)).To(Succeed())
})

It("sets the finalizer", func() {
defer func() {
Expect(testEnv.Delete(ctx, host)).To(Succeed())
}()
Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
Expand All @@ -235,18 +225,17 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
}
return false
}, timeout).Should(BeTrue())

Expect(testEnv.Delete(ctx, host)).To(Succeed())
})

It("deletes successfully", func() {
// Delete the host object
By("deleting the host object")
Expect(testEnv.Delete(ctx, host)).To(Succeed())

// Make sure the it has been deleted
By("making sure the it has been deleted")
Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return true
}
return false
return apierrors.IsNotFound(testEnv.Get(ctx, key, host))
}, timeout, time.Second).Should(BeTrue())
})
})
Expand Down Expand Up @@ -330,10 +319,14 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
It("reaches the state image installing", func() {
ph, err := patch.NewHelper(host, testEnv)
Expect(err).ShouldNot(HaveOccurred())

host.Spec.RootDeviceHints = &infrav1.RootDeviceHints{
WWN: helpers.DefaultWWN,
}
Expect(ph.Patch(ctx, host, patch.WithStatusObservedGeneration{})).To(Succeed())

Eventually(func() error {
return ph.Patch(ctx, host, patch.WithStatusObservedGeneration{})
}, timeout, time.Second).Should(BeNil())

Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
Expand Down Expand Up @@ -365,13 +358,6 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
})

It("gets selected from a bm machine and provisions", func() {
defer func() {
Expect(testEnv.Delete(ctx, bmMachine))
}()

var machine clusterv1.Machine
Expect(testEnv.Get(ctx, client.ObjectKey{Namespace: testNs.Name, Name: capiMachine.Name}, &machine)).To(Succeed())

Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
Expand Down Expand Up @@ -403,13 +389,6 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
})

It("gets selected from a bm machine and provisions", func() {
defer func() {
Expect(testEnv.Delete(ctx, bmMachine))
}()

var machine clusterv1.Machine
Expect(testEnv.Get(ctx, client.ObjectKey{Namespace: testNs.Name, Name: capiMachine.Name}, &machine)).To(Succeed())

Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
Expand Down Expand Up @@ -530,13 +509,6 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
})

It("gets selected from a bm machine and provisions", func() {
defer func() {
Expect(testEnv.Delete(ctx, bmMachine))
}()

var machine clusterv1.Machine
Expect(testEnv.Get(ctx, client.ObjectKey{Namespace: testNs.Name, Name: capiMachine.Name}, &machine)).To(Succeed())

Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
Expand Down Expand Up @@ -595,15 +567,6 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
Namespace: testNs.Name,
},
},
Status: clusterv1.ClusterStatus{
InfrastructureReady: true,
Conditions: clusterv1.Conditions{
{
Reason: "reason",
Message: "message",
},
},
},
}
Expect(testEnv.Create(ctx, capiCluster)).To(Succeed())

Expand Down Expand Up @@ -722,9 +685,6 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {

It("gives an error", func() {
Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
}
return isPresentAndFalseWithReason(key, host, infrav1.CredentialsAvailableCondition, infrav1.RescueSSHSecretMissingReason)
}, timeout).Should(BeTrue())
})
Expand All @@ -747,9 +707,6 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
}()

Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
}
return isPresentAndFalseWithReason(key, host, infrav1.CredentialsAvailableCondition, infrav1.SSHCredentialsInSecretInvalidReason)
}, timeout).Should(BeTrue())
})
Expand Down Expand Up @@ -780,9 +737,6 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {

It("gives the right error if secret is missing", func() {
Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
}
return isPresentAndFalseWithReason(key, host, infrav1.CredentialsAvailableCondition, infrav1.OSSSHSecretMissingReason)
}, timeout).Should(BeTrue())
})
Expand All @@ -805,9 +759,6 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
}()

Eventually(func() bool {
if err := testEnv.Get(ctx, key, host); err != nil {
return false
}
return isPresentAndFalseWithReason(key, host, infrav1.CredentialsAvailableCondition, infrav1.SSHCredentialsInSecretInvalidReason)
}, timeout).Should(BeTrue())
})
Expand Down
Loading

0 comments on commit 25ec6e7

Please sign in to comment.