Skip to content

Commit

Permalink
remove self from annotation
Browse files Browse the repository at this point in the history
Signed-off-by: haoqing0110 <[email protected]>
  • Loading branch information
haoqing0110 committed Mar 25, 2024
1 parent 4eca494 commit 03254b0
Show file tree
Hide file tree
Showing 5 changed files with 255 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package managementaddonconfig
package cmaconfig

import (
"context"
Expand Down Expand Up @@ -30,8 +30,8 @@ const (

type enqueueFunc func(obj interface{})

// clusterManagementAddonConfigController reconciles all interested addon config types (GroupVersionResource) on the hub.
type clusterManagementAddonConfigController struct {
// cmaConfigController reconciles all interested addon config types (GroupVersionResource) on the hub.
type cmaConfigController struct {
addonClient addonv1alpha1client.Interface
clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
clusterManagementAddonIndexer cache.Indexer
Expand All @@ -44,7 +44,7 @@ type clusterManagementAddonConfigController struct {
addonapiv1alpha1.ClusterManagementAddOnStatus]
}

func NewManagementAddonConfigController(
func NewCMAConfigController(
addonClient addonv1alpha1client.Interface,
clusterManagementAddonInformers addoninformerv1alpha1.ClusterManagementAddOnInformer,
configInformerFactory dynamicinformer.DynamicSharedInformerFactory,
Expand All @@ -53,7 +53,7 @@ func NewManagementAddonConfigController(
) factory.Controller {
syncCtx := factory.NewSyncContext(controllerName)

c := &clusterManagementAddonConfigController{
c := &cmaConfigController{
addonClient: addonClient,
clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
clusterManagementAddonIndexer: clusterManagementAddonInformers.Informer().GetIndexer(),
Expand All @@ -78,7 +78,7 @@ func NewManagementAddonConfigController(
WithSync(c.sync).ToController(controllerName)
}

func (c *clusterManagementAddonConfigController) buildConfigInformers(
func (c *cmaConfigController) buildConfigInformers(
configInformerFactory dynamicinformer.DynamicSharedInformerFactory,
configGVRs map[schema.GroupVersionResource]bool,
) []factory.Informer {
Expand All @@ -104,7 +104,7 @@ func (c *clusterManagementAddonConfigController) buildConfigInformers(
return configInformers
}

func (c *clusterManagementAddonConfigController) enqueueClusterManagementAddOnsByConfig(gvr schema.GroupVersionResource) enqueueFunc {
func (c *cmaConfigController) enqueueClusterManagementAddOnsByConfig(gvr schema.GroupVersionResource) enqueueFunc {
return func(obj interface{}) {
namespaceName, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
if err != nil {
Expand All @@ -129,7 +129,7 @@ func (c *clusterManagementAddonConfigController) enqueueClusterManagementAddOnsB
}
}

func (c *clusterManagementAddonConfigController) sync(ctx context.Context, syncCtx factory.SyncContext, key string) error {
func (c *cmaConfigController) sync(ctx context.Context, syncCtx factory.SyncContext, key string) error {
_, addonName, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
// ignore addon whose key is invalid
Expand Down Expand Up @@ -158,7 +158,7 @@ func (c *clusterManagementAddonConfigController) sync(ctx context.Context, syncC
return err
}

func (c *clusterManagementAddonConfigController) updateConfigSpecHash(cma *addonapiv1alpha1.ClusterManagementAddOn) error {
func (c *cmaConfigController) updateConfigSpecHash(cma *addonapiv1alpha1.ClusterManagementAddOn) error {

for i, defaultConfigReference := range cma.Status.DefaultConfigReferences {
if !utils.ContainGR(
Expand Down Expand Up @@ -203,7 +203,7 @@ func (c *clusterManagementAddonConfigController) updateConfigSpecHash(cma *addon
return nil
}

func (c *clusterManagementAddonConfigController) getConfigSpecHash(gr addonapiv1alpha1.ConfigGroupResource,
func (c *cmaConfigController) getConfigSpecHash(gr addonapiv1alpha1.ConfigGroupResource,
cr addonapiv1alpha1.ConfigReferent) (string, error) {
lister, ok := c.configListers[schema.GroupResource{Group: gr.Group, Resource: gr.Resource}]
if !ok {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package managementaddonconfig
package cmaconfig

import (
"context"
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestSync(t *testing.T) {

syncContext := addontesting.NewFakeSyncContext(t)

ctrl := &clusterManagementAddonConfigController{
ctrl := &cmaConfigController{
addonClient: fakeAddonClient,
clusterManagementAddonLister: addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Lister(),
configListers: map[schema.GroupResource]dynamiclister.Lister{},
Expand Down Expand Up @@ -348,7 +348,7 @@ func TestEnqueue(t *testing.T) {
addonInformers := addoninformers.NewSharedInformerFactory(fakeAddonClient, 10*time.Minute)
addonInformer := addonInformers.Addon().V1alpha1().ManagedClusterAddOns().Informer()

ctrl := &clusterManagementAddonConfigController{
ctrl := &cmaConfigController{
clusterManagementAddonIndexer: addonInformer.GetIndexer(),
queue: addontesting.NewFakeSyncContext(t).Queue(),
}
Expand Down
94 changes: 94 additions & 0 deletions pkg/addonmanager/controllers/cmamanagedby/controller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package cmamanagedby

import (
"context"

"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/cache"
addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned"
addoninformerv1alpha1 "open-cluster-management.io/api/client/addon/informers/externalversions/addon/v1alpha1"
addonlisterv1alpha1 "open-cluster-management.io/api/client/addon/listers/addon/v1alpha1"
"open-cluster-management.io/sdk-go/pkg/patcher"

"open-cluster-management.io/addon-framework/pkg/agent"
"open-cluster-management.io/addon-framework/pkg/basecontroller/factory"
)

const (
controllerName = "cma-managed-by-controller"
)

// cmaManagedByController reconciles clustermanagementaddon on the hub
// to update the annotation "addon.open-cluster-management.io/lifecycle" value.
// It removes the value "self" if exist, which indicate the
// the installation and upgrade of addon will no longer be managed by addon itself.
// Once removed, the value will be set to "addon-manager" by the general addon manager.
type cmaManagedByController struct {
addonClient addonv1alpha1client.Interface
clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
agentAddons map[string]agent.AgentAddon
addonFilterFunc factory.EventFilterFunc
addonPatcher patcher.Patcher[*addonapiv1alpha1.ClusterManagementAddOn,
addonapiv1alpha1.ClusterManagementAddOnSpec,
addonapiv1alpha1.ClusterManagementAddOnStatus]
}

func NewCMAManagedByController(
addonClient addonv1alpha1client.Interface,
clusterManagementAddonInformers addoninformerv1alpha1.ClusterManagementAddOnInformer,
agentAddons map[string]agent.AgentAddon,
addonFilterFunc factory.EventFilterFunc,
) factory.Controller {
syncCtx := factory.NewSyncContext(controllerName)

c := &cmaManagedByController{
addonClient: addonClient,
clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
agentAddons: agentAddons,
addonFilterFunc: addonFilterFunc,
addonPatcher: patcher.NewPatcher[*addonapiv1alpha1.ClusterManagementAddOn,
addonapiv1alpha1.ClusterManagementAddOnSpec,
addonapiv1alpha1.ClusterManagementAddOnStatus](addonClient.AddonV1alpha1().ClusterManagementAddOns()),
}

return factory.New().
WithSyncContext(syncCtx).
WithFilteredEventsInformersQueueKeysFunc(
func(obj runtime.Object) []string {
key, _ := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
return []string{key}
},
c.addonFilterFunc, clusterManagementAddonInformers.Informer()).
WithSync(c.sync).ToController(controllerName)
}

func (c *cmaManagedByController) sync(ctx context.Context, syncCtx factory.SyncContext, key string) error {
_, addonName, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
// ignore addon whose key is invalid
return nil
}

cma, err := c.clusterManagementAddonLister.Get(addonName)
if errors.IsNotFound(err) {
// addon cloud be deleted, ignore
return nil
}
if err != nil {
return err
}

// Remove the annotation value "self" since the WithInstallStrategy() is removed in addon-framework.
// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
cmaCopy := cma.DeepCopy()
if cmaCopy.Annotations == nil ||
cmaCopy.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] != addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue {
return nil
}
cmaCopy.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] = ""

_, err = c.addonPatcher.PatchLabelAnnotations(ctx, cmaCopy, cmaCopy.ObjectMeta, cma.ObjectMeta)
return err
}
134 changes: 134 additions & 0 deletions pkg/addonmanager/controllers/cmamanagedby/controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package cmamanagedby

import (
"context"
"encoding/json"
"testing"
"time"

"k8s.io/apimachinery/pkg/runtime"
clienttesting "k8s.io/client-go/testing"
"open-cluster-management.io/addon-framework/pkg/addonmanager/addontesting"
"open-cluster-management.io/addon-framework/pkg/agent"
"open-cluster-management.io/addon-framework/pkg/utils"
addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
fakeaddon "open-cluster-management.io/api/client/addon/clientset/versioned/fake"
addoninformers "open-cluster-management.io/api/client/addon/informers/externalversions"
clusterv1 "open-cluster-management.io/api/cluster/v1"
"open-cluster-management.io/sdk-go/pkg/patcher"
)

type testAgent struct {
name string
}

func (t *testAgent) Manifests(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) ([]runtime.Object, error) {
return nil, nil
}

func (t *testAgent) GetAgentAddonOptions() agent.AgentAddonOptions {
return agent.AgentAddonOptions{
AddonName: t.name,
}
}

func newClusterManagementAddonWithAnnotation(name string, annotations map[string]string) *addonapiv1alpha1.ClusterManagementAddOn {
cma := addontesting.NewClusterManagementAddon(name, "", "").Build()
cma.Annotations = annotations
return cma
}

func TestReconcile(t *testing.T) {
cases := []struct {
name string
cma []runtime.Object
testaddons map[string]agent.AgentAddon
validateAddonActions func(t *testing.T, actions []clienttesting.Action)
}{
{
name: "no patch annotation if nil",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", nil)},
validateAddonActions: addontesting.AssertNoActions,
testaddons: map[string]agent.AgentAddon{
"test": &testAgent{name: "test"},
},
},
{
name: "no patch annotation if managed by not exist",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
"test": "test",
})},
validateAddonActions: addontesting.AssertNoActions,
testaddons: map[string]agent.AgentAddon{
"test": &testAgent{name: "test"},
},
},
{
name: "no patch annotation if managed by is not self",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
"test": "test",
addonapiv1alpha1.AddonLifecycleAnnotationKey: "xxx",
})},
validateAddonActions: addontesting.AssertNoActions,
testaddons: map[string]agent.AgentAddon{
"test": &testAgent{name: "test"},
},
},
{
name: "patch annotation if managed by self",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
"test": "test",
addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue,
})},
validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
addontesting.AssertActions(t, actions, "patch")
patch := actions[0].(clienttesting.PatchActionImpl).Patch
cma := &addonapiv1alpha1.ClusterManagementAddOn{}
err := json.Unmarshal(patch, cma)
if err != nil {
t.Fatal(err)
}

if len(cma.Annotations) != 1 || cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] != "" {
t.Errorf("cma annotation is not correct, expected self but got %s", cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey])
}
},
testaddons: map[string]agent.AgentAddon{
"test": &testAgent{name: "test"},
},
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
fakeAddonClient := fakeaddon.NewSimpleClientset(c.cma...)
addonInformers := addoninformers.NewSharedInformerFactory(fakeAddonClient, 10*time.Minute)

for _, obj := range c.cma {
if err := addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Informer().GetStore().Add(obj); err != nil {
t.Fatal(err)
}
}

controller := cmaManagedByController{
addonClient: fakeAddonClient,
clusterManagementAddonLister: addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Lister(),
agentAddons: c.testaddons,
addonFilterFunc: utils.FilterByAddonName(c.testaddons),
addonPatcher: patcher.NewPatcher[*addonapiv1alpha1.ClusterManagementAddOn,
addonapiv1alpha1.ClusterManagementAddOnSpec,
addonapiv1alpha1.ClusterManagementAddOnStatus](fakeAddonClient.AddonV1alpha1().ClusterManagementAddOns()),
}

for _, obj := range c.cma {
cma := obj.(*addonapiv1alpha1.ClusterManagementAddOn)
syncContext := addontesting.NewFakeSyncContext(t)
err := controller.sync(context.TODO(), syncContext, cma.Name)
if err != nil {
t.Errorf("expected no error when sync: %v", err)
}
}
c.validateAddonActions(t, fakeAddonClient.Actions())
})
}
}
16 changes: 14 additions & 2 deletions pkg/addonmanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import (
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/addonconfig"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/agentdeploy"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/certificate"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/managementaddonconfig"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/cmaconfig"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/cmamanagedby"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/registration"
"open-cluster-management.io/addon-framework/pkg/agent"
"open-cluster-management.io/addon-framework/pkg/basecontroller/factory"
Expand Down Expand Up @@ -237,6 +238,16 @@ func (a *addonManager) StartWithInformers(ctx context.Context,
a.addonAgents,
)

// This controller is used during migrating addons to be managed by addon-manager.
// This should be removed when the migration is done.
// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
managementAddonController := cmamanagedby.NewCMAManagedByController(
addonClient,
addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
a.addonAgents,
utils.FilterByAddonName(a.addonAgents),
)

var addonConfigController, managementAddonConfigController factory.Controller
if len(a.addonConfigs) != 0 {
addonConfigController = addonconfig.NewAddonConfigController(
Expand All @@ -247,7 +258,7 @@ func (a *addonManager) StartWithInformers(ctx context.Context,
a.addonConfigs,
utils.FilterByAddonName(a.addonAgents),
)
managementAddonConfigController = managementaddonconfig.NewManagementAddonConfigController(
managementAddonConfigController = cmaconfig.NewCMAConfigController(
addonClient,
addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
dynamicInformers,
Expand Down Expand Up @@ -293,6 +304,7 @@ func (a *addonManager) StartWithInformers(ctx context.Context,

go deployController.Run(ctx, 1)
go registrationController.Run(ctx, 1)
go managementAddonController.Run(ctx, 1)

if addonConfigController != nil {
go addonConfigController.Run(ctx, 1)
Expand Down

0 comments on commit 03254b0

Please sign in to comment.