filter cluster management addon when running spec hash updating contr…

…ollers (#193) * filter cluster management addon when running spec hash updating controllers Signed-off-by: zhujian <[email protected]> * fix e2e Signed-off-by: zhujian <[email protected]> --------- Signed-off-by: zhujian <[email protected]>
open-cluster-management-io · Jul 24, 2023 · 6e88a5d · 6e88a5d
1 parent 21a8cbc
commit 6e88a5d
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 35 deletions.
diff --git a/build/Dockerfile b/build/Dockerfile
@@ -1,4 +1,4 @@
-FROM golang:1.19 AS builder
+FROM golang:1.19-bullseye AS builder
 ARG OS=linux
 ARG ARCH=amd64
 WORKDIR /go/src/open-cluster-management.io/addon-framework

diff --git a/build/Dockerfile.example b/build/Dockerfile.example
@@ -1,4 +1,4 @@
-FROM golang:1.19 AS builder
+FROM golang:1.19-bullseye AS builder
 WORKDIR /go/src/open-cluster-management.io/addon-framework
 COPY . .
 ENV GO_PACKAGE open-cluster-management.io/addon-framework

diff --git a/examples/deploy/hosted-ocm/install.sh b/examples/deploy/hosted-ocm/install.sh
@@ -77,13 +77,14 @@ ${KUBECTL} --context="${cluster_context}" -n kube-system scale --replicas=1 depl
 echo "###### loading image: ${EXAMPLE_IMAGE_NAME}"
 ${KIND} load docker-image ${EXAMPLE_IMAGE_NAME} --name ${MANAGED_CLUSTER_NAME}
 
-echo "###### deploy registration-operator"
-rm -rf "$WORK_DIR/registration-operator"
-git clone https://github.com/open-cluster-management-io/registration-operator.git "$WORK_DIR/registration-operator"
-${KUBECTL} apply -k "$WORK_DIR/registration-operator/deploy/cluster-manager/config/manifests"
-${KUBECTL} apply -k "$WORK_DIR/registration-operator/deploy/cluster-manager/config/samples"
-${KUBECTL} apply -k "$WORK_DIR/registration-operator/deploy/klusterlet/config/manifests"
-rm -rf "$WORK_DIR/registration-operator"
+echo "###### deploy operators"
+rm -rf "$WORK_DIR/_repo_ocm"
+git clone --depth 1 --branch main https://github.com/open-cluster-management-io/ocm.git "$WORK_DIR/_repo_ocm"
+
+${KUBECTL} apply -k "$WORK_DIR/_repo_ocm/deploy/cluster-manager/config/manifests"
+${KUBECTL} apply -k "$WORK_DIR/_repo_ocm/deploy/cluster-manager/config/samples"
+${KUBECTL} apply -k "$WORK_DIR/_repo_ocm/deploy/klusterlet/config/manifests"
+rm -rf "$WORK_DIR/_repo_ocm"
 
 ${KUBECTL} get ns open-cluster-management-agent || ${KUBECTL} create ns open-cluster-management-agent
 

diff --git a/examples/deploy/ocm/install.sh b/examples/deploy/ocm/install.sh
@@ -5,18 +5,18 @@ set -o pipefail
 
 KUBECTL=${KUBECTL:-kubectl}
 
-rm -rf registration-operator
+rm -rf _repo_ocm
 
-echo "############  Cloning registration-operator"
-git clone https://github.com/open-cluster-management-io/registration-operator.git
+echo "############  Cloning ocm repo"
+git clone --depth 1 --branch main https://github.com/open-cluster-management-io/ocm.git _repo_ocm
 
-cd registration-operator || {
-  printf "cd failed, registration-operator does not exist"
+cd _repo_ocm || {
+  printf "cd failed, _repo_ocm does not exist"
   return 1
 }
 
-echo "############  Deploying"
-make deploy
+echo "############  Deploying operators"
+make deploy-hub cluster-ip deploy-spoke-operator apply-spoke-cr
 if [ $? -ne 0 ]; then
  echo "############  Failed to deploy"
  exit 1
@@ -72,6 +72,6 @@ echo "############  All-in-one env is installed successfully!!"
 
 echo "############  Cleanup"
 cd ../ || exist
-rm -rf registration-operator
+rm -rf _repo_ocm
 
 echo "############  Finished installation!!!"
diff --git a/pkg/addonmanager/controllers/addonconfig/controller.go b/pkg/addonmanager/controllers/addonconfig/controller.go
@@ -37,27 +37,33 @@ type enqueueFunc func(obj interface{})
 
 // addonConfigController reconciles all interested addon config types (GroupVersionResource) on the hub.
 type addonConfigController struct {
-	addonClient   addonv1alpha1client.Interface
-	addonLister   addonlisterv1alpha1.ManagedClusterAddOnLister
-	addonIndexer  cache.Indexer
-	configListers map[schema.GroupResource]dynamiclister.Lister
-	queue         workqueue.RateLimitingInterface
+	addonClient                  addonv1alpha1client.Interface
+	addonLister                  addonlisterv1alpha1.ManagedClusterAddOnLister
+	addonIndexer                 cache.Indexer
+	configListers                map[schema.GroupResource]dynamiclister.Lister
+	queue                        workqueue.RateLimitingInterface
+	addonFilterFunc              factory.EventFilterFunc
+	clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
 }
 
 func NewAddonConfigController(
 	addonClient addonv1alpha1client.Interface,
 	addonInformers addoninformerv1alpha1.ManagedClusterAddOnInformer,
+	clusterManagementAddonInformers addoninformerv1alpha1.ClusterManagementAddOnInformer,
 	configInformerFactory dynamicinformer.DynamicSharedInformerFactory,
 	configGVRs map[schema.GroupVersionResource]bool,
+	addonFilterFunc factory.EventFilterFunc,
 ) factory.Controller {
 	syncCtx := factory.NewSyncContext(controllerName)
 
 	c := &addonConfigController{
-		addonClient:   addonClient,
-		addonLister:   addonInformers.Lister(),
-		addonIndexer:  addonInformers.Informer().GetIndexer(),
-		configListers: map[schema.GroupResource]dynamiclister.Lister{},
-		queue:         syncCtx.Queue(),
+		addonClient:                  addonClient,
+		addonLister:                  addonInformers.Lister(),
+		addonIndexer:                 addonInformers.Informer().GetIndexer(),
+		configListers:                map[schema.GroupResource]dynamiclister.Lister{},
+		queue:                        syncCtx.Queue(),
+		addonFilterFunc:              addonFilterFunc,
+		clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
 	}
 
 	configInformers := c.buildConfigInformers(configInformerFactory, configGVRs)
@@ -81,7 +87,8 @@ func (c *addonConfigController) buildConfigInformers(
 	configGVRs map[schema.GroupVersionResource]bool,
 ) []factory.Informer {
 	configInformers := []factory.Informer{}
-	for gvr := range configGVRs {
+	for gvrRaw := range configGVRs {
+		gvr := gvrRaw // copy the value since it will be used in the closure
 		genericInformer := configInformerFactory.ForResource(gvr)
 		indexInformer := genericInformer.Informer()
 		_, err := indexInformer.AddEventHandler(
@@ -154,13 +161,26 @@ func (c *addonConfigController) sync(ctx context.Context, syncCtx factory.SyncCo
 
 	addon, err := c.addonLister.ManagedClusterAddOns(addonNamespace).Get(addonName)
 	if errors.IsNotFound(err) {
-		// addon cloud be deleted, ignore
+		// addon could be deleted, ignore
 		return nil
 	}
 	if err != nil {
 		return err
 	}
 
+	cma, err := c.clusterManagementAddonLister.Get(addonName)
+	if errors.IsNotFound(err) {
+		// cluster management addon could be deleted, ignore
+		return nil
+	}
+	if err != nil {
+		return err
+	}
+
+	if !c.addonFilterFunc(cma) {
+		return nil
+	}
+
 	addonCopy := addon.DeepCopy()
 
 	if err := c.updateConfigSpecHashAndGenerations(addonCopy); err != nil {

diff --git a/pkg/addonmanager/controllers/addonconfig/controller_test.go b/pkg/addonmanager/controllers/addonconfig/controller_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -363,6 +364,15 @@ func TestSync(t *testing.T) {
 				}
 			}
 
+			cmaStore := addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Informer().GetStore()
+			if err := cmaStore.Add(&addonapiv1alpha1.ClusterManagementAddOn{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test",
+				},
+			}); err != nil {
+				t.Fatal(err)
+			}
+
 			fakeDynamicClient := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme())
 			configInformerFactory := dynamicinformer.NewDynamicSharedInformerFactory(fakeDynamicClient, 0)
 			configInformer := configInformerFactory.ForResource(fakeGVR)
@@ -376,9 +386,11 @@ func TestSync(t *testing.T) {
 			syncContext := addontesting.NewFakeSyncContext(t)
 
 			ctrl := &addonConfigController{
-				addonClient:   fakeAddonClient,
-				addonLister:   addonInformers.Addon().V1alpha1().ManagedClusterAddOns().Lister(),
-				configListers: map[schema.GroupResource]dynamiclister.Lister{},
+				addonClient:                  fakeAddonClient,
+				addonLister:                  addonInformers.Addon().V1alpha1().ManagedClusterAddOns().Lister(),
+				clusterManagementAddonLister: addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Lister(),
+				configListers:                map[schema.GroupResource]dynamiclister.Lister{},
+				addonFilterFunc:              func(obj interface{}) bool { return true },
 			}
 
 			ctrl.buildConfigInformers(configInformerFactory, map[schema.GroupVersionResource]bool{fakeGVR: true})

diff --git a/pkg/addonmanager/controllers/managementaddonconfig/controller.go b/pkg/addonmanager/controllers/managementaddonconfig/controller.go
@@ -43,13 +43,15 @@ type clusterManagementAddonConfigController struct {
 	clusterManagementAddonIndexer cache.Indexer
 	configListers                 map[schema.GroupResource]dynamiclister.Lister
 	queue                         workqueue.RateLimitingInterface
+	addonFilterFunc               factory.EventFilterFunc
 }
 
 func NewManagementAddonConfigController(
 	addonClient addonv1alpha1client.Interface,
 	clusterManagementAddonInformers addoninformerv1alpha1.ClusterManagementAddOnInformer,
 	configInformerFactory dynamicinformer.DynamicSharedInformerFactory,
 	configGVRs map[schema.GroupVersionResource]bool,
+	addonFilterFunc factory.EventFilterFunc,
 ) factory.Controller {
 	syncCtx := factory.NewSyncContext(controllerName)
 
@@ -59,6 +61,7 @@ func NewManagementAddonConfigController(
 		clusterManagementAddonIndexer: clusterManagementAddonInformers.Informer().GetIndexer(),
 		configListers:                 map[schema.GroupResource]dynamiclister.Lister{},
 		queue:                         syncCtx.Queue(),
+		addonFilterFunc:               addonFilterFunc,
 	}
 
 	configInformers := c.buildConfigInformers(configInformerFactory, configGVRs)
@@ -82,7 +85,8 @@ func (c *clusterManagementAddonConfigController) buildConfigInformers(
 	configGVRs map[schema.GroupVersionResource]bool,
 ) []factory.Informer {
 	configInformers := []factory.Informer{}
-	for gvr := range configGVRs {
+	for gvrRaw := range configGVRs {
+		gvr := gvrRaw // copy the value since it will be used in the closure
 		indexInformer := configInformerFactory.ForResource(gvr).Informer()
 		_, err := indexInformer.AddEventHandler(
 			cache.ResourceEventHandlerFuncs{
@@ -172,6 +176,10 @@ func (c *clusterManagementAddonConfigController) sync(ctx context.Context, syncC
 		return err
 	}
 
+	if !c.addonFilterFunc(cma) {
+		return nil
+	}
+
 	cmaCopy := cma.DeepCopy()
 
 	if err := c.updateConfigSpecHash(cmaCopy); err != nil {

diff --git a/pkg/addonmanager/controllers/managementaddonconfig/controller_test.go b/pkg/addonmanager/controllers/managementaddonconfig/controller_test.go
@@ -257,6 +257,7 @@ func TestSync(t *testing.T) {
 				addonClient:                  fakeAddonClient,
 				clusterManagementAddonLister: addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Lister(),
 				configListers:                map[schema.GroupResource]dynamiclister.Lister{},
+				addonFilterFunc:              func(obj interface{}) bool { return true },
 			}
 
 			ctrl.buildConfigInformers(configInformerFactory, map[schema.GroupVersionResource]bool{fakeGVR: true})

diff --git a/pkg/addonmanager/manager.go b/pkg/addonmanager/manager.go
@@ -183,14 +183,17 @@ func (a *addonManager) Start(ctx context.Context) error {
 		addonConfigController = addonconfig.NewAddonConfigController(
 			addonClient,
 			addonInformers.Addon().V1alpha1().ManagedClusterAddOns(),
+			addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
 			dynamicInformers,
 			a.addonConfigs,
+			utils.FilterByAddonName(a.addonAgents),
 		)
 		managementAddonConfigController = managementaddonconfig.NewManagementAddonConfigController(
 			addonClient,
 			addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
 			dynamicInformers,
 			a.addonConfigs,
+			utils.FilterByAddonName(a.addonAgents),
 		)
 
 		// start addonConfiguration controller, note this is to handle the case when the general addon-manager

diff --git a/pkg/utils/helpers.go b/pkg/utils/helpers.go
@@ -22,7 +22,6 @@ import (
 	addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned"
 
 	"open-cluster-management.io/addon-framework/pkg/agent"
-	"open-cluster-management.io/addon-framework/pkg/basecontroller/factory"
 )
 
 func MergeRelatedObjects(modified *bool, objs *[]addonapiv1alpha1.ObjectReference, obj addonapiv1alpha1.ObjectReference) {
@@ -305,7 +304,7 @@ func ManagedByAddonManager(obj interface{}) bool {
 	return value == addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue
 }
 
-func ManagedBySelf(agentAddons map[string]agent.AgentAddon) factory.EventFilterFunc {
+func ManagedBySelf(agentAddons map[string]agent.AgentAddon) func(obj interface{}) bool {
 	return func(obj interface{}) bool {
 		accessor, _ := meta.Accessor(obj)
 		if _, ok := agentAddons[accessor.GetName()]; !ok {
@@ -327,6 +326,14 @@ func ManagedBySelf(agentAddons map[string]agent.AgentAddon) factory.EventFilterF
 	}
 }
 
+func FilterByAddonName(agentAddons map[string]agent.AgentAddon) func(obj interface{}) bool {
+	return func(obj interface{}) bool {
+		accessor, _ := meta.Accessor(obj)
+		_, ok := agentAddons[accessor.GetName()]
+		return ok
+	}
+}
+
 func IsOwnedByCMA(addon *addonapiv1alpha1.ManagedClusterAddOn) bool {
 	for _, owner := range addon.OwnerReferences {
 		if owner.Kind != "ClusterManagementAddOn" {