Skip to content

Commit

Permalink
add annotation if addon uses InstallStrategy (#238)
Browse files Browse the repository at this point in the history
Signed-off-by: haoqing0110 <qhao@redhat.com>
  • Loading branch information
haoqing0110 committed Feb 3, 2024
1 parent 6f89159 commit 0221149
Show file tree
Hide file tree
Showing 9 changed files with 324 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
verbs: ["update", "patch"]
- apiGroups: ["addon.open-cluster-management.io"]
resources: ["clustermanagementaddons"]
verbs: ["get", "list", "watch"]
verbs: ["get", "list", "watch", "patch"]
- apiGroups: ["addon.open-cluster-management.io"]
resources: ["managedclusteraddons"]
verbs: ["get", "list", "watch", "create", "update", "delete"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
verbs: ["update", "patch"]
- apiGroups: ["addon.open-cluster-management.io"]
resources: ["clustermanagementaddons"]
verbs: ["get", "list", "watch"]
verbs: ["get", "list", "watch", "patch"]
- apiGroups: ["addon.open-cluster-management.io"]
resources: ["managedclusteraddons"]
verbs: ["get", "list", "watch", "create", "update", "delete"]
Expand Down
3 changes: 3 additions & 0 deletions pkg/addonfactory/addonfactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ func (f *AgentAddonFactory) WithGetValuesFuncs(getValuesFuncs ...GetValuesFunc)
}

// WithInstallStrategy defines the installation strategy of the manifests prescribed by Manifests(..).
// Deprecated: add annotation "addon.open-cluster-management.io/lifecycle: addon-manager" to ClusterManagementAddon
// and define install strategy in ClusterManagementAddon spec.installStrategy instead.
// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
func (f *AgentAddonFactory) WithInstallStrategy(strategy *agent.InstallStrategy) *AgentAddonFactory {
if strategy.InstallNamespace == "" {
strategy.InstallNamespace = AddonDefaultInstallNamespace
Expand Down
131 changes: 131 additions & 0 deletions pkg/addonmanager/controllers/managementaddon/controller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package managementaddon

import (
"context"
"encoding/json"
"fmt"

jsonpatch "github.com/evanphx/json-patch"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
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/addon-framework/pkg/agent"
"open-cluster-management.io/addon-framework/pkg/basecontroller/factory"
)

const (
controllerName = "management-addon-controller"
)

// clusterManagementAddonController reconciles cma on the hub.
type clusterManagementAddonController struct {
addonClient addonv1alpha1client.Interface
clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
agentAddons map[string]agent.AgentAddon
addonFilterFunc factory.EventFilterFunc
}

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

c := &clusterManagementAddonController{
addonClient: addonClient,
clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
agentAddons: agentAddons,
addonFilterFunc: addonFilterFunc,
}

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 *clusterManagementAddonController) 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
}

addon := c.agentAddons[cma.GetName()]
if addon.GetAgentAddonOptions().InstallStrategy == nil {
return nil
}

// If the addon defines install strategy via WithInstallStrategy(), force add annotation "addon.open-cluster-management.io/lifecycle: self" to cma.
// The annotation with value "self" will be removed when remove WithInstallStrategy() 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 = map[string]string{}
}
cmaCopy.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] = addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue

err = c.patchMgmtAddonAnnotations(ctx, cmaCopy, cma)
return err
}

func (c *clusterManagementAddonController) patchMgmtAddonAnnotations(ctx context.Context, new, old *addonapiv1alpha1.ClusterManagementAddOn) error {
if equality.Semantic.DeepEqual(new.Annotations, old.Annotations) {
return nil
}

oldData, err := json.Marshal(&addonapiv1alpha1.ClusterManagementAddOn{
ObjectMeta: metav1.ObjectMeta{
Annotations: old.Annotations,
},
})
if err != nil {
return err
}

newData, err := json.Marshal(&addonapiv1alpha1.ClusterManagementAddOn{
ObjectMeta: metav1.ObjectMeta{
UID: new.UID,
ResourceVersion: new.ResourceVersion,
Annotations: new.Annotations,
},
})
if err != nil {
return err
}

patchBytes, err := jsonpatch.CreateMergePatch(oldData, newData)
if err != nil {
return fmt.Errorf("failed to create patch for addon %s: %w", new.Name, err)
}

klog.V(2).Infof("Patching clustermanagementaddon %s annotations with %s", new.Name, string(patchBytes))
_, err = c.addonClient.AddonV1alpha1().ClusterManagementAddOns().Patch(
ctx, new.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{})
return err
}
147 changes: 147 additions & 0 deletions pkg/addonmanager/controllers/managementaddon/controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package managementaddon

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"
)

type testAgent struct {
name string
strategy *agent.InstallStrategy
}

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,
InstallStrategy: t.strategy,
}
}

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: "add annotation when uses install strategy",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
"test": "test",
})},
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] != addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue {
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", strategy: agent.InstallAllStrategy("test")},
},
},
{
name: "override annotation when uses install strategy",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
"test": "test",
addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue,
})},
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] != addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue {
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", strategy: agent.InstallAllStrategy("test")},
},
},
{
name: "no patch annotation if managed by self",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
"test": "test",
addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue,
})},
validateAddonActions: addontesting.AssertNoActions,
testaddons: map[string]agent.AgentAddon{
"test": &testAgent{name: "test", strategy: agent.InstallAllStrategy("test")},
},
},
{
name: "no patch annotation if no install strategy",
cma: []runtime.Object{newClusterManagementAddonWithAnnotation("test", map[string]string{
"test": "test",
addonapiv1alpha1.AddonLifecycleAnnotationKey: addonapiv1alpha1.AddonLifecycleAddonManagerAnnotationValue,
})},
validateAddonActions: addontesting.AssertNoActions,
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 := clusterManagementAddonController{
addonClient: fakeAddonClient,
clusterManagementAddonLister: addonInformers.Addon().V1alpha1().ClusterManagementAddOns().Lister(),
agentAddons: c.testaddons,
addonFilterFunc: utils.FilterByAddonName(c.testaddons),
}

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())
})
}
}
12 changes: 12 additions & 0 deletions pkg/addonmanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/addoninstall"
"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/managementaddon"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/managementaddonconfig"
"open-cluster-management.io/addon-framework/pkg/addonmanager/controllers/registration"
"open-cluster-management.io/addon-framework/pkg/agent"
Expand Down Expand Up @@ -247,6 +248,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 := managementaddon.NewManagementAddonController(
addonClient,
addonInformers.Addon().V1alpha1().ClusterManagementAddOns(),
a.addonAgents,
utils.FilterByAddonName(a.addonAgents),
)

// This is a duplicate controller in general addon-manager. This should be removed when we
// alway enable the addon-manager
addonOwnerController := addonowner.NewAddonOwnerController(
Expand Down Expand Up @@ -324,6 +335,7 @@ func (a *addonManager) StartWithInformers(ctx context.Context,
go deployController.Run(ctx, 1)
go registrationController.Run(ctx, 1)
go addonInstallController.Run(ctx, 1)
go managementAddonController.Run(ctx, 1)

go addonOwnerController.Run(ctx, 1)
if addonConfigController != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/agent/inteface.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type AgentAddonOptions struct {
// Addon will not be installed automatically until a ManagedClusterAddon is applied to the cluster's
// namespace if InstallStrategy is nil.
// Deprecated: use installStrategy config in ClusterManagementAddOn API instead
// The migration plan refer to https://github.com/open-cluster-management-io/ocm/issues/355.
// +optional
InstallStrategy *InstallStrategy

Expand Down
14 changes: 14 additions & 0 deletions test/e2e/helloworld_helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ var _ = ginkgo.Describe("install/uninstall helloworld helm addons", func() {
})

ginkgo.It("addon should be available", func() {
ginkgo.By("Make sure cma annotation is not added since no install strategy defined")
gomega.Eventually(func() error {
cma, err := hubAddOnClient.AddonV1alpha1().ClusterManagementAddOns().Get(context.Background(), helloWorldHelmAddonName, metav1.GetOptions{})
if err != nil {
return err
}

if _, exist := cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey]; exist {
return fmt.Errorf("addon should not have annotation, but get %v", cma.Annotations)
}

return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

ginkgo.By("Make sure addon is available and has pre-delete finalizer")
gomega.Eventually(func() error {
addon, err := hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.Background(), helloWorldHelmAddonName, metav1.GetOptions{})
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/helloworld_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ var _ = ginkgo.Describe("install/uninstall helloworld addons", func() {
})

ginkgo.It("addon should be worked", func() {
ginkgo.By("Make sure cma annotation managed by self is added")
gomega.Eventually(func() error {
cma, err := hubAddOnClient.AddonV1alpha1().ClusterManagementAddOns().Get(context.Background(), addonName, metav1.GetOptions{})
if err != nil {
return err
}

if cma.Annotations[addonapiv1alpha1.AddonLifecycleAnnotationKey] != addonapiv1alpha1.AddonLifecycleSelfManageAnnotationValue {
return fmt.Errorf("addon should have annotation, but get %v", cma.Annotations)
}

return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

ginkgo.By("Make sure addon is available")
gomega.Eventually(func() error {
addon, err := hubAddOnClient.AddonV1alpha1().ManagedClusterAddOns(managedClusterName).Get(context.Background(), addonName, metav1.GetOptions{})
Expand Down

0 comments on commit 0221149

Please sign in to comment.