diff --git a/examples/helloworld/helloworld_test.go b/examples/helloworld/helloworld_test.go index fce28de7..b7968e3b 100644 --- a/examples/helloworld/helloworld_test.go +++ b/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 diff --git a/pkg/addonfactory/addonfactory.go b/pkg/addonfactory/addonfactory.go index d92690f7..aebad33d 100644 --- a/pkg/addonfactory/addonfactory.go +++ b/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 diff --git a/pkg/addonmanager/controllers/agentdeploy/controller.go b/pkg/addonmanager/controllers/agentdeploy/controller.go index ffcf30f9..4cb23a53 100644 --- a/pkg/addonmanager/controllers/agentdeploy/controller.go +++ b/pkg/addonmanager/controllers/agentdeploy/controller.go @@ -381,6 +381,11 @@ 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, fmt.Errorf("addon %s configured condition is not is not set in status", addon.Name) + } + objects, err := agentAddon.Manifests(cluster, addon) if err != nil { meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{ @@ -443,6 +448,11 @@ 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, fmt.Errorf("addon %s configured condition is not is not set in status", addon.Name) + } + objects, err := agentAddon.Manifests(cluster, addon) if err != nil { meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{ diff --git a/pkg/addonmanager/controllers/agentdeploy/default_hook_sync_test.go b/pkg/addonmanager/controllers/agentdeploy/default_hook_sync_test.go index 13056b67..ff472f42 100644 --- a/pkg/addonmanager/controllers/agentdeploy/default_hook_sync_test.go +++ b/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, err: fmt.Errorf("addon test configured condition is not is not set in status")}, + 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()) diff --git a/pkg/addonmanager/controllers/agentdeploy/default_sync_test.go b/pkg/addonmanager/controllers/agentdeploy/default_sync_test.go index 2a7fada5..7fee57fe 100644 --- a/pkg/addonmanager/controllers/agentdeploy/default_sync_test.go +++ b/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, err: fmt.Errorf("addon test configured condition is not is not set in status")}, + validateAddonActions: addontesting.AssertNoActions, + validateWorkActions: addontesting.AssertNoActions, + }, } for _, c := range cases { diff --git a/pkg/addonmanager/controllers/agentdeploy/hosted_hook_sync_test.go b/pkg/addonmanager/controllers/agentdeploy/hosted_hook_sync_test.go index 4da37bc3..811626ec 100644 --- a/pkg/addonmanager/controllers/agentdeploy/hosted_hook_sync_test.go +++ b/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, err: fmt.Errorf("addon test configured condition is not is not set in status")}, + 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 { diff --git a/pkg/addonmanager/controllers/agentdeploy/hosted_sync_test.go b/pkg/addonmanager/controllers/agentdeploy/hosted_sync_test.go index 79966a18..979a329c 100644 --- a/pkg/addonmanager/controllers/agentdeploy/hosted_sync_test.go +++ b/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, err: fmt.Errorf("addon test configured condition is not is not set in status")}, + 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 { diff --git a/pkg/agent/inteface.go b/pkg/agent/inteface.go index edbbc8b8..d2861cb6 100644 --- a/pkg/agent/inteface.go +++ b/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