stop rendering manifests when addon configured condition is not true

Signed-off-by: haoqing0110 <[email protected]>

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,13 +66,53 @@ 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"),
 			managedClusterAddOn: func() *addonapiv1alpha1.ManagedClusterAddOn {
 				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)
 		}
 

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
 )

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=

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

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

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,
+			}},
+		},
 	}
 }

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