diff --git a/pkg/kubelet/nodelease/controller.go b/pkg/kubelet/nodelease/controller.go index 790974c6132..86069fe9d4d 100644 --- a/pkg/kubelet/nodelease/controller.go +++ b/pkg/kubelet/nodelease/controller.go @@ -108,7 +108,7 @@ func (c *controller) sync() { lease, created := c.backoffEnsureLease() c.latestLease = lease // we don't need to update the lease if we just created it - if !created { + if !created && lease != nil { if err := c.retryUpdateLease(lease); err != nil { klog.Errorf("%v, will retry after %v", err, c.renewInterval) } @@ -144,8 +144,15 @@ func (c *controller) backoffEnsureLease() (*coordinationv1.Lease, bool) { func (c *controller) ensureLease() (*coordinationv1.Lease, bool, error) { lease, err := c.leaseClient.Get(c.holderIdentity, metav1.GetOptions{}) if apierrors.IsNotFound(err) { - // lease does not exist, create it - lease, err := c.leaseClient.Create(c.newLease(nil)) + // lease does not exist, create it. + leaseToCreate := c.newLease(nil) + if len(leaseToCreate.OwnerReferences) == 0 { + // We want to ensure that a lease will always have OwnerReferences set. + // Thus, given that we weren't able to set it correctly, we simply + // not create it this time - we will retry in the next iteration. + return nil, false, nil + } + lease, err := c.leaseClient.Create(leaseToCreate) if err != nil { return nil, false, err } diff --git a/pkg/kubelet/nodelease/controller_test.go b/pkg/kubelet/nodelease/controller_test.go index a2eaea5ac11..00385ce413c 100644 --- a/pkg/kubelet/nodelease/controller_test.go +++ b/pkg/kubelet/nodelease/controller_test.go @@ -308,14 +308,17 @@ func TestUpdateUsingLatestLease(t *testing.T) { cases := []struct { desc string + existingObjs []runtime.Object latestLease *coordinationv1.Lease updateReactor func(action clienttesting.Action) (bool, runtime.Object, error) getReactor func(action clienttesting.Action) (bool, runtime.Object, error) createReactor func(action clienttesting.Action) (bool, runtime.Object, error) + expectLatestLease bool expectLeaseResourceVersion string }{ { desc: "latestLease is nil and need to create", + existingObjs: []runtime.Object{node}, latestLease: nil, updateReactor: nil, getReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { @@ -324,30 +327,50 @@ func TestUpdateUsingLatestLease(t *testing.T) { createReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { return true, makeLease(nodeName, "1"), nil }, + expectLatestLease: true, expectLeaseResourceVersion: "1", }, { - desc: "latestLease is nil and need to update", - latestLease: nil, + desc: "latestLease is nil and need to create, node doesn't exist", + existingObjs: nil, + latestLease: nil, + updateReactor: nil, + getReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { + return true, nil, notFoundErr + }, + createReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { + return true, makeLease(nodeName, "1"), nil + }, + expectLatestLease: false, + expectLeaseResourceVersion: "1", + }, + { + desc: "latestLease is nil and need to update", + existingObjs: []runtime.Object{node}, + latestLease: nil, updateReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { return true, makeLease(nodeName, "2"), nil }, getReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { return true, makeLease(nodeName, "1"), nil }, + expectLatestLease: true, expectLeaseResourceVersion: "2", }, { - desc: "latestLease exist and need to update", - latestLease: makeLease(nodeName, "1"), + desc: "latestLease exist and need to update", + existingObjs: []runtime.Object{node}, + latestLease: makeLease(nodeName, "1"), updateReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { return true, makeLease(nodeName, "2"), nil }, + expectLatestLease: true, expectLeaseResourceVersion: "2", }, { - desc: "update with latest lease failed", - latestLease: makeLease(nodeName, "1"), + desc: "update with latest lease failed", + existingObjs: []runtime.Object{node}, + latestLease: makeLease(nodeName, "1"), updateReactor: func() func(action clienttesting.Action) (bool, runtime.Object, error) { i := 0 return func(action clienttesting.Action) (bool, runtime.Object, error) { @@ -366,12 +389,13 @@ func TestUpdateUsingLatestLease(t *testing.T) { getReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { return true, makeLease(nodeName, "2"), nil }, + expectLatestLease: true, expectLeaseResourceVersion: "3", }, } for _, tc := range cases { t.Run(tc.desc, func(t *testing.T) { - cl := fake.NewSimpleClientset(node) + cl := fake.NewSimpleClientset(tc.existingObjs...) if tc.updateReactor != nil { cl.PrependReactor("update", "leases", tc.updateReactor) } @@ -392,8 +416,14 @@ func TestUpdateUsingLatestLease(t *testing.T) { c.sync() - if tc.expectLeaseResourceVersion != c.latestLease.ResourceVersion { - t.Fatalf("latestLease RV got %v, expected %v", c.latestLease.ResourceVersion, tc.expectLeaseResourceVersion) + if tc.expectLatestLease { + if tc.expectLeaseResourceVersion != c.latestLease.ResourceVersion { + t.Fatalf("latestLease RV got %v, expected %v", c.latestLease.ResourceVersion, tc.expectLeaseResourceVersion) + } + } else { + if c.latestLease != nil { + t.Fatalf("unexpected latestLease: %v", c.latestLease) + } } }) } diff --git a/test/e2e/common/node_lease.go b/test/e2e/common/node_lease.go index 7f54de68e7a..cbb2408d589 100644 --- a/test/e2e/common/node_lease.go +++ b/test/e2e/common/node_lease.go @@ -84,6 +84,30 @@ var _ = framework.KubeDescribe("NodeLease", func() { time.Duration(*lease.Spec.LeaseDurationSeconds/4)*time.Second) }) + ginkgo.It("should have OwnerReferences set", func() { + leaseClient := f.ClientSet.CoordinationV1().Leases(v1.NamespaceNodeLease) + var ( + err error + leaseList *coordinationv1.LeaseList + ) + gomega.Eventually(func() error { + leaseList, err = leaseClient.List(metav1.ListOptions{}) + if err != nil { + return err + } + return nil + }, 5*time.Minute, 5*time.Second).Should(gomega.BeNil()) + // All the leases should have OwnerReferences set to their corresponding + // Node object. + for i := range leaseList.Items { + lease := &leaseList.Items[i] + ownerRefs := lease.ObjectMeta.OwnerReferences + framework.ExpectEqual(len(ownerRefs), 1) + framework.ExpectEqual(ownerRefs[0].Kind, v1.SchemeGroupVersion.WithKind("Node").Kind) + framework.ExpectEqual(ownerRefs[0].APIVersion, v1.SchemeGroupVersion.WithKind("Node").Version) + } + }) + ginkgo.It("the kubelet should report node status infrequently", func() { ginkgo.By("wait until node is ready") e2enode.WaitForNodeToBeReady(f.ClientSet, nodeName, 5*time.Minute)