Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Controller Unit Tests #118

Merged
merged 5 commits into from
Feb 19, 2024
Merged

Refactor Controller Unit Tests #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c20657
Refactor nmo controller unit test and suite
razo7 Feb 7, 2024
e4527ee
Refactor unit test for initMaintenanceStatus and exclude remediation …
razo7 Feb 7, 2024
c068b59
Refactor unit tests for reconciliation behavior
razo7 Feb 7, 2024
199b494
Fixing remarks for clearer test readability
razo7 Feb 8, 2024
b3ced39
Pass context in tests
razo7 Feb 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 32 additions & 33 deletions controllers/suite_test.go → controllers/controllers_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"path/filepath"
"testing"

"github.com/medik8s/common/pkg/lease"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

Expand All @@ -31,7 +32,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/envtest"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
metricsServer "sigs.k8s.io/controller-runtime/pkg/metrics/server"

nodemaintenancev1beta1 "github.com/medik8s/node-maintenance-operator/api/v1beta1"
//+kubebuilder:scaffold:imports
Expand All @@ -40,10 +40,14 @@ import (
// These tests use Ginkgo (BDD-style Go testing framework). Refer to
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.

var cfg *rest.Config
var k8sClient client.Client
var testEnv *envtest.Environment
var ctxFromSignalHandler context.Context
var (
cfg *rest.Config
k8sClient client.Client
testEnv *envtest.Environment
ctx context.Context
cancel context.CancelFunc
r *NodeMaintenanceReconciler
)

func TestControllers(t *testing.T) {
RegisterFailHandler(Fail)
Expand All @@ -55,9 +59,6 @@ var _ = BeforeSuite(func() {

// call start or refactor when moving to "normal" testEnv test

})

func startTestEnv() {
By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
Expand All @@ -74,40 +75,38 @@ func startTestEnv() {
Expect(err).NotTo(HaveOccurred())

//+kubebuilder:scaffold:scheme
k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{Scheme: scheme.Scheme})
Expect(err).NotTo(HaveOccurred())
Expect(k8sManager).NotTo(BeNil())

k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
Expect(err).NotTo(HaveOccurred())
Expect(k8sClient).NotTo(BeNil())

k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
Scheme: scheme.Scheme,
Metrics: metricsServer.Options{BindAddress: "0"},
})
Expect(err).ToNot(HaveOccurred())
mockManager, _ := lease.NewManager(k8sClient, "")
// Create a ReconcileNodeMaintenance object with the scheme and fake client
r = &NodeMaintenanceReconciler{
Client: k8sClient,
Scheme: scheme.Scheme,
LeaseManager: &mockLeaseManager{mockManager},
logger: ctrl.Log.WithName("unit test"),
}
Expect(initDrainer(r, cfg)).To(Succeed())
// in test pods are not evicted, so don't wait forever for them
r.drainer.SkipWaitForDeleteTimeoutSeconds = 0

// comment in when moving to "normal" testEnv test
// this isn't needed atm, because the controller tests call relevant funcs of the controller themself
//err = (&NodeMaintenanceReconciler{
// Client: k8sManager.GetClient(),
// Scheme: k8sManager.GetScheme(),
//}).SetupWithManager(k8sManager)
//Expect(err).ToNot(HaveOccurred())
err = (r).SetupWithManager(k8sManager)
Expect(err).NotTo(HaveOccurred())

go func() {
if ctxFromSignalHandler == nil {
ctxFromSignalHandler = ctrl.SetupSignalHandler()
}
err = k8sManager.Start(ctxFromSignalHandler)
Expect(err).ToNot(HaveOccurred())
// https://github.com/kubernetes-sigs/controller-runtime/issues/1571
ctx, cancel = context.WithCancel(ctrl.SetupSignalHandler())
Expect(k8sManager.Start(ctx)).To(Succeed())
}()
}

var _ = AfterSuite(func() {
// call stop or refactor when moving to "normal" testEnv test
})

func stopTestEnv() {
var _ = AfterSuite(func() {
By("tearing down the test environment")
err := testEnv.Stop()
Expect(err).NotTo(HaveOccurred())
}
cancel()
Expect(testEnv.Stop()).To(Succeed())
})
36 changes: 18 additions & 18 deletions controllers/nodemaintenance_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return reconcile.Result{}, nil
}

err = r.initMaintenanceStatus(instance)
err = initMaintenanceStatus(instance, r.drainer, r.Client)
if err != nil {
r.logger.Error(err, "Failed to update NodeMaintenance with \"Running\" status")
return r.onReconcileError(instance, err)
Expand All @@ -152,7 +152,7 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return r.onReconcileError(instance, err)
}

r.setOwnerRefToNode(instance, node)
setOwnerRefToNode(instance, node, r.logger)

updateOwnedLeaseFailed, err := r.obtainLease(node)
if err != nil && updateOwnedLeaseFailed {
Expand Down Expand Up @@ -180,7 +180,7 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
}
}
//Add exclude from remediation label
if err := r.addExcludeRemediationLabel(ctx, node); err != nil {
if err := addExcludeRemediationLabel(ctx, node, r.Client, r.logger); err != nil {
return r.onReconcileError(instance, err)
}

Expand Down Expand Up @@ -285,15 +285,15 @@ func initDrainer(r *NodeMaintenanceReconciler, config *rest.Config) error {
return nil
}

func (r *NodeMaintenanceReconciler) setOwnerRefToNode(instance *nodemaintenancev1beta1.NodeMaintenance, node *corev1.Node) {
func setOwnerRefToNode(nm *nodemaintenancev1beta1.NodeMaintenance, node *corev1.Node, log logr.Logger) {

for _, ref := range instance.ObjectMeta.GetOwnerReferences() {
for _, ref := range nm.ObjectMeta.GetOwnerReferences() {
if ref.APIVersion == node.TypeMeta.APIVersion && ref.Kind == node.TypeMeta.Kind && ref.Name == node.ObjectMeta.GetName() && ref.UID == node.ObjectMeta.GetUID() {
return
}
}

r.logger.Info("setting owner ref to node")
log.Info("setting owner ref to node")

nodeMeta := node.TypeMeta
ref := metav1.OwnerReference{
Expand All @@ -305,7 +305,7 @@ func (r *NodeMaintenanceReconciler) setOwnerRefToNode(instance *nodemaintenancev
Controller: pointer.Bool(false),
}

instance.ObjectMeta.SetOwnerReferences(append(instance.ObjectMeta.GetOwnerReferences(), ref))
nm.ObjectMeta.SetOwnerReferences(append(nm.ObjectMeta.GetOwnerReferences(), ref))
}

func (r *NodeMaintenanceReconciler) obtainLease(node *corev1.Node) (bool, error) {
Expand All @@ -320,28 +320,28 @@ func (r *NodeMaintenanceReconciler) obtainLease(node *corev1.Node) (bool, error)
return false, nil
}

func (r *NodeMaintenanceReconciler) addExcludeRemediationLabel(ctx context.Context, node *corev1.Node) error {
func addExcludeRemediationLabel(ctx context.Context, node *corev1.Node, r client.Client, log logr.Logger) error {
if node.Labels[labels.ExcludeFromRemediation] != "true" {
patch := client.MergeFrom(node.DeepCopy())
if node.Labels == nil {
node.Labels = map[string]string{labels.ExcludeFromRemediation: "true"}
} else if node.Labels[labels.ExcludeFromRemediation] != "true" {
node.Labels[labels.ExcludeFromRemediation] = "true"
}
if err := r.Client.Patch(ctx, node, patch); err != nil {
r.logger.Error(err, "Failed to add exclude from remediation label from the node", "node name", node.Name)
if err := r.Patch(ctx, node, patch); err != nil {
log.Error(err, "Failed to add exclude from remediation label from the node", "node name", node.Name)
return err
}
}
return nil
}

func (r *NodeMaintenanceReconciler) removeExcludeRemediationLabel(ctx context.Context, node *corev1.Node) error {
func removeExcludeRemediationLabel(ctx context.Context, node *corev1.Node, r client.Client, log logr.Logger) error {
if node.Labels[labels.ExcludeFromRemediation] == "true" {
patch := client.MergeFrom(node.DeepCopy())
delete(node.Labels, labels.ExcludeFromRemediation)
if err := r.Client.Patch(ctx, node, patch); err != nil {
r.logger.Error(err, "Failed to remove exclude from remediation label from the node", "node name", node.Name)
if err := r.Patch(ctx, node, patch); err != nil {
log.Error(err, "Failed to remove exclude from remediation label from the node", "node name", node.Name)
return err
}
}
Expand All @@ -362,7 +362,7 @@ func (r *NodeMaintenanceReconciler) stopNodeMaintenanceImp(ctx context.Context,
if err := r.LeaseManager.InvalidateLease(ctx, node); err != nil {
return err
}
return r.removeExcludeRemediationLabel(ctx, node)
return removeExcludeRemediationLabel(ctx, node, r.Client, r.logger)
}

func (r *NodeMaintenanceReconciler) stopNodeMaintenanceOnDeletion(ctx context.Context, nodeName string) error {
Expand Down Expand Up @@ -392,11 +392,11 @@ func (r *NodeMaintenanceReconciler) fetchNode(nodeName string) (*corev1.Node, er
return node, nil
}

func (r *NodeMaintenanceReconciler) initMaintenanceStatus(nm *nodemaintenancev1beta1.NodeMaintenance) error {
func initMaintenanceStatus(nm *nodemaintenancev1beta1.NodeMaintenance, drainer *drain.Helper, r client.Client) error {
if nm.Status.Phase == "" {
nm.Status.Phase = nodemaintenancev1beta1.MaintenanceRunning
setLastUpdate(nm)
pendingList, errlist := r.drainer.GetPodsForDeletion(nm.Spec.NodeName)
pendingList, errlist := drainer.GetPodsForDeletion(nm.Spec.NodeName)
if errlist != nil {
return fmt.Errorf("failed to get pods for eviction while initializing status")
}
Expand All @@ -405,7 +405,7 @@ func (r *NodeMaintenanceReconciler) initMaintenanceStatus(nm *nodemaintenancev1b
}
nm.Status.EvictionPods = len(nm.Status.PendingPods)

podlist, err := r.drainer.Client.CoreV1().Pods(metav1.NamespaceAll).List(
podlist, err := drainer.Client.CoreV1().Pods(metav1.NamespaceAll).List(
context.Background(),
metav1.ListOptions{
FieldSelector: fields.SelectorFromSet(fields.Set{"spec.nodeName": nm.Spec.NodeName}).String(),
Expand All @@ -414,7 +414,7 @@ func (r *NodeMaintenanceReconciler) initMaintenanceStatus(nm *nodemaintenancev1b
return err
}
nm.Status.TotalPods = len(podlist.Items)
err = r.Client.Status().Update(context.TODO(), nm)
err = r.Status().Update(context.TODO(), nm)
return err
}
return nil
Expand Down
Loading
Loading