From 3113daf5ae4c3715e9364d955507fb2e119460a6 Mon Sep 17 00:00:00 2001 From: haoqing0110 Date: Fri, 27 Sep 2024 09:22:11 +0000 Subject: [PATCH] stop rendering manifests when addon configured condition is not true Signed-off-by: haoqing0110 --- examples/helloworld/helloworld_test.go | 79 +++++++++++++++++-- go.mod | 2 +- go.sum | 4 +- pkg/addonfactory/template_agentaddon.go | 8 ++ pkg/addonfactory/template_agentaddon_test.go | 56 ++++++++++++- pkg/addonfactory/test_helper.go | 6 ++ vendor/modules.txt | 2 +- .../v1alpha1/types_managedclusteraddon.go | 1 - 8 files changed, 144 insertions(+), 14 deletions(-) diff --git a/examples/helloworld/helloworld_test.go b/examples/helloworld/helloworld_test.go index fce28de72..ab923e41f 100644 --- a/examples/helloworld/helloworld_test.go +++ b/examples/helloworld/helloworld_test.go @@ -1,6 +1,8 @@ package helloworld import ( + "fmt" + "strings" "testing" appsv1 "k8s.io/api/apps/v1" @@ -31,12 +33,20 @@ func TestManifestAddonAgent(t *testing.T) { managedClusterAddOn *addonapiv1alpha1.ManagedClusterAddOn configs []runtime.Object verifyDeployment func(t *testing.T, objs []runtime.Object) + expectedErr error }{ { - name: "no configs", - managedCluster: addontesting.NewManagedCluster("cluster1"), - managedClusterAddOn: addontesting.NewAddon("helloworld", "cluster1"), - configs: []runtime.Object{}, + name: "no configs", + managedCluster: addontesting.NewManagedCluster("cluster1"), + managedClusterAddOn: func() *addonapiv1alpha1.ManagedClusterAddOn { + addon := addontesting.NewAddon("helloworld", "cluster1") + addon.Status.Conditions = []metav1.Condition{{ + Type: addonapiv1alpha1.ManagedClusterAddOnConditionConfigured, + Status: metav1.ConditionTrue, + }} + return addon + }(), + configs: []runtime.Object{}, verifyDeployment: func(t *testing.T, objs []runtime.Object) { deployment := findHelloWorldDeployment(objs) if deployment == nil { @@ -56,6 +66,42 @@ func TestManifestAddonAgent(t *testing.T) { } }, }, + { + name: "no configured condition", + managedCluster: addontesting.NewManagedCluster("cluster1"), + managedClusterAddOn: func() *addonapiv1alpha1.ManagedClusterAddOn { + addon := addontesting.NewAddon("helloworld", "cluster1") + return addon + }(), + configs: []runtime.Object{}, + verifyDeployment: func(t *testing.T, objs []runtime.Object) { + deployment := findHelloWorldDeployment(objs) + if deployment != nil { + t.Fatalf("expected no deployment, but failed") + } + }, + expectedErr: fmt.Errorf("addon helloworld deployment config is not configured in status"), + }, + { + name: "false configured condition", + managedCluster: addontesting.NewManagedCluster("cluster1"), + managedClusterAddOn: func() *addonapiv1alpha1.ManagedClusterAddOn { + addon := addontesting.NewAddon("helloworld", "cluster1") + addon.Status.Conditions = []metav1.Condition{{ + Type: addonapiv1alpha1.ManagedClusterAddOnConditionConfigured, + Status: metav1.ConditionFalse, + }} + return addon + }(), + configs: []runtime.Object{}, + verifyDeployment: func(t *testing.T, objs []runtime.Object) { + deployment := findHelloWorldDeployment(objs) + if deployment != nil { + t.Fatalf("expected no deployment, but failed") + } + }, + expectedErr: fmt.Errorf("addon helloworld deployment config is not configured in status"), + }, { name: "override image with annotation", managedCluster: addontesting.NewManagedCluster("cluster1"), @@ -63,6 +109,10 @@ func TestManifestAddonAgent(t *testing.T) { addon := addontesting.NewAddon("test", "cluster1") addon.Annotations = map[string]string{ "addon.open-cluster-management.io/values": `{"Image":"quay.io/test:test"}`} + addon.Status.Conditions = []metav1.Condition{{ + Type: addonapiv1alpha1.ManagedClusterAddOnConditionConfigured, + Status: metav1.ConditionTrue, + }} return addon }(), configs: []runtime.Object{}, @@ -100,8 +150,19 @@ func TestManifestAddonAgent(t *testing.T) { Namespace: "cluster1", Name: "config", }, + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config", + }, + SpecHash: "fake-spec-hash", + }, }, } + addon.Status.Conditions = []metav1.Condition{{ + Type: addonapiv1alpha1.ManagedClusterAddOnConditionConfigured, + Status: metav1.ConditionTrue, + }} return addon }(), configs: []runtime.Object{ @@ -169,10 +230,16 @@ func TestManifestAddonAgent(t *testing.T) { objects, err := agentAddon.Manifests(c.managedCluster, c.managedClusterAddOn) if err != nil { - t.Fatalf("failed to get manifests %v", err) + if c.expectedErr == nil || !strings.EqualFold(err.Error(), c.expectedErr.Error()) { + t.Errorf("expected error %v, but got error %s", c.expectedErr, err) + } + } else { + if c.expectedErr != nil { + t.Errorf("expected error %v, but got no error", c.expectedErr) + } } - if len(objects) != 3 { + if c.expectedErr == nil && len(objects) != 3 { t.Fatalf("expected 3 manifests, but %v", objects) } diff --git a/go.mod b/go.mod index 3350152ad..25c2ab26b 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( k8s.io/component-base v0.30.2 k8s.io/klog/v2 v2.120.1 k8s.io/utils v0.0.0-20240310230437-4693a0247e57 - open-cluster-management.io/api v0.14.1-0.20240627145512-bd6f2229b53c + open-cluster-management.io/api v0.14.1-0.20240929023505-ab092a65ab63 open-cluster-management.io/sdk-go v0.14.1-0.20240628095929-9ffb1b19e566 sigs.k8s.io/controller-runtime v0.18.4 ) diff --git a/go.sum b/go.sum index 7073347ba..2ff053283 100644 --- a/go.sum +++ b/go.sum @@ -479,8 +479,8 @@ k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7F k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98= k8s.io/utils v0.0.0-20240310230437-4693a0247e57 h1:gbqbevonBh57eILzModw6mrkbwM0gQBEuevE/AaBsHY= k8s.io/utils v0.0.0-20240310230437-4693a0247e57/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= -open-cluster-management.io/api v0.14.1-0.20240627145512-bd6f2229b53c h1:gYfgkX/U6fv2d3Ly8D6N1GM9zokORupLSgCxx791zZw= -open-cluster-management.io/api v0.14.1-0.20240627145512-bd6f2229b53c/go.mod h1:9erZEWEn4bEqh0nIX2wA7f/s3KCuFycQdBrPrRzi0QM= +open-cluster-management.io/api v0.14.1-0.20240929023505-ab092a65ab63 h1:UV1OCtyt0EH/mgsdsvyxOPg9xva5pHjhGgAa0+gLpUM= +open-cluster-management.io/api v0.14.1-0.20240929023505-ab092a65ab63/go.mod h1:9erZEWEn4bEqh0nIX2wA7f/s3KCuFycQdBrPrRzi0QM= open-cluster-management.io/sdk-go v0.14.1-0.20240628095929-9ffb1b19e566 h1:8dgPiM3byX/rtOrFJIsea2haV4hSFTND65Tlj1EdK18= open-cluster-management.io/sdk-go v0.14.1-0.20240628095929-9ffb1b19e566/go.mod h1:xFmN3Db5nN68oLGnstmIRv4us8HJCdXFnBNMXVp0jWY= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.29.0 h1:/U5vjBbQn3RChhv7P11uhYvCSm5G2GaIi5AIGBS6r4c= diff --git a/pkg/addonfactory/template_agentaddon.go b/pkg/addonfactory/template_agentaddon.go index fd771aa16..e78a871bf 100644 --- a/pkg/addonfactory/template_agentaddon.go +++ b/pkg/addonfactory/template_agentaddon.go @@ -3,6 +3,8 @@ package addonfactory import ( "fmt" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/klog/v2" @@ -59,6 +61,12 @@ func (a *TemplateAgentAddon) Manifests( addon *addonapiv1alpha1.ManagedClusterAddOn) ([]runtime.Object, error) { var objects []runtime.Object + configCond := meta.FindStatusCondition(addon.Status.Conditions, addonapiv1alpha1.ManagedClusterAddOnConditionConfigured) + if configCond == nil || configCond.Status == metav1.ConditionFalse { + klog.InfoS("Addon deployment config is not configured in status", "addonName", addon.Name) + return nil, fmt.Errorf("addon %s deployment config is not configured in status", addon.Name) + } + configValues, err := a.getValues(cluster, addon) if err != nil { return objects, err diff --git a/pkg/addonfactory/template_agentaddon_test.go b/pkg/addonfactory/template_agentaddon_test.go index 713b2c8a5..451553b53 100644 --- a/pkg/addonfactory/template_agentaddon_test.go +++ b/pkg/addonfactory/template_agentaddon_test.go @@ -3,10 +3,13 @@ package addonfactory import ( "embed" "fmt" + "strings" "testing" appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "open-cluster-management.io/addon-framework/pkg/addonmanager/addontesting" "open-cluster-management.io/addon-framework/pkg/agent" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" clusterv1 "open-cluster-management.io/api/cluster/v1" @@ -36,6 +39,7 @@ func TestTemplateAddon_Manifests(t *testing.T) { scheme *runtime.Scheme clusterName string addonName string + addon *addonapiv1alpha1.ManagedClusterAddOn installNamespace string getValuesFunc GetValuesFunc annotationConfig string @@ -45,6 +49,7 @@ func TestTemplateAddon_Manifests(t *testing.T) { expectedObjectCnt int expectedHubKubeConfigSecret string expectedManagedKubeConfigSecret string + expectedErr error }{ { name: "template render ok with annotation config and default scheme", @@ -138,6 +143,38 @@ func TestTemplateAddon_Manifests(t *testing.T) { expectedHubKubeConfigSecret: "external-hub-kubeconfig", expectedManagedKubeConfigSecret: "external-managed-kubeconfig", }, + { + name: "template render failed without addon configured condition", + dir: "testmanifests/template", + scheme: scheme, + clusterName: "local-cluster", + addonName: "helloworld", + addon: func() *addonapiv1alpha1.ManagedClusterAddOn { + addon := addontesting.NewAddon("helloworld", "local-cluster") + return addon + }(), + annotationConfig: `{"NodeSelector":{"host":"ssd"},"Image":"quay.io/helloworld:2.4"}`, + expectedObjectCnt: 0, + expectedErr: fmt.Errorf("addon helloworld deployment config is not configured in status"), + }, + { + name: "template render failed with addon configured condition false", + dir: "testmanifests/template", + scheme: scheme, + clusterName: "local-cluster", + addonName: "helloworld", + addon: func() *addonapiv1alpha1.ManagedClusterAddOn { + addon := addontesting.NewAddon("helloworld", "local-cluster") + addon.Status.Conditions = []metav1.Condition{{ + Type: addonapiv1alpha1.ManagedClusterAddOnConditionConfigured, + Status: metav1.ConditionFalse, + }} + return addon + }(), + annotationConfig: `{"NodeSelector":{"host":"ssd"},"Image":"quay.io/helloworld:2.4"}`, + expectedObjectCnt: 0, + expectedErr: fmt.Errorf("addon helloworld deployment config is not configured in status"), + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -150,8 +187,13 @@ func TestTemplateAddon_Manifests(t *testing.T) { } cluster := NewFakeManagedCluster(c.clusterName, "1.10.1") - clusterAddon := NewFakeManagedClusterAddon(c.addonName, c.clusterName, c.installNamespace, - c.annotationConfig) + var clusterAddon *addonapiv1alpha1.ManagedClusterAddOn + if c.addon != nil { + clusterAddon = c.addon + } else { + clusterAddon = NewFakeManagedClusterAddon(c.addonName, c.clusterName, c.installNamespace, + c.annotationConfig) + } agentAddon, err := NewAgentAddonFactory(c.addonName, templateFS, c.dir). WithScheme(c.scheme). @@ -161,10 +203,18 @@ func TestTemplateAddon_Manifests(t *testing.T) { if err != nil { t.Errorf("expected no error, got err %v", err) } + objects, err := agentAddon.Manifests(cluster, clusterAddon) if err != nil { - t.Errorf("expected no error, got err %v", err) + if c.expectedErr == nil || !strings.EqualFold(err.Error(), c.expectedErr.Error()) { + t.Errorf("expected error %v, but got error %s", c.expectedErr, err) + } + } else { + if c.expectedErr != nil { + t.Errorf("expected error %v, but got no error", c.expectedErr) + } } + if len(objects) != c.expectedObjectCnt { t.Errorf("expected %v objects, but got %v", c.expectedObjectCnt, len(objects)) } diff --git a/pkg/addonfactory/test_helper.go b/pkg/addonfactory/test_helper.go index 942814163..f799ae6a6 100644 --- a/pkg/addonfactory/test_helper.go +++ b/pkg/addonfactory/test_helper.go @@ -27,5 +27,11 @@ func NewFakeManagedClusterAddon(name, clusterName, installNamespace, values stri }, }, Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{InstallNamespace: installNamespace}, + Status: addonapiv1alpha1.ManagedClusterAddOnStatus{ + Conditions: []metav1.Condition{{ + Type: addonapiv1alpha1.ManagedClusterAddOnConditionConfigured, + Status: metav1.ConditionTrue, + }}, + }, } } diff --git a/vendor/modules.txt b/vendor/modules.txt index f8139c9be..a5db1a7d5 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1352,7 +1352,7 @@ k8s.io/utils/pointer k8s.io/utils/ptr k8s.io/utils/strings/slices k8s.io/utils/trace -# open-cluster-management.io/api v0.14.1-0.20240627145512-bd6f2229b53c +# open-cluster-management.io/api v0.14.1-0.20240929023505-ab092a65ab63 ## explicit; go 1.22.0 open-cluster-management.io/api/addon/v1alpha1 open-cluster-management.io/api/client/addon/clientset/versioned diff --git a/vendor/open-cluster-management.io/api/addon/v1alpha1/types_managedclusteraddon.go b/vendor/open-cluster-management.io/api/addon/v1alpha1/types_managedclusteraddon.go index 82ad206c5..1ed2134bc 100644 --- a/vendor/open-cluster-management.io/api/addon/v1alpha1/types_managedclusteraddon.go +++ b/vendor/open-cluster-management.io/api/addon/v1alpha1/types_managedclusteraddon.go @@ -318,7 +318,6 @@ const ( // the managed cluster. ManagedClusterAddOnConditionDegraded string = "Degraded" - // Deprecated: Use ManagedClusterAddOnConditionProgressing instead // ManagedClusterAddOnConditionConfigured represents that the addon agent is configured with its configuration ManagedClusterAddOnConditionConfigured string = "Configured"