Skip to content

⚠️ use managedclustraddon install namespace when addondeploymentconfig not set #1027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

⚠️ use managedclustraddon install namespace when addondeploymentconfig not set #1027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/addon/templateagent/decorator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type namespaceDecorator struct {
paths map[string][]string
}

func newNamespaceDecorator(privateValues addonfactory.Values) *namespaceDecorator {
func newNamespaceDecorator(defaultNs string, privateValues addonfactory.Values) *namespaceDecorator {
decorator := &namespaceDecorator{
paths: map[string][]string{
"ClusterRoleBinding": {"subjects", "namespace"},
Expand All @@ -40,6 +40,8 @@ func newNamespaceDecorator(privateValues addonfactory.Values) *namespaceDecorato
namespace, ok := privateValues[InstallNamespacePrivateValueKey]
if ok {
decorator.installNamespace = namespace.(string)
} else if len(defaultNs) != 0 {
decorator.installNamespace = defaultNs
}

return decorator
Expand Down
5 changes: 3 additions & 2 deletions pkg/addon/templateagent/decorator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
)

func TestNamespaceDecorator(t *testing.T) {
defaultNs := "ocm-addon"
tests := []struct {
name string
namespace string
Expand All @@ -29,7 +30,7 @@ func TestNamespaceDecorator(t *testing.T) {
name: "no namespace set",
object: testingcommon.NewUnstructured("v1", "Pod", "default", "test"),
validateObject: func(t *testing.T, obj *unstructured.Unstructured) {
testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", "default")
testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", defaultNs)
},
},
{
Expand Down Expand Up @@ -153,7 +154,7 @@ func TestNamespaceDecorator(t *testing.T) {
values[InstallNamespacePrivateValueKey] = tc.namespace
}

d := newNamespaceDecorator(values)
d := newNamespaceDecorator(defaultNs, values)
result, err := d.decorate(tc.object)
if err != nil {
t.Fatal(err)
Expand Down
85 changes: 47 additions & 38 deletions pkg/addon/templateagent/template_agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,14 @@ func (a *CRDTemplateAgentAddon) renderObjects(
return objects, err
}

object, err = a.decorateObject(template, object, presetValues, privateValues)
object, err = a.decorateObject(addon, template, object, presetValues, privateValues)
if err != nil {
return objects, err
}
objects = append(objects, object)
}

additionalObjects, err := a.injectAdditionalObjects(template, presetValues, privateValues)
additionalObjects, err := a.injectAdditionalObjects(addon, template, presetValues, privateValues)
if err != nil {
return objects, err
}
Expand All @@ -206,14 +206,16 @@ func (a *CRDTemplateAgentAddon) renderObjects(
}

func (a *CRDTemplateAgentAddon) decorateObject(
addon *addonapiv1alpha1.ManagedClusterAddOn,
template *addonapiv1alpha1.AddOnTemplate,
obj *unstructured.Unstructured,
orderedValues orderedValues,
privateValues addonfactory.Values) (*unstructured.Unstructured, error) {
privateValues addonfactory.Values,
) (*unstructured.Unstructured, error) {
decorators := []decorator{
newDeploymentDecorator(a.logger, a.addonName, template, orderedValues, privateValues),
newDaemonSetDecorator(a.logger, a.addonName, template, orderedValues, privateValues),
newNamespaceDecorator(privateValues),
newNamespaceDecorator(addon.Spec.InstallNamespace, privateValues),
}

var err error
Expand All @@ -228,6 +230,7 @@ func (a *CRDTemplateAgentAddon) decorateObject(
}

func (a *CRDTemplateAgentAddon) injectAdditionalObjects(
addon *addonapiv1alpha1.ManagedClusterAddOn,
template *addonapiv1alpha1.AddOnTemplate,
orderedValues orderedValues,
privateValues addonfactory.Values) ([]runtime.Object, error) {
Expand All @@ -237,7 +240,7 @@ func (a *CRDTemplateAgentAddon) injectAdditionalObjects(

decorators := []decorator{
// decorate the namespace of the additional objects
newNamespaceDecorator(privateValues),
newNamespaceDecorator(addon.Spec.InstallNamespace, privateValues),
}

var objs []runtime.Object
Expand Down Expand Up @@ -296,54 +299,60 @@ func (a *CRDTemplateAgentAddon) getDesiredAddOnTemplateInner(
return template.DeepCopy(), nil
}

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

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

if firstDeploymentNamespace == "" {
if _, err = utils.ConvertToDeployment(object); err == nil {
firstDeploymentNamespace = object.GetNamespace()
break
// pick the namespace of the first deployment, if there is no deployment, pick the namespace of the first daemonset
desiredNs = "open-cluster-management-agent-addon"
var firstDeploymentNamespace, firstDaemonSetNamespace string
for _, manifest := range template.Spec.AgentSpec.Workload.Manifests {
object := &unstructured.Unstructured{}
if err := object.UnmarshalJSON(manifest.Raw); err != nil {
a.logger.Error(err, "failed to extract the object")
continue
}
}
if firstDaemonSetNamespace == "" {
if _, err = utils.ConvertToDaemonSet(object); err == nil {
firstDaemonSetNamespace = object.GetNamespace()

if firstDeploymentNamespace == "" {
if _, err = utils.ConvertToDeployment(object); err == nil {
firstDeploymentNamespace = object.GetNamespace()
break
}
}
if firstDaemonSetNamespace == "" {
if _, err = utils.ConvertToDaemonSet(object); err == nil {
firstDaemonSetNamespace = object.GetNamespace()
}
}
}
}

if firstDeploymentNamespace != "" {
desiredNS = firstDeploymentNamespace
} else if firstDaemonSetNamespace != "" {
desiredNS = firstDaemonSetNamespace
if firstDeploymentNamespace != "" {
desiredNs = firstDeploymentNamespace
} else if firstDaemonSetNamespace != "" {
desiredNs = firstDaemonSetNamespace
}
}

overrideNs, err := utils.AgentInstallNamespaceFromDeploymentConfigFunc(
utils.NewAddOnDeploymentConfigGetter(a.addonClient))(addon)
if err != nil {
return "", err
}
if len(overrideNs) > 0 {
desiredNS = overrideNs
desiredNs = overrideNs
}
return desiredNS, nil
return desiredNs, nil
}
115 changes: 114 additions & 1 deletion pkg/addon/templateagent/template_agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
cases := []struct {
name string
addonTemplatePath string
mcaInstallNamespace string
addonDeploymentConfig *addonapiv1alpha1.AddOnDeploymentConfig
managedCluster *clusterv1.ManagedCluster
expectedErr string
Expand Down Expand Up @@ -474,7 +475,7 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if daemonSet.Namespace != "open-cluster-management-agent-addon-ds" { // first daemonset ns
if daemonSet.Namespace != "open-cluster-management-agent-addon-ds" {
t.Errorf("unexpected namespace %s", daemonSet.Namespace)
}
validatePodTemplate(t, daemonSet.Spec.Template,
Expand Down Expand Up @@ -505,6 +506,88 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
validateAgentOptions(t, o, mca, "open-cluster-management-agent-addon-ds")
},
},
{
name: "manifests with only daemonset and with mca.spec.namespace configured rendered successfully",
addonTemplatePath: "./testmanifests/addontemplate_daemonset.yaml",
mcaInstallNamespace: "ocm-addon",
addonDeploymentConfig: &addonapiv1alpha1.AddOnDeploymentConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "hello-config",
Namespace: "default",
},
Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{
// AgentInstallNamespace: "test-install-namespace",
CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{
{
Name: "LOG_LEVEL",
Value: "4",
},
},
NodePlacement: &addonapiv1alpha1.NodePlacement{
NodeSelector: map[string]string{
"host": "ssd",
},
Tolerations: []corev1.Toleration{
{
Key: "foo",
Operator: corev1.TolerationOpExists,
Effect: corev1.TaintEffectNoExecute,
},
},
},
Registries: []addonapiv1alpha1.ImageMirror{
{
Source: "quay.io/open-cluster-management",
Mirror: "quay.io/ocm",
},
},
},
},
managedCluster: addonfactory.NewFakeManagedCluster(clusterName, "1.10.1"),
validateObjects: func(t *testing.T, objects []runtime.Object) {
if len(objects) != 3 {
t.Fatalf("expected 3 objects, but got %v", len(objects))
}

unstructDaemonSet, ok := objects[0].(*unstructured.Unstructured)
if !ok {
t.Errorf("expected object to be *appsv1.DaemonSet, but got %T", objects[0])
}
daemonSet, err := utils.ConvertToDaemonSet(unstructDaemonSet)
if err != nil {
t.Fatal(err)
}
if daemonSet.Namespace != "ocm-addon" {
t.Errorf("unexpected namespace %s", daemonSet.Namespace)
}
validatePodTemplate(t, daemonSet.Spec.Template,
[]corev1.EnvVar{
{Name: "LOG_LEVEL", Value: "4"},
{Name: "HUB_KUBECONFIG", Value: "/managed/hub-kubeconfig/kubeconfig"},
{Name: "CLUSTER_NAME", Value: clusterName},
// {Name: "INSTALL_NAMESPACE", Value: "test-install-namespace"},
},
false)

// check clusterrole
unstructCRB, ok := objects[2].(*unstructured.Unstructured)
if !ok {
t.Errorf("expected object to be unstructured, but got %T", objects[2])
}
clusterRoleBinding := &rbacv1.ClusterRoleBinding{}
err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructCRB.Object, clusterRoleBinding)
if err != nil {
t.Fatal(err)
}
if clusterRoleBinding.Subjects[0].Namespace != "ocm-addon" {
t.Errorf("clusterRolebinding namespace does not match, got %s", clusterRoleBinding.Subjects[0].Namespace)
}
},
validateAgentOptions: func(t *testing.T, o agent.AgentAddonOptions,
mca *addonapiv1alpha1.ManagedClusterAddOn) {
validateAgentOptions(t, o, mca, "ocm-addon")
},
},
{
name: "manifests with only deployment rendered successfully",
addonTemplatePath: "./testmanifests/addontemplate_deployment.yaml",
Expand Down Expand Up @@ -640,6 +723,9 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
Name: addonName,
Namespace: clusterName,
},
Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{
InstallNamespace: tc.mcaInstallNamespace,
},
},
)

Expand Down Expand Up @@ -757,6 +843,33 @@ func TestAgentInstallNamespace(t *testing.T) {
),
expected: "test-install-namespace",
},
{
name: "install namespace is set by mca.spec.installnamespace",
addonTemplate: &addonapiv1alpha1.AddOnTemplate{
ObjectMeta: metav1.ObjectMeta{
Name: "hello-template",
},
Spec: addonapiv1alpha1.AddOnTemplateSpec{
AgentSpec: workapiv1.ManifestWorkSpec{
Workload: workapiv1.ManifestsTemplate{
Manifests: []workapiv1.Manifest{},
},
},
},
},
managedClusterAddonBuilder: newManagedClusterAddonBuilder(
&addonapiv1alpha1.ManagedClusterAddOn{
ObjectMeta: metav1.ObjectMeta{
Name: addonName,
Namespace: clusterName,
},
Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{
InstallNamespace: "test-addon",
},
},
),
expected: "test-addon",
},
{
name: "not support addon deployment config",
addonTemplate: &addonapiv1alpha1.AddOnTemplate{
Expand Down
Loading