Skip to content

Commit

Permalink
Separate nm status initalization from update
Browse files Browse the repository at this point in the history
Separation of initMaintenanceStatus logic to only  nm status initalization and update is done afterwards. It is mostly needed for UTs of the initMaintenanceStatus fucntion
  • Loading branch information
razo7 committed Feb 28, 2024
1 parent c9582b6 commit 80b4e31
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
8 changes: 5 additions & 3 deletions controllers/nodemaintenance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,11 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return emptyResult, nil
}

err = initMaintenanceStatus(ctx, nm, drainer, r.Client)
err = initMaintenanceStatus(ctx, nm, drainer)
if err != nil {
if errUpdate := r.Client.Status().Update(ctx, nm); errUpdate != nil {
err = fmt.Errorf("errUpdate %w - %w", errUpdate, err)
}
r.logger.Error(err, "Failed to update NodeMaintenance with \"Running\" status")
return r.onReconcileError(ctx, nm, drainer, err)
}
Expand Down Expand Up @@ -390,7 +393,7 @@ func (r *NodeMaintenanceReconciler) fetchNode(ctx context.Context, drainer *drai
return node, nil
}

func initMaintenanceStatus(ctx context.Context, nm *v1beta1.NodeMaintenance, drainer *drain.Helper, r client.Client) error {
func initMaintenanceStatus(ctx context.Context, nm *v1beta1.NodeMaintenance, drainer *drain.Helper) error {
if nm.Status.Phase == "" {
nm.Status.Phase = v1beta1.MaintenanceRunning
setLastUpdate(nm)
Expand All @@ -412,7 +415,6 @@ func initMaintenanceStatus(ctx context.Context, nm *v1beta1.NodeMaintenance, dra
return err
}
nm.Status.TotalPods = len(podlist.Items)
err = r.Status().Update(ctx, nm)
return err
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions controllers/nodemaintenance_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ var _ = Describe("Node Maintenance", func() {
})
When("Status was initalized", func() {
It("should be set for running with 2 pods to drain", func() {
Expect(initMaintenanceStatus(ctx, nm, drainer, r.Client)).To(HaveOccurred())
Expect(initMaintenanceStatus(ctx, nm, drainer)).To(Succeed())
// status was initialized but the function will fail on updating the CR status, since we don't create a nm CR here
Expect(nm.Status.Phase).To(Equal(v1beta1.MaintenanceRunning))
Expect(len(nm.Status.PendingPods)).To(Equal(2))
Expand All @@ -78,7 +78,7 @@ var _ = Describe("Node Maintenance", func() {
})
When("Owner ref was set", func() {
It("should be set properly", func() {
Expect(initMaintenanceStatus(ctx, nm, drainer, r.Client)).To(HaveOccurred())
Expect(initMaintenanceStatus(ctx, nm, drainer)).To(Succeed())
// status was initialized but the function will fail on updating the CR status, since we don't create a nm CR here
By("Setting owner ref for a modified nm CR")
node := &corev1.Node{}
Expand All @@ -101,7 +101,7 @@ var _ = Describe("Node Maintenance", func() {
It("Should not modify the CR after initalization", func() {
nmCopy := nm.DeepCopy()
nmCopy.Status.Phase = v1beta1.MaintenanceFailed
Expect(initMaintenanceStatus(ctx, nmCopy, drainer, r.Client)).To(Succeed())
Expect(initMaintenanceStatus(ctx, nmCopy, drainer)).To(Succeed())
// status was not initialized thus the function succeeds
Expect(nmCopy.Status.Phase).To(Equal(v1beta1.MaintenanceFailed))
Expect(len(nmCopy.Status.PendingPods)).To(Equal(0))
Expand Down

0 comments on commit 80b4e31

Please sign in to comment.