Skip to content

Commit

Permalink
Add unit test for healthy nodes with not owned leases
Browse files Browse the repository at this point in the history
- lease should not be touched
- status update should still succeed

Signed-off-by: Marc Sluiter <[email protected]>
  • Loading branch information
slintes authored and openshift-cherrypick-robot committed Aug 31, 2023
1 parent 3154c51 commit 12e24c5
Showing 1 changed file with 47 additions and 0 deletions.
47 changes: 47 additions & 0 deletions controllers/nodehealthcheck_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,53 @@ var _ = Describe("Node Health Check CR", func() {
}, "2s", "100ms").Should(BeTrue(), "lease wasn't removed")

})

It("node lease not owned by us isn't removed, but status is updated (invalidate lease error is ignored)", func() {
cr := newRemediationCR(unhealthyNodeName, underTest)
err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(cr), cr)
Expect(err).ToNot(HaveOccurred())
//Verify lease exist
lease := &coordv1.Lease{}
err = k8sClient.Get(context.Background(), client.ObjectKey{Name: leaseName, Namespace: leaseNs}, lease)
Expect(err).ToNot(HaveOccurred())

// change lease owner
newLeaseOwner := "someone-else"
lease.Spec.HolderIdentity = pointer.String(newLeaseOwner)
Expect(k8sClient.Update(context.Background(), lease)).To(Succeed(), "failed to update lease owner")

//Mock node becoming healthy
node := &v1.Node{}
err = k8sClient.Get(context.Background(), client.ObjectKey{Name: unhealthyNodeName}, node)
Expect(err).ToNot(HaveOccurred())
for i, c := range node.Status.Conditions {
if c.Type == v1.NodeReady {
node.Status.Conditions[i].Status = v1.ConditionTrue
}
}
err = k8sClient.Status().Update(context.Background(), node)
Expect(err).ToNot(HaveOccurred())

//Remediation should be removed
Eventually(func() bool {
err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(cr), cr)
return errors.IsNotFound(err)
}, "2s", "100ms").Should(BeTrue(), "remediation CR wasn't removed")

// Status should be updated even though lease isn't owned by us anymore
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(underTest), underTest)).To(Succeed())
g.Expect(underTest.Status.InFlightRemediations).To(BeEmpty())
g.Expect(underTest.Status.UnhealthyNodes).To(BeEmpty())
}, "2s", "100ms").Should(Succeed(), "status update failed")

//Verify NHC didn't touch the lease
Consistently(func(g Gomega) {
g.Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: leaseName, Namespace: leaseNs}, lease)).To(Succeed(), "failed to get lease")
g.Expect(*lease.Spec.HolderIdentity).To(Equal(newLeaseOwner))
}, "2s", "100ms").Should(Succeed(), "lease was touched even though it's not owned by us")

})
})

When("an unhealthy node lease is already taken", func() {
Expand Down

0 comments on commit 12e24c5

Please sign in to comment.