Merge pull request #128 from skeeey/addon-manager-lease

compatibility - manage addon lease

pkg/hub/addon/healthcheck_controller.go

@@ -22,7 +22,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
-const addOnAvailableConditionType = "ManagedClusterAddOnConditionAvailable" //TODO add this to ManagedClusterAddOn api
+const addOnAvailableConditionType = "Available" //TODO add this to ManagedClusterAddOn api
 
 // managedClusterAddonHealthCheckController udpates managed cluster addons status through watching the managed cluster status on
 // the hub cluster.

pkg/hub/addon/healthcheck_controller_test.go

@@ -60,7 +60,7 @@ func TestSync(t *testing.T) {
 
 				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				addOn := actual.(*addonv1alpha1.ManagedClusterAddOn)
-				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "Available")
 				if addOnCond == nil {
 					t.Errorf("expected addon available condition, but failed")
 				}

pkg/spoke/addon/configuration.go

@@ -1,13 +1,12 @@
 package addon
 
 import (
-	"fmt"
-
 	addonv1alpha1 "github.com/open-cluster-management/api/addon/v1alpha1"
 )
 
 const (
-	installationNamespaceAnnotation = "addon.open-cluster-management.io/installNamespace"
+	defaultAddOnInstallationNamespace = "open-cluster-management-agent-addon"
+	installationNamespaceAnnotation   = "addon.open-cluster-management.io/installNamespace"
 )
 
 // addonConfig contains addon configuration information.
@@ -20,7 +19,7 @@ type addOnConfig struct {
 func getAddOnConfig(addOn *addonv1alpha1.ManagedClusterAddOn) (*addOnConfig, error) {
 	installationNamespace, ok := addOn.Annotations[installationNamespaceAnnotation]
 	if !ok {
-		return nil, fmt.Errorf("failed to get addon installation namesapce from addon %q", addOn.Name)
+		installationNamespace = defaultAddOnInstallationNamespace
 	}
 
 	addOnConfig := &addOnConfig{

pkg/spoke/addon/lease_controller.go

@@ -2,6 +2,7 @@ package addon
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	addonv1alpha1 "github.com/open-cluster-management/api/addon/v1alpha1"
@@ -20,13 +21,14 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/clock"
 	coordinformers "k8s.io/client-go/informers/coordination/v1"
+	coordv1client "k8s.io/client-go/kubernetes/typed/coordination/v1"
 	coordlisters "k8s.io/client-go/listers/coordination/v1"
 	"k8s.io/client-go/tools/cache"
 )
 
 const (
 	//TODO add this to ManagedClusterAddOn api
-	addOnAvailableConditionType = "ManagedClusterAddOnConditionAvailable"
+	addOnAvailableConditionType = "Available"
 	//TODO add this to ManagedClusterAddOn api
 	leaseDurationSeconds = 60
 	leaseDurationTimes   = 5
@@ -35,26 +37,29 @@ const (
 // managedClusterAddOnLeaseController udpates managed cluster addons status on the hub cluster through watching the managed
 // cluster status on the managed cluster.
 type managedClusterAddOnLeaseController struct {
-	clusterName string
-	clock       clock.Clock
-	addOnClient addonclient.Interface
-	addOnLister addonlisterv1alpha1.ManagedClusterAddOnLister
-	leaseLister coordlisters.LeaseLister
+	clusterName    string
+	clock          clock.Clock
+	addOnClient    addonclient.Interface
+	addOnLister    addonlisterv1alpha1.ManagedClusterAddOnLister
+	hubLeaseClient coordv1client.CoordinationV1Interface
+	leaseLister    coordlisters.LeaseLister
 }
 
 // NewManagedClusterAddOnLeaseController returns an instance of managedClusterAddOnLeaseController
 func NewManagedClusterAddOnLeaseController(clusterName string,
 	addOnClient addonclient.Interface,
 	addOnInformer addoninformerv1alpha1.ManagedClusterAddOnInformer,
+	hubLeaseClient coordv1client.CoordinationV1Interface,
 	leaseInformer coordinformers.LeaseInformer,
 	resyncInterval time.Duration,
 	recorder events.Recorder) factory.Controller {
 	c := &managedClusterAddOnLeaseController{
-		clusterName: clusterName,
-		clock:       clock.RealClock{},
-		addOnClient: addOnClient,
-		addOnLister: addOnInformer.Lister(),
-		leaseLister: leaseInformer.Lister(),
+		clusterName:    clusterName,
+		clock:          clock.RealClock{},
+		addOnClient:    addOnClient,
+		addOnLister:    addOnInformer.Lister(),
+		hubLeaseClient: hubLeaseClient,
+		leaseLister:    leaseInformer.Lister(),
 	}
 	return factory.New().
 		WithInformersQueueKeyFunc(c.queueKeyFunc, leaseInformer.Informer()).
@@ -76,7 +81,7 @@ func (c *managedClusterAddOnLeaseController) sync(ctx context.Context, syncCtx f
 				continue
 			}
 			// enqueue the addon to reconcile
-			syncCtx.Queue().Add(addOn)
+			syncCtx.Queue().Add(fmt.Sprintf("%s/%s", addOn.Namespace, addOn.Name))
 		}
 		return nil
 	}
@@ -103,12 +108,39 @@ func (c *managedClusterAddOnLeaseController) syncSingle(ctx context.Context,
 	leaseNamespace string,
 	addOn *addonv1alpha1.ManagedClusterAddOn,
 	recorder events.Recorder) error {
+	now := c.clock.Now()
+	gracePeriod := time.Duration(leaseDurationTimes*leaseDurationSeconds) * time.Second
 	// addon lease name should be same with the addon name.
 	observedLease, err := c.leaseLister.Leases(leaseNamespace).Get(addOn.Name)
 
 	var condition metav1.Condition
 	switch {
 	case errors.IsNotFound(err):
+		// for backward compatible, before release-2.3, addons update their leases on hub cluster,
+		// so if we cannot find addon lease on managed cluster, we will try to use addon hub lease.
+		// TODO: after release-2.3, we will remove these code
+		observedLease, err = c.hubLeaseClient.Leases(addOn.Namespace).Get(ctx, addOn.Name, metav1.GetOptions{})
+		if err == nil {
+			if now.Before(observedLease.Spec.RenewTime.Add(gracePeriod)) {
+				// the lease is constantly updated, update its addon status to available
+				condition = metav1.Condition{
+					Type:    addOnAvailableConditionType,
+					Status:  metav1.ConditionTrue,
+					Reason:  "ManagedClusterAddOnLeaseUpdated",
+					Message: "Managed cluster addon agent updates its lease constantly.",
+				}
+				break
+			}
+
+			// the lease is not constantly updated, update its addon status to unavailable
+			condition = metav1.Condition{
+				Type:    addOnAvailableConditionType,
+				Status:  metav1.ConditionFalse,
+				Reason:  "ManagedClusterAddOnLeaseUpdateStopped",
+				Message: "Managed cluster addon agent stopped updating its lease.",
+			}
+			break
+		}
 		condition = metav1.Condition{
 			Type:    addOnAvailableConditionType,
 			Status:  metav1.ConditionUnknown,
@@ -118,8 +150,6 @@ func (c *managedClusterAddOnLeaseController) syncSingle(ctx context.Context,
 	case err != nil:
 		return err
 	case err == nil:
-		now := c.clock.Now()
-		gracePeriod := time.Duration(leaseDurationTimes*leaseDurationSeconds) * time.Second
 		if now.Before(observedLease.Spec.RenewTime.Add(gracePeriod)) {
 			// the lease is constantly updated, update its addon status to available
 			condition = metav1.Condition{

pkg/spoke/addon/lease_controller_test.go

@@ -94,23 +94,26 @@ func TestSync(t *testing.T) {
 		name            string
 		queueKey        string
 		addOns          []runtime.Object
+		hubLeases       []runtime.Object
 		leases          []runtime.Object
 		validateActions func(t *testing.T, ctx *testinghelpers.FakeSyncContext, actions []clienttesting.Action)
 	}{
 		{
-			name:     "bad queue key",
-			queueKey: "test/test/test",
-			addOns:   []runtime.Object{},
-			leases:   []runtime.Object{},
+			name:      "bad queue key",
+			queueKey:  "test/test/test",
+			addOns:    []runtime.Object{},
+			hubLeases: []runtime.Object{},
+			leases:    []runtime.Object{},
 			validateActions: func(t *testing.T, ctx *testinghelpers.FakeSyncContext, actions []clienttesting.Action) {
 				testinghelpers.AssertNoActions(t, actions)
 			},
 		},
 		{
-			name:     "no addons",
-			queueKey: "test/test",
-			addOns:   []runtime.Object{},
-			leases:   []runtime.Object{},
+			name:      "no addons",
+			queueKey:  "test/test",
+			addOns:    []runtime.Object{},
+			hubLeases: []runtime.Object{},
+			leases:    []runtime.Object{},
 			validateActions: func(t *testing.T, ctx *testinghelpers.FakeSyncContext, actions []clienttesting.Action) {
 				testinghelpers.AssertNoActions(t, actions)
 			},
@@ -125,12 +128,13 @@ func TestSync(t *testing.T) {
 					Annotations: map[string]string{"addon.open-cluster-management.io/installNamespace": "test"},
 				},
 			}},
-			leases: []runtime.Object{},
+			hubLeases: []runtime.Object{},
+			leases:    []runtime.Object{},
 			validateActions: func(t *testing.T, ctx *testinghelpers.FakeSyncContext, actions []clienttesting.Action) {
 				testinghelpers.AssertActions(t, actions, "get", "update")
 				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				addOn := actual.(*addonv1alpha1.ManagedClusterAddOn)
-				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "Available")
 				if addOnCond == nil {
 					t.Errorf("expected addon available condition, but failed")
 				}
@@ -149,6 +153,7 @@ func TestSync(t *testing.T) {
 					Annotations: map[string]string{"addon.open-cluster-management.io/installNamespace": "test"},
 				},
 			}},
+			hubLeases: []runtime.Object{},
 			leases: []runtime.Object{
 				testinghelpers.NewAddOnLease("test", "test", now.Add(-5*time.Minute)),
 			},
@@ -156,7 +161,7 @@ func TestSync(t *testing.T) {
 				testinghelpers.AssertActions(t, actions, "get", "update")
 				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				addOn := actual.(*addonv1alpha1.ManagedClusterAddOn)
-				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "Available")
 				if addOnCond == nil {
 					t.Errorf("expected addon available condition, but failed")
 				}
@@ -175,6 +180,7 @@ func TestSync(t *testing.T) {
 					Annotations: map[string]string{"addon.open-cluster-management.io/installNamespace": "test"},
 				},
 			}},
+			hubLeases: []runtime.Object{},
 			leases: []runtime.Object{
 				testinghelpers.NewAddOnLease("test", "test", now),
 			},
@@ -182,7 +188,7 @@ func TestSync(t *testing.T) {
 				testinghelpers.AssertActions(t, actions, "get", "update")
 				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				addOn := actual.(*addonv1alpha1.ManagedClusterAddOn)
-				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "Available")
 				if addOnCond == nil {
 					t.Errorf("expected addon available condition, but failed")
 				}
@@ -203,7 +209,7 @@ func TestSync(t *testing.T) {
 				Status: addonv1alpha1.ManagedClusterAddOnStatus{
 					Conditions: []metav1.Condition{
 						{
-							Type:    "ManagedClusterAddOnConditionAvailable",
+							Type:    "Available",
 							Status:  metav1.ConditionTrue,
 							Reason:  "ManagedClusterAddOnLeaseUpdated",
 							Message: "Managed cluster addon agent updates its lease constantly.",
@@ -211,6 +217,7 @@ func TestSync(t *testing.T) {
 					},
 				},
 			}},
+			hubLeases: []runtime.Object{},
 			leases: []runtime.Object{
 				testinghelpers.NewAddOnLease("test", "test", now),
 			},
@@ -236,12 +243,37 @@ func TestSync(t *testing.T) {
 					},
 				},
 			},
+			hubLeases: []runtime.Object{},
 			leases: []runtime.Object{
 				testinghelpers.NewAddOnLease("test1", "test1", now.Add(-5*time.Minute)),
 			},
 			validateActions: func(t *testing.T, ctx *testinghelpers.FakeSyncContext, actions []clienttesting.Action) {
-				if ctx.Queue().Len() != 1 {
-					t.Errorf("expected one addon in queue, but failed")
+				if ctx.Queue().Len() != 2 {
+					t.Errorf("expected two addons in queue, but failed")
+				}
+			},
+		},
+		{
+			name:     "addon update its lease constantly (compatibility)",
+			queueKey: "test/test",
+			addOns: []runtime.Object{&addonv1alpha1.ManagedClusterAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: testinghelpers.TestManagedClusterName,
+					Name:      "test",
+				},
+			}},
+			hubLeases: []runtime.Object{testinghelpers.NewAddOnLease(testinghelpers.TestManagedClusterName, "test", now)},
+			leases:    []runtime.Object{},
+			validateActions: func(t *testing.T, ctx *testinghelpers.FakeSyncContext, actions []clienttesting.Action) {
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				actual := actions[1].(clienttesting.UpdateActionImpl).Object
+				addOn := actual.(*addonv1alpha1.ManagedClusterAddOn)
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, "Available")
+				if addOnCond == nil {
+					t.Errorf("expected addon available condition, but failed")
+				}
+				if addOnCond.Status != metav1.ConditionTrue {
+					t.Errorf("expected addon available condition is available, but failed")
 				}
 			},
 		},
@@ -256,6 +288,8 @@ func TestSync(t *testing.T) {
 				addOnStroe.Add(addOn)
 			}
 
+			hubClient := kubefake.NewSimpleClientset(c.hubLeases...)
+
 			leaseClient := kubefake.NewSimpleClientset(c.leases...)
 			leaseInformerFactory := kubeinformers.NewSharedInformerFactory(leaseClient, time.Minute*10)
 			leaseStore := leaseInformerFactory.Coordination().V1().Leases().Informer().GetStore()
@@ -264,11 +298,12 @@ func TestSync(t *testing.T) {
 			}
 
 			ctrl := &managedClusterAddOnLeaseController{
-				clusterName: testinghelpers.TestManagedClusterName,
-				clock:       clock.NewFakeClock(time.Now()),
-				addOnClient: addOnClient,
-				addOnLister: addOnInformerFactory.Addon().V1alpha1().ManagedClusterAddOns().Lister(),
-				leaseLister: leaseInformerFactory.Coordination().V1().Leases().Lister(),
+				clusterName:    testinghelpers.TestManagedClusterName,
+				clock:          clock.NewFakeClock(time.Now()),
+				hubLeaseClient: hubClient.CoordinationV1(),
+				addOnClient:    addOnClient,
+				addOnLister:    addOnInformerFactory.Addon().V1alpha1().ManagedClusterAddOns().Lister(),
+				leaseLister:    leaseInformerFactory.Coordination().V1().Leases().Lister(),
 			}
 			syncCtx := testinghelpers.NewFakeSyncContext(t, c.queueKey)
 			syncErr := ctrl.sync(context.TODO(), syncCtx)

pkg/spoke/spokeagent.go

@@ -306,6 +306,7 @@ func (o *SpokeAgentOptions) RunSpokeAgent(ctx context.Context, controllerContext
 			o.ClusterName,
 			addOnClient,
 			addOnInformerFactory.Addon().V1alpha1().ManagedClusterAddOns(),
+			hubKubeClient.CoordinationV1(),
 			spokeKubeInformerFactory.Coordination().V1().Leases(),
 			5*time.Minute, //TODO: this interval time should be allowed to change from outside
 			controllerContext.EventRecorder,

test/e2e/addon_lease_test.go

@@ -79,7 +79,7 @@ var _ = ginkgo.Describe("Addon Health Check", func() {
 				if found.Status.Conditions == nil {
 					return false, nil
 				}
-				cond := meta.FindStatusCondition(found.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				cond := meta.FindStatusCondition(found.Status.Conditions, "Available")
 				return cond.Status == metav1.ConditionTrue, nil
 			})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -106,7 +106,7 @@ var _ = ginkgo.Describe("Addon Health Check", func() {
 				if found.Status.Conditions == nil {
 					return false, nil
 				}
-				cond := meta.FindStatusCondition(found.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				cond := meta.FindStatusCondition(found.Status.Conditions, "Available")
 				return cond.Status == metav1.ConditionTrue, nil
 			})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -126,7 +126,7 @@ var _ = ginkgo.Describe("Addon Health Check", func() {
 				if found.Status.Conditions == nil {
 					return false, nil
 				}
-				cond := meta.FindStatusCondition(found.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				cond := meta.FindStatusCondition(found.Status.Conditions, "Available")
 				return cond.Status == metav1.ConditionFalse, nil
 			})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -153,7 +153,7 @@ var _ = ginkgo.Describe("Addon Health Check", func() {
 				if found.Status.Conditions == nil {
 					return false, nil
 				}
-				cond := meta.FindStatusCondition(found.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				cond := meta.FindStatusCondition(found.Status.Conditions, "Available")
 				return cond.Status == metav1.ConditionTrue, nil
 			})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -170,7 +170,7 @@ var _ = ginkgo.Describe("Addon Health Check", func() {
 				if found.Status.Conditions == nil {
 					return false, nil
 				}
-				cond := meta.FindStatusCondition(found.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				cond := meta.FindStatusCondition(found.Status.Conditions, "Available")
 				return cond.Status == metav1.ConditionUnknown, nil
 			})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -231,7 +231,7 @@ var _ = ginkgo.Describe("Addon Health Check", func() {
 				if found.Status.Conditions == nil {
 					return false, nil
 				}
-				cond := meta.FindStatusCondition(found.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				cond := meta.FindStatusCondition(found.Status.Conditions, "Available")
 				return cond.Status == metav1.ConditionTrue, nil
 			})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -267,7 +267,7 @@ var _ = ginkgo.Describe("Addon Health Check", func() {
 				if found.Status.Conditions == nil {
 					return false, nil
 				}
-				cond := meta.FindStatusCondition(found.Status.Conditions, "ManagedClusterAddOnConditionAvailable")
+				cond := meta.FindStatusCondition(found.Status.Conditions, "Available")
 				return cond.Status == metav1.ConditionUnknown, nil
 			})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())