Merge pull request #2005 from pixelsoccupied/manageclusterCRupdated

OCPBUGS-38489: ztp: remove unsupported KlusterletAddonConfig clusterLabels and update managecluster CR to include cloud and vendor label
openshift-kni · Aug 27, 2024 · 382aa4d · 382aa4d
2 parents 7ad7d8c + db10710
commit 382aa4d
Show file tree

Hide file tree

Showing 26 changed files with 203 additions and 82 deletions.
diff --git a/ztp/gitops-subscriptions/argocd/README.md b/ztp/gitops-subscriptions/argocd/README.md
@@ -108,7 +108,6 @@ In order to deploy the OpenShift GitOps operator v1.12 you may apply the provide
 ```
     $ oc patch multiclusterengines.multicluster.openshift.io multiclusterengine --type=merge --patch-file out/argocd/deployment/disable-cluster-proxy-addon.json
 ```
-
 5. Prepare the ArgoCD pipeline configuration
 - Create a git repository with a directory structure similar to the example directory.
 - Configure access to the repository using the ArgoCD UI. Under *Settings* configure:
@@ -123,6 +122,11 @@ In order to deploy the OpenShift GitOps operator v1.12 you may apply the provide
     oc apply -k out/argocd/deployment
 ```
 
+7. Optionally: If configuring an existing hub cluster (i.e skipped step 6, pipeline configuration), starting with ACM 2.10, the following patch is needed to allow ACM to update `vendor` and `cloud` labels in ManagedCluster CRs. This enables observability without user intervention.
+```
+    $ oc patch applications.argoproj.io clusters -n openshift-gitops --type='json' --patch-file out/argocd/deployment/allow-acm-managedcluster-control.json
+```
+
 ### Deploying a site
 The following steps prepare the hub cluster for site deployment and initiate ZTP by pushing CRs to your GIT repository.
 1. Create the secrets needed for the site installation. These resources must be in a namespace with a name matching the cluster name. In *out/argocd/example/siteconfig/example-sno.yaml* the cluster *name* & *namespace* is `example-sno`.

diff --git a/ztp/gitops-subscriptions/argocd/deployment/allow-acm-managedcluster-control.json b/ztp/gitops-subscriptions/argocd/deployment/allow-acm-managedcluster-control.json
@@ -0,0 +1,20 @@
+[
+  {
+    "op": "add",
+    "path": "/spec/ignoreDifferences",
+    "value": [
+      {
+        "group": "cluster.open-cluster-management.io",
+        "kind": "ManagedCluster",
+        "managedFieldsManagers": [
+          "controller"
+        ]
+      }
+    ]
+  },
+  {
+    "op": "add",
+    "path": "/spec/syncPolicy/syncOptions/-",
+    "value": "RespectIgnoreDifferences=true"
+  }
+]
diff --git a/ztp/gitops-subscriptions/argocd/deployment/clusters-app.yaml b/ztp/gitops-subscriptions/argocd/deployment/clusters-app.yaml
@@ -21,10 +21,20 @@ spec:
     # the sitconfig.yaml exist AND use the ../../hack/patch-argocd-dev.sh script to re-patch the deployment-repo-server
 #    plugin:
 #      name: kustomize-with-local-plugins
+  ignoreDifferences: # recommended way to allow ACM controller to manage its fields. alternative approach documented below (1)
+    - group: cluster.open-cluster-management.io
+      kind: ManagedCluster
+      managedFieldsManagers:
+        - controller
+# (1) alternatively you can choose to ignore a specific path like so (replace managedFieldsManagers with jsonPointers)
+#      jsonPointers:
+#        - /metadata/labels/cloud
+#        - /metadata/labels/vendor
   syncPolicy:
     automated:
       prune: true
       selfHeal: true
     syncOptions:
-    - CreateNamespace=true
-    - PrunePropagationPolicy=background
+      - CreateNamespace=true
+      - PrunePropagationPolicy=background
+      - RespectIgnoreDifferences=true
diff --git a/ztp/gitops-subscriptions/argocd/example/siteconfig/KlusterletAddonConfigOverride.yaml b/ztp/gitops-subscriptions/argocd/example/siteconfig/KlusterletAddonConfigOverride.yaml
@@ -8,9 +8,6 @@ metadata:
 spec:
   clusterName: "{{ .Cluster.ClusterName }}"
   clusterNamespace: "{{ .Cluster.ClusterName }}"
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: false
   certPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/clusterCRsV1.go b/ztp/siteconfig-generator/siteConfig/clusterCRsV1.go
@@ -169,9 +169,6 @@ metadata:
 spec:
   clusterName: "{{ .Cluster.ClusterName }}"
   clusterNamespace: "{{ .Cluster.ClusterName }}"
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: false
   certPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/clusterCRsV2.go b/ztp/siteconfig-generator/siteConfig/clusterCRsV2.go
@@ -177,9 +177,6 @@ metadata:
 spec:
   clusterName: "{{ .Cluster.ClusterName }}"
   clusterNamespace: "{{ .Cluster.ClusterName }}"
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: false
   certPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/siteConfig.go b/ztp/siteconfig-generator/siteConfig/siteConfig.go
@@ -10,25 +10,28 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
-const localExtraManifestPath = "extra-manifest"
-const workloadPath = "workload"
-const workloadFile = "03-workload-partitioning.yaml"
-const workloadCrioFile = "crio.conf"
-const workloadKubeletFile = "kubelet.conf"
-const cpuset = "$cpuset"
-const SNO = "sno"
-const Standard = "standard"
-const Master = "master"
-const ZtpAnnotation = "ran.openshift.io/ztp-gitops-generated"
-const ZtpAnnotationDefaultValue = "{}"
-const UnsetStringValue = "__unset_value__"
-const FileExt = ".yaml"
-const inspectAnnotationPrefix = "inspect.metal3.io"
-const ZtpWarningAnnotation = "ran.openshift.io/ztp-warning"
-const ZtpDeprecationWarningAnnotationPostfix = "field-deprecation"
-const nodeLabelPrefix = "bmac.agent-install.openshift.io.node-label"
-const siteConfigAPIGroup = "ran.openshift.io"
-const aZTP = "accelerated-ztp"
+const (
+	localExtraManifestPath                 = "extra-manifest"
+	workloadPath                           = "workload"
+	workloadFile                           = "03-workload-partitioning.yaml"
+	workloadCrioFile                       = "crio.conf"
+	workloadKubeletFile                    = "kubelet.conf"
+	cpuset                                 = "$cpuset"
+	SNO                                    = "sno"
+	Standard                               = "standard"
+	Master                                 = "master"
+	ZtpAnnotation                          = "ran.openshift.io/ztp-gitops-generated"
+	ZtpAnnotationDefaultValue              = "{}"
+	UnsetStringValue                       = "__unset_value__"
+	FileExt                                = ".yaml"
+	inspectAnnotationPrefix                = "inspect.metal3.io"
+	ZtpWarningAnnotation                   = "ran.openshift.io/ztp-warning"
+	ZtpDeprecationWarningAnnotationPostfix = "field-deprecation"
+	nodeLabelPrefix                        = "bmac.agent-install.openshift.io.node-label"
+	siteConfigAPIGroup                     = "ran.openshift.io"
+	aZTP                                   = "accelerated-ztp"
+	acmAutoDetect                          = "auto-detect" // acm uses this as a value for "vendor" and "cloud" in ManagedCluster CR to allow for easy configuration of observability stack
+)
 
 var Separator = []byte("---\n")
 

diff --git a/ztp/siteconfig-generator/siteConfig/siteConfigBuilder.go b/ztp/siteconfig-generator/siteConfig/siteConfigBuilder.go
@@ -268,6 +268,24 @@ func (scbuilder *SiteConfigBuilder) getClusterCRs(clusterId int, siteConfigTemp
 					instantiatedCR = updateAgentClusterInstall(cluster, instantiatedCR)
 				}
 			}
+
+			if kind == "ManagedCluster" {
+				if metadata, ok := instantiatedCR["metadata"].(map[string]interface{}); ok {
+					// ManagedCluster CR needs to have vendor and cloud label for observability stack to work without additional setup
+					labels, ok := metadata["labels"].(map[string]string)
+					if !ok {
+						labels = make(map[string]string)
+						metadata["labels"] = labels
+					}
+					if _, ok := labels["vendor"]; !ok {
+						labels["vendor"] = acmAutoDetect
+					}
+					if _, ok := labels["cloud"]; !ok {
+						labels["cloud"] = acmAutoDetect
+					}
+				}
+			}
+
 			clusterCRs = append(clusterCRs, instantiatedCR)
 		}
 	}

diff --git a/ztp/siteconfig-generator/siteConfig/siteConfigBuilder_test.go b/ztp/siteconfig-generator/siteConfig/siteConfigBuilder_test.go
@@ -1862,6 +1862,101 @@ spec:
 	assert.NotEqual(t, nmState, nil)
 }
 
+func Test_managedclusterLabel(t *testing.T) {
+	// test for when user did not provide vendor and cloud labels
+	ManagedCluster := `
+apiVersion: ran.openshift.io/v1
+kind: SiteConfig
+metadata:
+  name: "test-site"
+spec:
+  clusterImageSetNameRef: "openshift-v4.8.0"
+  clusters:
+  - clusterName: "cluster1"
+    clusterLabels:
+      group-du-sno: ""
+      common: true
+      sites : "test-site"
+    nodes:
+      - hostName: "node1"
+`
+	sc := SiteConfig{}
+	err := yaml.Unmarshal([]byte(ManagedCluster), &sc)
+	assert.Equal(t, err, nil)
+
+	scBuilder, _ := NewSiteConfigBuilder()
+	scBuilder.SetLocalExtraManifestPath("testdata/extra-manifest")
+	result, err := scBuilder.Build(sc)
+	mcls, err := getKind(result["test-site/cluster1"], "ManagedCluster")
+
+	metadata := mcls["metadata"].(map[string]interface{})
+	labels, _ := metadata["labels"].(map[string]string)
+	assert.Equal(t, labels["vendor"], "auto-detect")
+	assert.Equal(t, labels["cloud"], "auto-detect")
+	assert.Equal(t, labels["sites"], "test-site")
+
+	// test for when user provided vendor but not cloud
+	ManagedCluster = `
+apiVersion: ran.openshift.io/v1
+kind: SiteConfig
+metadata:
+  name: "test-site"
+spec:
+  clusterImageSetNameRef: "openshift-v4.8.0"
+  clusters:
+  - clusterName: "cluster1"
+    clusterLabels:
+      group-du-sno: ""
+      common: true
+      sites : "test-site"
+      vendor: "my-vendor"
+    nodes:
+      - hostName: "node1"
+`
+	sc = SiteConfig{}
+	err = yaml.Unmarshal([]byte(ManagedCluster), &sc)
+	assert.Equal(t, err, nil)
+
+	scBuilder, _ = NewSiteConfigBuilder()
+	scBuilder.SetLocalExtraManifestPath("testdata/extra-manifest")
+	result, err = scBuilder.Build(sc)
+	mcls, err = getKind(result["test-site/cluster1"], "ManagedCluster")
+
+	metadata = mcls["metadata"].(map[string]interface{})
+	labels, _ = metadata["labels"].(map[string]string)
+	assert.Equal(t, labels["vendor"], "my-vendor")
+	assert.Equal(t, labels["cloud"], "auto-detect")
+	assert.Equal(t, labels["sites"], "test-site")
+
+	// test for when no labels are provided from SiteConfig
+	ManagedCluster = `
+apiVersion: ran.openshift.io/v1
+kind: SiteConfig
+metadata:
+  name: "test-site"
+spec:
+  clusterImageSetNameRef: "openshift-v4.8.0"
+  clusters:
+  - clusterName: "cluster1"
+    nodes:
+      - hostName: "node1"
+`
+	sc = SiteConfig{}
+	err = yaml.Unmarshal([]byte(ManagedCluster), &sc)
+	assert.Equal(t, err, nil)
+
+	scBuilder, _ = NewSiteConfigBuilder()
+	scBuilder.SetLocalExtraManifestPath("testdata/extra-manifest")
+	result, err = scBuilder.Build(sc)
+	mcls, err = getKind(result["test-site/cluster1"], "ManagedCluster")
+
+	metadata = mcls["metadata"].(map[string]interface{})
+	labels, _ = metadata["labels"].(map[string]string)
+	assert.Equal(t, labels["vendor"], "auto-detect")
+	assert.Equal(t, labels["cloud"], "auto-detect")
+	assert.Equal(t, len(labels), 2)
+}
+
 func Test_filterExtraManifests(t *testing.T) {
 
 	getMapWithFileNames := func(root string) map[string]interface{} {

diff --git a/...-generator/siteConfig/testdata/KlusterletAddonConfigOverride-MissingArgocdAnnotation.yaml b/...-generator/siteConfig/testdata/KlusterletAddonConfigOverride-MissingArgocdAnnotation.yaml
@@ -8,9 +8,6 @@ metadata:
 spec:
   clusterName: "{{ .Cluster.ClusterName }}"
   clusterNamespace: "{{ .Cluster.ClusterName }}"
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: true 
   certPolicyController:

diff --git a/...teconfig-generator/siteConfig/testdata/KlusterletAddonConfigOverride-MissingMetadata.yaml b/...teconfig-generator/siteConfig/testdata/KlusterletAddonConfigOverride-MissingMetadata.yaml
@@ -3,9 +3,6 @@ kind: KlusterletAddonConfig
 spec:
   clusterName: "{{ .Cluster.ClusterName }}"
   clusterNamespace: "{{ .Cluster.ClusterName }}"
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: true 
   certPolicyController:

diff --git a/...nerator/siteConfig/testdata/KlusterletAddonConfigOverride-MissingMetadataAnnotations.yaml b/...nerator/siteConfig/testdata/KlusterletAddonConfigOverride-MissingMetadataAnnotations.yaml
@@ -6,9 +6,6 @@ metadata:
 spec:
   clusterName: "{{ .Cluster.ClusterName }}"
   clusterNamespace: "{{ .Cluster.ClusterName }}"
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: true 
   certPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/testdata/KlusterletAddonConfigOverride-NotTemplated.yaml b/ztp/siteconfig-generator/siteConfig/testdata/KlusterletAddonConfigOverride-NotTemplated.yaml
@@ -8,9 +8,6 @@ metadata:
 spec:
   clusterName: cluster1
   clusterNamespace: cluster1
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: true 
   certPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/testdata/KlusterletAddonConfigOverride.yaml b/ztp/siteconfig-generator/siteConfig/testdata/KlusterletAddonConfigOverride.yaml
@@ -8,9 +8,6 @@ metadata:
 spec:
   clusterName: "{{ .Cluster.ClusterName }}"
   clusterNamespace: "{{ .Cluster.ClusterName }}"
-  clusterLabels:
-    cloud: auto-detect
-    vendor: auto-detect
   applicationManager:
     enabled: true 
   certPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/testdata/SplitAndMergeMachineConfigExpected/mergeMC.yaml b/ztp/siteconfig-generator/siteConfig/testdata/SplitAndMergeMachineConfigExpected/mergeMC.yaml
@@ -425,6 +425,9 @@ metadata:
     annotations:
         argocd.argoproj.io/sync-wave: "2"
         ran.openshift.io/ztp-gitops-generated: '{}'
+    labels:
+        cloud: auto-detect
+        vendor: auto-detect
     name: cluster1
 spec:
     hubAcceptsClient: true
@@ -442,9 +445,6 @@ spec:
         enabled: false
     certPolicyController:
         enabled: false
-    clusterLabels:
-        cloud: auto-detect
-        vendor: auto-detect
     clusterName: cluster1
     clusterNamespace: cluster1
     iamPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/testdata/SplitAndMergeMachineConfigExpected/splitMC.yaml b/ztp/siteconfig-generator/siteConfig/testdata/SplitAndMergeMachineConfigExpected/splitMC.yaml
@@ -513,6 +513,9 @@ metadata:
     annotations:
         argocd.argoproj.io/sync-wave: "2"
         ran.openshift.io/ztp-gitops-generated: '{}'
+    labels:
+        cloud: auto-detect
+        vendor: auto-detect
     name: cluster1
 spec:
     hubAcceptsClient: true
@@ -530,9 +533,6 @@ spec:
         enabled: false
     certPolicyController:
         enabled: false
-    clusterLabels:
-        cloud: auto-detect
-        vendor: auto-detect
     clusterName: cluster1
     clusterNamespace: cluster1
     iamPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/testdata/filteredoutput/onlyUserCR.yaml b/ztp/siteconfig-generator/siteConfig/testdata/filteredoutput/onlyUserCR.yaml
@@ -125,6 +125,9 @@ metadata:
     annotations:
         argocd.argoproj.io/sync-wave: "2"
         ran.openshift.io/ztp-gitops-generated: '{}'
+    labels:
+        cloud: auto-detect
+        vendor: auto-detect
     name: cluster1
 spec:
     hubAcceptsClient: true
@@ -142,9 +145,6 @@ spec:
         enabled: false
     certPolicyController:
         enabled: false
-    clusterLabels:
-        cloud: auto-detect
-        vendor: auto-detect
     clusterName: cluster1
     clusterNamespace: cluster1
     iamPolicyController:

diff --git a/ztp/siteconfig-generator/siteConfig/testdata/filteredoutput/partialfilter.yaml b/ztp/siteconfig-generator/siteConfig/testdata/filteredoutput/partialfilter.yaml
@@ -164,6 +164,9 @@ metadata:
     annotations:
         argocd.argoproj.io/sync-wave: "2"
         ran.openshift.io/ztp-gitops-generated: '{}'
+    labels:
+        cloud: auto-detect
+        vendor: auto-detect
     name: cluster1
 spec:
     hubAcceptsClient: true
@@ -181,9 +184,6 @@ spec:
         enabled: false
     certPolicyController:
         enabled: false
-    clusterLabels:
-        cloud: auto-detect
-        vendor: auto-detect
     clusterName: cluster1
     clusterNamespace: cluster1
     iamPolicyController: