From b245eac0aaa442d8c56d5ed9f790a6ca50478a57 Mon Sep 17 00:00:00 2001 From: liuwei Date: Thu, 8 Apr 2021 11:40:15 +0000 Subject: [PATCH] addon lease - compatibility Signed-off-by: liuwei --- pkg/hub/addon/healthcheck_controller.go | 2 +- pkg/hub/addon/healthcheck_controller_test.go | 2 +- pkg/spoke/addon/configuration.go | 7 +- pkg/spoke/addon/lease_controller.go | 58 +++++++++++---- pkg/spoke/addon/lease_controller_test.go | 75 ++++++++++++++------ pkg/spoke/spokeagent.go | 1 + test/e2e/addon_lease_test.go | 14 ++-- 7 files changed, 112 insertions(+), 47 deletions(-) diff --git a/pkg/hub/addon/healthcheck_controller.go b/pkg/hub/addon/healthcheck_controller.go index fd73bfbbc..a680ddae1 100644 --- a/pkg/hub/addon/healthcheck_controller.go +++ b/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. diff --git a/pkg/hub/addon/healthcheck_controller_test.go b/pkg/hub/addon/healthcheck_controller_test.go index b49f8a087..46036f2ba 100644 --- a/pkg/hub/addon/healthcheck_controller_test.go +++ b/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") } diff --git a/pkg/spoke/addon/configuration.go b/pkg/spoke/addon/configuration.go index 6b908d406..9cacf534b 100644 --- a/pkg/spoke/addon/configuration.go +++ b/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{ diff --git a/pkg/spoke/addon/lease_controller.go b/pkg/spoke/addon/lease_controller.go index 58e929b30..5566bc6a8 100644 --- a/pkg/spoke/addon/lease_controller.go +++ b/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{ diff --git a/pkg/spoke/addon/lease_controller_test.go b/pkg/spoke/addon/lease_controller_test.go index 18fc6c005..e647fd460 100644 --- a/pkg/spoke/addon/lease_controller_test.go +++ b/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) diff --git a/pkg/spoke/spokeagent.go b/pkg/spoke/spokeagent.go index 3e27fe650..0fe2dc5ca 100644 --- a/pkg/spoke/spokeagent.go +++ b/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, diff --git a/test/e2e/addon_lease_test.go b/test/e2e/addon_lease_test.go index 072c231b4..6106c341b 100644 --- a/test/e2e/addon_lease_test.go +++ b/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())