Skip to content

Commit 3120a30

Browse files
zhujian7zhujian7
committed
use managedclustraddon install namespace when addondeploymentconfig not set
Signed-off-by: zhujian <[email protected]>
1 parent fb5ba3a commit 3120a30

File tree

4 files changed

+167
-42
lines changed

4 files changed

+167
-42
lines changed

‎pkg/addon/templateagent/decorator.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type namespaceDecorator struct {
2929
paths map[string][]string
3030
}
3131

32-
func newNamespaceDecorator(privateValues addonfactory.Values) *namespaceDecorator {
32+
func newNamespaceDecorator(defaultNs string, privateValues addonfactory.Values) *namespaceDecorator {
3333
decorator := &namespaceDecorator{
3434
paths: map[string][]string{
3535
"ClusterRoleBinding": {"subjects", "namespace"},
@@ -40,6 +40,8 @@ func newNamespaceDecorator(privateValues addonfactory.Values) *namespaceDecorato
4040
namespace, ok := privateValues[InstallNamespacePrivateValueKey]
4141
if ok {
4242
decorator.installNamespace = namespace.(string)
43+
} else if len(defaultNs) != 0 {
44+
decorator.installNamespace = defaultNs
4345
}
4446

4547
return decorator

‎pkg/addon/templateagent/decorator_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
)
2020

2121
func TestNamespaceDecorator(t *testing.T) {
22+
defaultNs := "ocm-addon"
2223
tests := []struct {
2324
name string
2425
namespace string
@@ -29,7 +30,7 @@ func TestNamespaceDecorator(t *testing.T) {
2930
name: "no namespace set",
3031
object: testingcommon.NewUnstructured("v1", "Pod", "default", "test"),
3132
validateObject: func(t *testing.T, obj *unstructured.Unstructured) {
32-
testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", "default")
33+
testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", defaultNs)
3334
},
3435
},
3536
{
@@ -153,7 +154,7 @@ func TestNamespaceDecorator(t *testing.T) {
153154
values[InstallNamespacePrivateValueKey] = tc.namespace
154155
}
155156

156-
d := newNamespaceDecorator(values)
157+
d := newNamespaceDecorator(defaultNs, values)
157158
result, err := d.decorate(tc.object)
158159
if err != nil {
159160
t.Fatal(err)

‎pkg/addon/templateagent/template_agent.go

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -189,14 +189,14 @@ func (a *CRDTemplateAgentAddon) renderObjects(
189189
return objects, err
190190
}
191191

192-
object, err = a.decorateObject(template, object, presetValues, privateValues)
192+
object, err = a.decorateObject(addon, template, object, presetValues, privateValues)
193193
if err != nil {
194194
return objects, err
195195
}
196196
objects = append(objects, object)
197197
}
198198

199-
additionalObjects, err := a.injectAdditionalObjects(template, presetValues, privateValues)
199+
additionalObjects, err := a.injectAdditionalObjects(addon, template, presetValues, privateValues)
200200
if err != nil {
201201
return objects, err
202202
}
@@ -206,14 +206,16 @@ func (a *CRDTemplateAgentAddon) renderObjects(
206206
}
207207

208208
func (a *CRDTemplateAgentAddon) decorateObject(
209+
addon *addonapiv1alpha1.ManagedClusterAddOn,
209210
template *addonapiv1alpha1.AddOnTemplate,
210211
obj *unstructured.Unstructured,
211212
orderedValues orderedValues,
212-
privateValues addonfactory.Values) (*unstructured.Unstructured, error) {
213+
privateValues addonfactory.Values,
214+
) (*unstructured.Unstructured, error) {
213215
decorators := []decorator{
214216
newDeploymentDecorator(a.logger, a.addonName, template, orderedValues, privateValues),
215217
newDaemonSetDecorator(a.logger, a.addonName, template, orderedValues, privateValues),
216-
newNamespaceDecorator(privateValues),
218+
newNamespaceDecorator(addon.Spec.InstallNamespace, privateValues),
217219
}
218220

219221
var err error
@@ -228,6 +230,7 @@ func (a *CRDTemplateAgentAddon) decorateObject(
228230
}
229231

230232
func (a *CRDTemplateAgentAddon) injectAdditionalObjects(
233+
addon *addonapiv1alpha1.ManagedClusterAddOn,
231234
template *addonapiv1alpha1.AddOnTemplate,
232235
orderedValues orderedValues,
233236
privateValues addonfactory.Values) ([]runtime.Object, error) {
@@ -237,7 +240,7 @@ func (a *CRDTemplateAgentAddon) injectAdditionalObjects(
237240

238241
decorators := []decorator{
239242
// decorate the namespace of the additional objects
240-
newNamespaceDecorator(privateValues),
243+
newNamespaceDecorator(addon.Spec.InstallNamespace, privateValues),
241244
}
242245

243246
var objs []runtime.Object
@@ -296,54 +299,60 @@ func (a *CRDTemplateAgentAddon) getDesiredAddOnTemplateInner(
296299
return template.DeepCopy(), nil
297300
}
298301

299-
// TemplateAgentRegistrationNamespaceFunc reads deployment/daemonset resources in the manifests and use that namespace
300-
// as the default registration namespace. If addonDeploymentConfig is set, uses the namespace in it.
302+
// TemplateAgentRegistrationNamespaceFunc reads the registration namespace according to the following
303+
// priorities, from high to low:
304+
// 1. namespace configured in the addonDeploymentConfig
305+
// 2. managedClusterAddon.spec.installNamespace
306+
// 3. namespace of deployment/daemonset resources defined in the addonTemplated manifests
301307
func (a *CRDTemplateAgentAddon) TemplateAgentRegistrationNamespaceFunc(
302308
addon *addonapiv1alpha1.ManagedClusterAddOn) (string, error) {
303-
template, err := a.getDesiredAddOnTemplateInner(addon.Name, addon.Status.ConfigReferences)
304-
if err != nil {
305-
return "", err
306-
}
307-
if template == nil {
308-
return "", fmt.Errorf("addon %s template not found in status", addon.Name)
309-
}
310309

311-
// pick the namespace of the first deployment, if there is no deployment, pick the namespace of the first daemonset
312-
var desiredNS = "open-cluster-management-agent-addon"
313-
var firstDeploymentNamespace, firstDaemonSetNamespace string
314-
for _, manifest := range template.Spec.AgentSpec.Workload.Manifests {
315-
object := &unstructured.Unstructured{}
316-
if err := object.UnmarshalJSON(manifest.Raw); err != nil {
317-
a.logger.Error(err, "failed to extract the object")
318-
continue
310+
desiredNs := addon.Spec.InstallNamespace
311+
if len(desiredNs) == 0 {
312+
template, err := a.getDesiredAddOnTemplateInner(addon.Name, addon.Status.ConfigReferences)
313+
if err != nil {
314+
return "", err
315+
}
316+
if template == nil {
317+
return "", fmt.Errorf("addon %s template not found in status", addon.Name)
319318
}
320319

321-
if firstDeploymentNamespace == "" {
322-
if _, err = utils.ConvertToDeployment(object); err == nil {
323-
firstDeploymentNamespace = object.GetNamespace()
324-
break
320+
// pick the namespace of the first deployment, if there is no deployment, pick the namespace of the first daemonset
321+
desiredNs = "open-cluster-management-agent-addon"
322+
var firstDeploymentNamespace, firstDaemonSetNamespace string
323+
for _, manifest := range template.Spec.AgentSpec.Workload.Manifests {
324+
object := &unstructured.Unstructured{}
325+
if err := object.UnmarshalJSON(manifest.Raw); err != nil {
326+
a.logger.Error(err, "failed to extract the object")
327+
continue
325328
}
326-
}
327-
if firstDaemonSetNamespace == "" {
328-
if _, err = utils.ConvertToDaemonSet(object); err == nil {
329-
firstDaemonSetNamespace = object.GetNamespace()
329+
330+
if firstDeploymentNamespace == "" {
331+
if _, err = utils.ConvertToDeployment(object); err == nil {
332+
firstDeploymentNamespace = object.GetNamespace()
333+
break
334+
}
335+
}
336+
if firstDaemonSetNamespace == "" {
337+
if _, err = utils.ConvertToDaemonSet(object); err == nil {
338+
firstDaemonSetNamespace = object.GetNamespace()
339+
}
330340
}
331341
}
332-
}
333342

334-
if firstDeploymentNamespace != "" {
335-
desiredNS = firstDeploymentNamespace
336-
} else if firstDaemonSetNamespace != "" {
337-
desiredNS = firstDaemonSetNamespace
343+
if firstDeploymentNamespace != "" {
344+
desiredNs = firstDeploymentNamespace
345+
} else if firstDaemonSetNamespace != "" {
346+
desiredNs = firstDaemonSetNamespace
347+
}
338348
}
339-
340349
overrideNs, err := utils.AgentInstallNamespaceFromDeploymentConfigFunc(
341350
utils.NewAddOnDeploymentConfigGetter(a.addonClient))(addon)
342351
if err != nil {
343352
return "", err
344353
}
345354
if len(overrideNs) > 0 {
346-
desiredNS = overrideNs
355+
desiredNs = overrideNs
347356
}
348-
return desiredNS, nil
357+
return desiredNs, nil
349358
}

‎pkg/addon/templateagent/template_agent_test.go

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
154154
cases := []struct {
155155
name string
156156
addonTemplatePath string
157+
mcaInstallNamespace string
157158
addonDeploymentConfig *addonapiv1alpha1.AddOnDeploymentConfig
158159
managedCluster *clusterv1.ManagedCluster
159160
expectedErr string
@@ -474,7 +475,7 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
474475
if err != nil {
475476
t.Fatal(err)
476477
}
477-
if daemonSet.Namespace != "open-cluster-management-agent-addon-ds" { // first daemonset ns
478+
if daemonSet.Namespace != "open-cluster-management-agent-addon-ds" {
478479
t.Errorf("unexpected namespace %s", daemonSet.Namespace)
479480
}
480481
validatePodTemplate(t, daemonSet.Spec.Template,
@@ -505,6 +506,88 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
505506
validateAgentOptions(t, o, mca, "open-cluster-management-agent-addon-ds")
506507
},
507508
},
509+
{
510+
name: "manifests with only daemonset and with mca.spec.namespace configured rendered successfully",
511+
addonTemplatePath: "./testmanifests/addontemplate_daemonset.yaml",
512+
mcaInstallNamespace: "ocm-addon",
513+
addonDeploymentConfig: &addonapiv1alpha1.AddOnDeploymentConfig{
514+
ObjectMeta: metav1.ObjectMeta{
515+
Name: "hello-config",
516+
Namespace: "default",
517+
},
518+
Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{
519+
// AgentInstallNamespace: "test-install-namespace",
520+
CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{
521+
{
522+
Name: "LOG_LEVEL",
523+
Value: "4",
524+
},
525+
},
526+
NodePlacement: &addonapiv1alpha1.NodePlacement{
527+
NodeSelector: map[string]string{
528+
"host": "ssd",
529+
},
530+
Tolerations: []corev1.Toleration{
531+
{
532+
Key: "foo",
533+
Operator: corev1.TolerationOpExists,
534+
Effect: corev1.TaintEffectNoExecute,
535+
},
536+
},
537+
},
538+
Registries: []addonapiv1alpha1.ImageMirror{
539+
{
540+
Source: "quay.io/open-cluster-management",
541+
Mirror: "quay.io/ocm",
542+
},
543+
},
544+
},
545+
},
546+
managedCluster: addonfactory.NewFakeManagedCluster(clusterName, "1.10.1"),
547+
validateObjects: func(t *testing.T, objects []runtime.Object) {
548+
if len(objects) != 3 {
549+
t.Fatalf("expected 3 objects, but got %v", len(objects))
550+
}
551+
552+
unstructDaemonSet, ok := objects[0].(*unstructured.Unstructured)
553+
if !ok {
554+
t.Errorf("expected object to be *appsv1.DaemonSet, but got %T", objects[0])
555+
}
556+
daemonSet, err := utils.ConvertToDaemonSet(unstructDaemonSet)
557+
if err != nil {
558+
t.Fatal(err)
559+
}
560+
if daemonSet.Namespace != "ocm-addon" {
561+
t.Errorf("unexpected namespace %s", daemonSet.Namespace)
562+
}
563+
validatePodTemplate(t, daemonSet.Spec.Template,
564+
[]corev1.EnvVar{
565+
{Name: "LOG_LEVEL", Value: "4"},
566+
{Name: "HUB_KUBECONFIG", Value: "/managed/hub-kubeconfig/kubeconfig"},
567+
{Name: "CLUSTER_NAME", Value: clusterName},
568+
// {Name: "INSTALL_NAMESPACE", Value: "test-install-namespace"},
569+
},
570+
false)
571+
572+
// check clusterrole
573+
unstructCRB, ok := objects[2].(*unstructured.Unstructured)
574+
if !ok {
575+
t.Errorf("expected object to be unstructured, but got %T", objects[2])
576+
}
577+
clusterRoleBinding := &rbacv1.ClusterRoleBinding{}
578+
err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructCRB.Object, clusterRoleBinding)
579+
if err != nil {
580+
t.Fatal(err)
581+
}
582+
if clusterRoleBinding.Subjects[0].Namespace != "ocm-addon" {
583+
t.Errorf("clusterRolebinding namespace does not match, got %s", clusterRoleBinding.Subjects[0].Namespace)
584+
}
585+
},
586+
validateAgentOptions: func(t *testing.T, o agent.AgentAddonOptions,
587+
mca *addonapiv1alpha1.ManagedClusterAddOn) {
588+
validateAgentOptions(t, o, mca, "ocm-addon")
589+
},
590+
},
508591
{
509592
name: "manifests with only deployment rendered successfully",
510593
addonTemplatePath: "./testmanifests/addontemplate_deployment.yaml",
@@ -640,6 +723,9 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
640723
Name: addonName,
641724
Namespace: clusterName,
642725
},
726+
Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{
727+
InstallNamespace: tc.mcaInstallNamespace,
728+
},
643729
},
644730
)
645731

@@ -757,6 +843,33 @@ func TestAgentInstallNamespace(t *testing.T) {
757843
),
758844
expected: "test-install-namespace",
759845
},
846+
{
847+
name: "install namespace is set by mca.spec.installnamespace",
848+
addonTemplate: &addonapiv1alpha1.AddOnTemplate{
849+
ObjectMeta: metav1.ObjectMeta{
850+
Name: "hello-template",
851+
},
852+
Spec: addonapiv1alpha1.AddOnTemplateSpec{
853+
AgentSpec: workapiv1.ManifestWorkSpec{
854+
Workload: workapiv1.ManifestsTemplate{
855+
Manifests: []workapiv1.Manifest{},
856+
},
857+
},
858+
},
859+
},
860+
managedClusterAddonBuilder: newManagedClusterAddonBuilder(
861+
&addonapiv1alpha1.ManagedClusterAddOn{
862+
ObjectMeta: metav1.ObjectMeta{
863+
Name: addonName,
864+
Namespace: clusterName,
865+
},
866+
Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{
867+
InstallNamespace: "test-addon",
868+
},
869+
},
870+
),
871+
expected: "test-addon",
872+
},
760873
{
761874
name: "not support addon deployment config",
762875
addonTemplate: &addonapiv1alpha1.AddOnTemplate{

0 commit comments

Comments
 (0)