Merge pull request #84998 from wojtek-t/fix_owner_refs

Ensure that Node lease has OwnerReference set

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 does not exist, create it.
-		lease, err := c.leaseClient.Create(c.newLease(nil))
+		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
 		}

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,10 +327,26 @@ 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 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
@@ -335,18 +354,22 @@ func TestUpdateUsingLatestLease(t *testing.T) {
 			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",
+			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",
+			existingObjs: []runtime.Object{node},
 			latestLease:  makeLease(nodeName, "1"),
 			updateReactor: func() func(action clienttesting.Action) (bool, runtime.Object, error) {
 				i := 0
@@ -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,9 +416,15 @@ func TestUpdateUsingLatestLease(t *testing.T) {
 
 			c.sync()
 
+			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)
+				}
+			}
 		})
 	}
 }

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)