stop rendering manifests when addon configured condition is not true

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

examples/helloworld/helloworld_test.go

@@ -100,6 +100,13 @@ func TestManifestAddonAgent(t *testing.T) {
 							Namespace: "cluster1",
 							Name:      "config",
 						},
+						DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{
+							ConfigReferent: addonapiv1alpha1.ConfigReferent{
+								Namespace: "cluster1",
+								Name:      "config",
+							},
+							SpecHash: "fake-spec-hash",
+						},
 					},
 				}
 				return addon

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.20241008081048-f6c658202790
 	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.20241008081048-f6c658202790 h1:XszHWAR6PhYXBFPN4qgk8D5HVl8W/61j+bNMsXVuW7U=
+open-cluster-management.io/api v0.14.1-0.20241008081048-f6c658202790/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/addonfactory.go

@@ -110,6 +110,12 @@ func (f *AgentAddonFactory) WithAgentHostedInfoFn(
 	return f
 }
 
+// WithConfigCheckEnabledOption will enable the configured condition check before rendering manifests.
+func (f *AgentAddonFactory) WithConfigCheckEnabledOption() *AgentAddonFactory {
+	f.agentAddonOptions.ConfigCheckEnabled = true
+	return f
+}
+
 // WithTrimCRDDescription is to enable trim the description of CRDs in manifestWork.
 func (f *AgentAddonFactory) WithTrimCRDDescription() *AgentAddonFactory {
 	f.trimCRDDescription = true

pkg/addonmanager/controllers/agentdeploy/controller.go

@@ -381,6 +381,12 @@ func (c *addonDeployController) buildDeployManifestWorksFunc(addonWorkBuilder *a
 			return nil, nil, fmt.Errorf("failed to get agentAddon")
 		}
 
+		if agentAddon.GetAgentAddonOptions().ConfigCheckEnabled &&
+			!meta.IsStatusConditionTrue(addon.Status.Conditions, addonapiv1alpha1.ManagedClusterAddOnConditionConfigured) {
+			klog.InfoS("Addon configured condition is not set in status", "addonName", addon.Name)
+			return nil, nil, nil
+		}
+
 		objects, err := agentAddon.Manifests(cluster, addon)
 		if err != nil {
 			meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
@@ -443,6 +449,12 @@ func (c *addonDeployController) buildHookManifestWorkFunc(addonWorkBuilder *addo
 			return nil, fmt.Errorf("failed to get agentAddon")
 		}
 
+		if agentAddon.GetAgentAddonOptions().ConfigCheckEnabled &&
+			!meta.IsStatusConditionTrue(addon.Status.Conditions, addonapiv1alpha1.ManagedClusterAddOnConditionConfigured) {
+			klog.InfoS("Addon configured condition is not set in status", "addonName", addon.Name)
+			return nil, nil
+		}
+
 		objects, err := agentAddon.Manifests(cluster, addon)
 		if err != nil {
 			meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{

pkg/addonmanager/controllers/agentdeploy/default_hook_sync_test.go

@@ -379,6 +379,43 @@ func TestDefaultHookReconcile(t *testing.T) {
 				addontesting.AssertActions(t, actions, "update")
 			},
 		},
+		{
+			name:    "deploy hook manifest when ConfigCheckEnabled is true",
+			key:     "cluster1/test",
+			addon:   []runtime.Object{addontesting.NewAddonWithConditions("test", "cluster1", registrationAppliedCondition, configuredCondition)},
+			cluster: []runtime.Object{addontesting.NewManagedCluster("cluster1")},
+			testaddon: &testAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewUnstructured("v1", "ConfigMap", "default", "test"),
+				addontesting.NewHookJob("default", "test")}, ConfigCheckEnabled: true},
+			validateWorkActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "create")
+				actual := actions[0].(clienttesting.CreateActionImpl).Object
+				deployWork := actual.(*workapiv1.ManifestWork)
+				if deployWork.Namespace != "cluster1" || deployWork.Name != fmt.Sprintf("%s-%d", constants.DeployWorkNamePrefix("test"), 0) {
+					t.Errorf("the deployWork %v/%v is incorrect.", deployWork.Namespace, deployWork.Name)
+				}
+			},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "update")
+				actual := actions[0].(clienttesting.UpdateActionImpl).Object
+				addOn := actual.(*addonapiv1alpha1.ManagedClusterAddOn)
+				if !addonHasFinalizer(addOn, addonapiv1alpha1.AddonPreDeleteHookFinalizer) {
+					t.Errorf("the preDeleteHookFinalizer should be added.")
+				}
+			},
+		},
+		{
+			name:    "not deploy hook manifest when ConfigCheckEnabled is true",
+			key:     "cluster1/test",
+			addon:   []runtime.Object{addontesting.NewAddonWithConditions("test", "cluster1", registrationAppliedCondition)},
+			cluster: []runtime.Object{addontesting.NewManagedCluster("cluster1")},
+			testaddon: &testAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewUnstructured("v1", "ConfigMap", "default", "test"),
+				addontesting.NewHookJob("default", "test")},
+				ConfigCheckEnabled: true},
+			validateWorkActions:  addontesting.AssertNoActions,
+			validateAddonActions: addontesting.AssertNoActions,
+		},
 	}
 
 	for _, c := range cases {
@@ -431,8 +468,11 @@ func TestDefaultHookReconcile(t *testing.T) {
 
 			syncContext := addontesting.NewFakeSyncContext(t)
 			err = controller.sync(context.TODO(), syncContext, c.key)
-			if err != nil {
-				t.Errorf("expected no error when sync: %v", err)
+			if (err == nil && c.testaddon.err != nil) || (err != nil && c.testaddon.err == nil) {
+				t.Errorf("expected error %v when sync got %v", c.testaddon.err, err)
+			}
+			if err != nil && c.testaddon.err != nil && err.Error() != c.testaddon.err.Error() {
+				t.Errorf("expected error %v when sync got %v", c.testaddon.err, err)
 			}
 			c.validateAddonActions(t, fakeAddonClient.Actions())
 			c.validateWorkActions(t, fakeWorkClient.Actions())

pkg/addonmanager/controllers/agentdeploy/default_sync_test.go

@@ -43,13 +43,21 @@ var mainfestWorkAppliedCondition = metav1.Condition{
 	Message: "Registration of the addon agent is configured",
 }
 
+var configuredCondition = metav1.Condition{
+	Type:    addonapiv1alpha1.ManagedClusterAddOnConditionConfigured,
+	Status:  metav1.ConditionTrue,
+	Reason:  "ConfigurationsConfigured",
+	Message: "Configurations configured",
+}
+
 type testAgent struct {
-	name            string
-	objects         []runtime.Object
-	err             error
-	healthProber    *agent.HealthProber
-	Updaters        []agent.Updater
-	ManifestConfigs []workapiv1.ManifestConfigOption
+	name               string
+	objects            []runtime.Object
+	err                error
+	healthProber       *agent.HealthProber
+	Updaters           []agent.Updater
+	ManifestConfigs    []workapiv1.ManifestConfigOption
+	ConfigCheckEnabled bool
 }
 
 func (t *testAgent) Manifests(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) ([]runtime.Object, error) {
@@ -58,10 +66,11 @@ func (t *testAgent) Manifests(cluster *clusterv1.ManagedCluster, addon *addonapi
 
 func (t *testAgent) GetAgentAddonOptions() agent.AgentAddonOptions {
 	return agent.AgentAddonOptions{
-		AddonName:       t.name,
-		HealthProber:    t.healthProber,
-		Updaters:        t.Updaters,
-		ManifestConfigs: t.ManifestConfigs,
+		AddonName:          t.name,
+		HealthProber:       t.healthProber,
+		Updaters:           t.Updaters,
+		ManifestConfigs:    t.ManifestConfigs,
+		ConfigCheckEnabled: t.ConfigCheckEnabled,
 	}
 }
 
@@ -248,6 +257,30 @@ func TestDefaultReconcile(t *testing.T) {
 				}
 			},
 		},
+		{
+			name:    "deploy manifests for an addon when ConfigCheckEnabled is true",
+			key:     "cluster1/test",
+			addon:   []runtime.Object{addontesting.NewAddonWithConditions("test", "cluster1", registrationAppliedCondition, configuredCondition)},
+			cluster: []runtime.Object{addontesting.NewManagedCluster("cluster1")},
+			testaddon: &testAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewUnstructured("v1", "ConfigMap", "default", "test"),
+			}, ConfigCheckEnabled: true},
+			validateAddonActions: addontesting.AssertNoActions,
+			validateWorkActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "create")
+			},
+		},
+		{
+			name:    "not deploy manifests for an addon when ConfigCheckEnabled is true",
+			key:     "cluster1/test",
+			addon:   []runtime.Object{addontesting.NewAddonWithConditions("test", "cluster1", registrationAppliedCondition)},
+			cluster: []runtime.Object{addontesting.NewManagedCluster("cluster1")},
+			testaddon: &testAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewUnstructured("v1", "ConfigMap", "default", "test"),
+			}, ConfigCheckEnabled: true},
+			validateAddonActions: addontesting.AssertNoActions,
+			validateWorkActions:  addontesting.AssertNoActions,
+		},
 	}
 
 	for _, c := range cases {

pkg/addonmanager/controllers/agentdeploy/hosted_hook_sync_test.go

@@ -413,6 +413,64 @@ func TestHostingHookReconcile(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "deploy hook manifest when ConfigCheckEnabled is true",
+			key:  "cluster1/test",
+			addon: []runtime.Object{
+				addontesting.NewHostedModeAddonWithFinalizer("test", "cluster1", "cluster2",
+					registrationAppliedCondition, configuredCondition)},
+			cluster: []runtime.Object{
+				addontesting.NewManagedCluster("cluster1"),
+				addontesting.NewManagedCluster("cluster2"),
+			},
+			testaddon: &testHostedAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewHostingUnstructured("v1", "ConfigMap", "default", "test"),
+				addontesting.NewHostedHookJob("test", "default"),
+			}, ConfigCheckEnabled: true},
+			validateWorkActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "create")
+			},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "update")
+				actual := actions[0].(clienttesting.UpdateActionImpl).Object
+				addOn := actual.(*addonapiv1alpha1.ManagedClusterAddOn)
+				if !addonHasFinalizer(addOn, addonapiv1alpha1.AddonHostingPreDeleteHookFinalizer) {
+					t.Errorf("the preDeleteHookFinalizer should be added.")
+				}
+			},
+		},
+		{
+			name: "not deploy hook manifest when ConfigCheckEnabled is true",
+			key:  "cluster1/test",
+			addon: []runtime.Object{
+				addontesting.NewHostedModeAddonWithFinalizer("test", "cluster1", "cluster2",
+					registrationAppliedCondition)},
+			cluster: []runtime.Object{
+				addontesting.NewManagedCluster("cluster1"),
+				addontesting.NewManagedCluster("cluster2"),
+			},
+			testaddon: &testHostedAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewHostingUnstructured("v1", "ConfigMap", "default", "test"),
+				addontesting.NewHostedHookJob("test", "default"),
+			}, ConfigCheckEnabled: true},
+			validateWorkActions: addontesting.AssertNoActions,
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "patch")
+				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				addOn := &addonapiv1alpha1.ManagedClusterAddOn{}
+				err := json.Unmarshal(patch, addOn)
+				if err != nil {
+					t.Fatal(err)
+				}
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, addonapiv1alpha1.ManagedClusterAddOnHostingClusterValidity)
+				if addOnCond == nil {
+					t.Fatal("condition should not be nil")
+				}
+				if addOnCond.Reason != addonapiv1alpha1.HostingClusterValidityReasonValid {
+					t.Errorf("Condition Reason is not correct: %v", addOnCond.Reason)
+				}
+			},
+		},
 	}
 
 	for _, c := range cases {

pkg/addonmanager/controllers/agentdeploy/hosted_sync_test.go

@@ -30,9 +30,10 @@ import (
 )
 
 type testHostedAgent struct {
-	name    string
-	objects []runtime.Object
-	err     error
+	name               string
+	objects            []runtime.Object
+	err                error
+	ConfigCheckEnabled bool
 }
 
 func (t *testHostedAgent) Manifests(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) (
@@ -45,6 +46,7 @@ func (t *testHostedAgent) GetAgentAddonOptions() agent.AgentAddonOptions {
 		AddonName:          t.name,
 		HostedModeEnabled:  true,
 		HostedModeInfoFunc: constants.GetHostedModeInfo,
+		ConfigCheckEnabled: t.ConfigCheckEnabled,
 	}
 }
 
@@ -398,6 +400,72 @@ func TestHostingReconcile(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "deploy manifests for an addon when ConfigCheckEnabled is true",
+			key:  "cluster1/test",
+			addon: []runtime.Object{addontesting.NewHostedModeAddonWithFinalizer("test", "cluster1", "cluster2",
+				registrationAppliedCondition, configuredCondition)},
+			cluster: []runtime.Object{
+				addontesting.NewManagedCluster("cluster1"),
+				addontesting.NewManagedCluster("cluster2"),
+			},
+			testaddon: &testHostedAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewHostingUnstructured("v1", "ConfigMap", "default", "test"),
+			}, ConfigCheckEnabled: true},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "patch")
+				assertHostingClusterValid(t, actions[0])
+
+				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				addOn := &addonapiv1alpha1.ManagedClusterAddOn{}
+				err := json.Unmarshal(patch, addOn)
+				if err != nil {
+					t.Fatal(err)
+				}
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, addonapiv1alpha1.ManagedClusterAddOnHostingClusterValidity)
+				if addOnCond == nil {
+					t.Fatal("condition should not be nil")
+				}
+				if addOnCond.Reason != addonapiv1alpha1.HostingClusterValidityReasonValid {
+					t.Errorf("Condition Reason is not correct: %v", addOnCond.Reason)
+				}
+			},
+			validateWorkActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "create")
+			},
+		},
+		{
+			name: "not deploy manifests for an addon when ConfigCheckEnabled is true",
+			key:  "cluster1/test",
+			addon: []runtime.Object{addontesting.NewHostedModeAddonWithFinalizer("test", "cluster1", "cluster2",
+				registrationAppliedCondition)},
+			cluster: []runtime.Object{
+				addontesting.NewManagedCluster("cluster1"),
+				addontesting.NewManagedCluster("cluster2"),
+			},
+			testaddon: &testHostedAgent{name: "test", objects: []runtime.Object{
+				addontesting.NewHostingUnstructured("v1", "ConfigMap", "default", "test"),
+			}, ConfigCheckEnabled: true},
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				addontesting.AssertActions(t, actions, "patch")
+				assertHostingClusterValid(t, actions[0])
+
+				patch := actions[0].(clienttesting.PatchActionImpl).Patch
+				addOn := &addonapiv1alpha1.ManagedClusterAddOn{}
+				err := json.Unmarshal(patch, addOn)
+				if err != nil {
+					t.Fatal(err)
+				}
+				addOnCond := meta.FindStatusCondition(addOn.Status.Conditions, addonapiv1alpha1.ManagedClusterAddOnHostingClusterValidity)
+				if addOnCond == nil {
+					t.Fatal("condition should not be nil")
+				}
+				if addOnCond.Reason != addonapiv1alpha1.HostingClusterValidityReasonValid {
+					t.Errorf("Condition Reason is not correct: %v", addOnCond.Reason)
+				}
+			},
+			validateWorkActions: addontesting.AssertNoActions,
+		},
 	}
 
 	for _, c := range cases {

pkg/agent/inteface.go

@@ -91,6 +91,11 @@ type AgentAddonOptions struct {
 	//   json path which is already in the existing rules, compare by the path name.
 	// +optional
 	ManifestConfigs []workapiv1.ManifestConfigOption
+
+	// ConfigCheckEnabled defines whether to check the configured condition before rendering manifests.
+	// If not set, will be defaulted to false.
+	// +optional
+	ConfigCheckEnabled bool
 }
 
 type CSRSignerFunc func(csr *certificatesv1.CertificateSigningRequest) []byte

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.20241008081048-f6c658202790
 ## explicit; go 1.22.0
 open-cluster-management.io/api/addon/v1alpha1
 open-cluster-management.io/api/client/addon/clientset/versioned