Skip to content

Commit c18f49f

Browse files
zhujian7claude
andauthored
Set explicit default namespace for addon registration (#338)
- Set default Status.Namespace to "open-cluster-management-agent-addon" - Clarify namespace priority order with explicit default value - Add test case for default namespace behavior - Improve test error messages to show actual vs expected values This ensures a consistent default namespace even when InstallNamespace is deprecated and agentInstallNamespace is empty. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: zhujian <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent cbcc715 commit c18f49f

File tree

2 files changed

+53
-13
lines changed

2 files changed

+53
-13
lines changed

pkg/addonmanager/controllers/registration/controller.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,21 +185,28 @@ func (c *addonRegistrationController) sync(ctx context.Context, syncCtx factory.
185185
}
186186
managedClusterAddonCopy.Status.Registrations = configs
187187

188-
var agentInstallNamespace string
188+
// explicitly set the default namespace value, since the mca.spec.installNamespace is deprceated and
189+
// the addonDeploymentConfig.spec.agentInstallNamespace could be empty
190+
managedClusterAddonCopy.Status.Namespace = "open-cluster-management-agent-addon"
191+
192+
// Set the default namespace to registrationOption.Namespace
193+
if len(registrationOption.Namespace) > 0 {
194+
managedClusterAddonCopy.Status.Namespace = registrationOption.Namespace
195+
}
196+
197+
if len(managedClusterAddonCopy.Spec.InstallNamespace) > 0 {
198+
managedClusterAddonCopy.Status.Namespace = managedClusterAddonCopy.Spec.InstallNamespace
199+
}
200+
189201
if registrationOption.AgentInstallNamespace != nil {
190-
agentInstallNamespace, err = registrationOption.AgentInstallNamespace(managedClusterAddonCopy)
202+
ns, err := registrationOption.AgentInstallNamespace(managedClusterAddonCopy)
191203
if err != nil {
192204
return err
193205
}
194-
}
195-
196-
// Set the default namespace to registrationOption.Namespace
197-
managedClusterAddonCopy.Status.Namespace = registrationOption.Namespace
198-
// Override if agentInstallNamespace or InstallNamespace is specified
199-
if len(agentInstallNamespace) > 0 {
200-
managedClusterAddonCopy.Status.Namespace = agentInstallNamespace
201-
} else if len(managedClusterAddonCopy.Spec.InstallNamespace) > 0 {
202-
managedClusterAddonCopy.Status.Namespace = managedClusterAddonCopy.Spec.InstallNamespace
206+
// Override if agentInstallNamespace or InstallNamespace is specified
207+
if len(ns) > 0 {
208+
managedClusterAddonCopy.Status.Namespace = ns
209+
}
203210
}
204211

205212
meta.SetStatusCondition(&managedClusterAddonCopy.Status.Conditions, metav1.Condition{

pkg/addonmanager/controllers/registration/controller_test.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func TestReconcile(t *testing.T) {
173173
t.Errorf("Registration config is not updated")
174174
}
175175
if addOn.Status.Namespace != "default2" {
176-
t.Errorf("Namespace in status is not correct")
176+
t.Errorf("Namespace %s in status is not correct default2", addOn.Status.Namespace)
177177
}
178178
},
179179
testaddon: &testAgent{name: "test", namespace: "default", registrations: []addonapiv1alpha1.RegistrationConfig{
@@ -207,7 +207,7 @@ func TestReconcile(t *testing.T) {
207207
t.Errorf("Registration config is not updated")
208208
}
209209
if addOn.Status.Namespace != "default3" {
210-
t.Errorf("Namespace in status is not correct")
210+
t.Errorf("Namespace %s in status is not correct default3", addOn.Status.Namespace)
211211
}
212212
},
213213
testaddon: &testAgent{name: "test", namespace: "default",
@@ -217,6 +217,39 @@ func TestReconcile(t *testing.T) {
217217
},
218218
},
219219
},
220+
{
221+
name: "default namespace when no namespace is specified",
222+
cluster: []runtime.Object{addontesting.NewManagedCluster("cluster1")},
223+
addon: []runtime.Object{
224+
func() *addonapiv1alpha1.ManagedClusterAddOn {
225+
addon := addontesting.NewAddon("test", "cluster1", metav1.OwnerReference{
226+
Kind: "ClusterManagementAddOn",
227+
Name: "test",
228+
})
229+
return addon
230+
}(),
231+
},
232+
validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
233+
addontesting.AssertActions(t, actions, "patch")
234+
actual := actions[0].(clienttesting.PatchActionImpl).Patch
235+
addOn := &addonapiv1alpha1.ManagedClusterAddOn{}
236+
err := json.Unmarshal(actual, addOn)
237+
if err != nil {
238+
t.Fatal(err)
239+
}
240+
if addOn.Status.Registrations[0].SignerName != "test" {
241+
t.Errorf("Registration config is not updated")
242+
}
243+
if addOn.Status.Namespace != "open-cluster-management-agent-addon" {
244+
t.Errorf("Namespace %s in status is not correct, expected open-cluster-management-agent-addon", addOn.Status.Namespace)
245+
}
246+
},
247+
testaddon: &testAgent{name: "test", namespace: "", registrations: []addonapiv1alpha1.RegistrationConfig{
248+
{
249+
SignerName: "test",
250+
},
251+
}},
252+
},
220253
}
221254

222255
for _, c := range cases {

0 commit comments

Comments
 (0)