OCPBUGS-22095: Add default MCP objects for rendering (#833)

* feat: update render command to create default MCPs added default MCP creation to help correctly render out the resources for performance profile to use Signed-off-by: ehila <ehila@redhat.com> upkeep: fix spelling Signed-off-by: ehila <ehila@redhat.com> * feat: add default mcpools to tuned renderer Signed-off-by: ehila <ehila@redhat.com> --------- Signed-off-by: ehila <ehila@redhat.com>
openshift · Jan 25, 2024 · 2aa0b63 · 2aa0b63
1 parent fa70e3b
commit 2aa0b63
Show file tree

Hide file tree

Showing 32 changed files with 1,691 additions and 2 deletions.
diff --git a/pkg/performanceprofile/cmd/render/render.go b/pkg/performanceprofile/cmd/render/render.go
@@ -161,6 +161,9 @@ func render(inputDir, outputDir string) error {
 		}
 	}
 
+	// Append any missing default manifests (i.e. `master`/`worker`)
+	mcPools = util.AppendMissingDefaultMCPManifests(mcPools)
+
 	for _, pp := range perfProfiles {
 		mcp, err := selectMachineConfigPool(mcPools, pp.Spec.NodeSelector)
 		if err != nil {

diff --git a/pkg/tuned/cmd/render/render.go b/pkg/tuned/cmd/render/render.go
@@ -129,6 +129,9 @@ func render(inputDir []string, outputDir string, mcpName string) error {
 		}
 	}
 
+	// Append any missing default manifests (i.e. `master`/`worker`)
+	mcPools = util.AppendMissingDefaultMCPManifests(mcPools)
+
 	mcp := findMachineConfigPoolByName(mcPools, mcpName)
 	if mcp == nil {
 		klog.Errorf("Unable to find MachineConfigPool:%q in input folders", mcpName)

diff --git a/pkg/util/manifests.go b/pkg/util/manifests.go
@@ -24,6 +24,9 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller/performanceprofile/components"
+	mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	yamlutil "k8s.io/apimachinery/pkg/util/yaml"
 )
 
@@ -97,3 +100,68 @@ func ListFilesFromMultiplePaths(dirPaths []string) ([]string, error) {
 	}
 	return results, nil
 }
+
+// AppendMissingDefaultMCPManifests When default MCPs are missing, it is desirable to still generate the relevant
+// files based off of the standard MCP labels and node selectors.
+//
+// Here we create the default `master` and `worker` MCP if they are missing with their respective base Labels and NodeSelector Labels,
+// this allows any resource such as PAO to utilize the default during bootstrap rendering.
+func AppendMissingDefaultMCPManifests(currentMCPs []*mcfgv1.MachineConfigPool) []*mcfgv1.MachineConfigPool {
+	const (
+		master             = "master"
+		worker             = "worker"
+		labelPrefix        = "pools.operator.machineconfiguration.openshift.io/"
+		masterLabels       = labelPrefix + master
+		workerLabels       = labelPrefix + worker
+		masterNodeSelector = components.NodeRoleLabelPrefix + master
+		workerNodeSelector = components.NodeRoleLabelPrefix + worker
+	)
+	var (
+		finalMCPList = []*mcfgv1.MachineConfigPool{}
+		defaultMCPs  = []*mcfgv1.MachineConfigPool{
+			{
+				ObjectMeta: v1.ObjectMeta{
+					Labels: map[string]string{
+						masterLabels: "",
+					},
+					Name: master,
+				},
+				Spec: mcfgv1.MachineConfigPoolSpec{
+					NodeSelector: v1.AddLabelToSelector(&v1.LabelSelector{}, masterNodeSelector, ""),
+				},
+			},
+			{
+				ObjectMeta: v1.ObjectMeta{
+					Labels: map[string]string{
+						workerLabels: "",
+					},
+					Name: worker,
+				},
+				Spec: mcfgv1.MachineConfigPoolSpec{
+					NodeSelector: v1.AddLabelToSelector(&v1.LabelSelector{}, workerNodeSelector, ""),
+				},
+			},
+		}
+	)
+
+	if len(currentMCPs) == 0 {
+		return defaultMCPs
+	}
+
+	for _, defaultMCP := range defaultMCPs {
+		missing := true
+		for _, mcp := range currentMCPs {
+			// Since users can supply MCP files and these will be raw files with out going through the API
+			// server validation, we normalize the file name so as to not mask any configuration errors.
+			if strings.ToLower(mcp.Name) == defaultMCP.Name {
+				missing = false
+				break
+			}
+		}
+		if missing {
+			finalMCPList = append(finalMCPList, defaultMCP)
+		}
+	}
+
+	return append(finalMCPList, currentMCPs...)
+}
diff --git a/...e2e/performanceprofile/cluster-setup/bootstrap-cluster/extra-mcp/machine_config_pool.yaml b/...e2e/performanceprofile/cluster-setup/bootstrap-cluster/extra-mcp/machine_config_pool.yaml
@@ -0,0 +1,17 @@
+apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfigPool
+metadata:
+  name: worker-cnf
+  namespace: openshift-machine-config-operator
+  labels:
+    machineconfiguration.openshift.io/role: worker-cnf
+spec:
+  paused: true
+  machineConfigSelector:
+    matchExpressions:
+      - key: machineconfiguration.openshift.io/role
+        operator: In
+        values: [worker,worker-cnf]
+  nodeSelector:
+    matchLabels:
+      node-role.kubernetes.io/worker-cnf: ""
diff --git a/test/e2e/performanceprofile/cluster-setup/bootstrap-cluster/performance/pao_master.yaml b/test/e2e/performanceprofile/cluster-setup/bootstrap-cluster/performance/pao_master.yaml
@@ -0,0 +1,12 @@
+apiVersion: performance.openshift.io/v2
+kind: PerformanceProfile
+metadata:
+  name: openshift-bootstrap-master
+spec:
+  cpu:
+    isolated: 0-1
+    reserved: 2-7
+  machineConfigPoolSelector:
+    pools.operator.machineconfiguration.openshift.io/master: ""
+  nodeSelector:
+    node-role.kubernetes.io/master: ""
diff --git a/test/e2e/performanceprofile/cluster-setup/bootstrap-cluster/performance/pao_worker.yaml b/test/e2e/performanceprofile/cluster-setup/bootstrap-cluster/performance/pao_worker.yaml
@@ -0,0 +1,12 @@
+apiVersion: performance.openshift.io/v2
+kind: PerformanceProfile
+metadata:
+  name: openshift-bootstrap-worker
+spec:
+  cpu:
+    isolated: 0-1
+    reserved: 2-3
+  machineConfigPoolSelector:
+    pools.operator.machineconfiguration.openshift.io/worker: ""
+  nodeSelector:
+    node-role.kubernetes.io/worker: ""
diff --git a/test/e2e/performanceprofile/functests-render-command/1_render_command/render_test.go b/test/e2e/performanceprofile/functests-render-command/1_render_command/render_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -12,8 +13,9 @@ import (
 )
 
 const (
-	defaultExpectedDir = "default"
-	pinnedExpectedDir  = "pinned"
+	defaultExpectedDir   = "default"
+	pinnedExpectedDir    = "pinned"
+	bootstrapExpectedDir = "bootstrap"
 )
 
 var (
@@ -23,13 +25,17 @@ var (
 	testDataPath       string
 	defaultPinnedDir   string
 	snoLegacyPinnedDir string
+	bootstrapPPDir     string
+	extraMCPDir        string
 )
 
 var _ = Describe("render command e2e test", func() {
 
 	BeforeEach(func() {
 		assetsOutDir = createTempAssetsDir()
 		assetsInDir := filepath.Join(workspaceDir, "test", "e2e", "performanceprofile", "cluster-setup", "base", "performance")
+		bootstrapPPDir = filepath.Join(workspaceDir, "test", "e2e", "performanceprofile", "cluster-setup", "bootstrap-cluster", "performance")
+		extraMCPDir = filepath.Join(workspaceDir, "test", "e2e", "performanceprofile", "cluster-setup", "bootstrap-cluster", "extra-mcp")
 		ppDir = filepath.Join(workspaceDir, "test", "e2e", "performanceprofile", "cluster-setup", "manual-cluster", "performance")
 		defaultPinnedDir = filepath.Join(workspaceDir, "test", "e2e", "performanceprofile", "cluster-setup", "pinned-cluster", "default")
 		snoLegacyPinnedDir = filepath.Join(workspaceDir, "test", "e2e", "performanceprofile", "cluster-setup", "pinned-cluster", "single-node-legacy")
@@ -101,6 +107,44 @@ var _ = Describe("render command e2e test", func() {
 		})
 	})
 
+	Context("With no MCPs manifest resources during bootstrap", func() {
+		It("should render PerformanceProfile with default", func() {
+
+			bootstrapPPDirs := []string{bootstrapPPDir, defaultPinnedDir}
+
+			cmdline := []string{
+				filepath.Join(binPath, "cluster-node-tuning-operator"),
+				"render",
+				"--asset-input-dir", strings.Join(bootstrapPPDirs, ","),
+				"--asset-output-dir", assetsOutDir,
+			}
+			fmt.Fprintf(GinkgoWriter, "running: %v\n", cmdline)
+
+			cmd := exec.Command(cmdline[0], cmdline[1:]...)
+			runAndCompare(cmd, path.Join(bootstrapExpectedDir, "no-mcp"))
+
+		})
+	})
+
+	Context("With extra MCP manifest resources during bootstrap", func() {
+		It("should render PerformanceProfile with default", func() {
+
+			bootstrapPPDirs := []string{bootstrapPPDir, defaultPinnedDir, extraMCPDir}
+
+			cmdline := []string{
+				filepath.Join(binPath, "cluster-node-tuning-operator"),
+				"render",
+				"--asset-input-dir", strings.Join(bootstrapPPDirs, ","),
+				"--asset-output-dir", assetsOutDir,
+			}
+			fmt.Fprintf(GinkgoWriter, "running: %v\n", cmdline)
+
+			cmd := exec.Command(cmdline[0], cmdline[1:]...)
+			runAndCompare(cmd, path.Join(bootstrapExpectedDir, "extra-mcp"))
+
+		})
+	})
+
 	AfterEach(func() {
 		cleanArtifacts()
 	})

diff --git a/...put/bootstrap/extra-mcp/01_01-master-cpu-partitioning_workload_pinning_machineconfig.yaml b/...put/bootstrap/extra-mcp/01_01-master-cpu-partitioning_workload_pinning_machineconfig.yaml
@@ -0,0 +1,42 @@
+apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfig
+metadata:
+  creationTimestamp: null
+  labels:
+    machineconfiguration.openshift.io/role: master
+  name: 01-master-cpu-partitioning
+spec:
+  baseOSExtensionsContainerImage: ""
+  config:
+    ignition:
+      config:
+        replace:
+          verification: {}
+      proxy: {}
+      security:
+        tls: {}
+      timeouts: {}
+      version: 3.2.0
+    passwd: {}
+    storage:
+      files:
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,CnsKICAibWFuYWdlbWVudCI6IHsKICAgICJjcHVzZXQiOiAiIgogIH0KfQo=
+          verification: {}
+        group: {}
+        mode: 420
+        path: /etc/kubernetes/openshift-workload-pinning
+        user: {}
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,CltjcmlvLnJ1bnRpbWUud29ya2xvYWRzLm1hbmFnZW1lbnRdCmFjdGl2YXRpb25fYW5ub3RhdGlvbiA9ICJ0YXJnZXQud29ya2xvYWQub3BlbnNoaWZ0LmlvL21hbmFnZW1lbnQiCmFubm90YXRpb25fcHJlZml4ID0gInJlc291cmNlcy53b3JrbG9hZC5vcGVuc2hpZnQuaW8iCnJlc291cmNlcyA9IHsgImNwdXNoYXJlcyIgPSAwLCAiY3B1c2V0IiA9ICIiIH0K
+          verification: {}
+        group: {}
+        mode: 420
+        path: /etc/crio/crio.conf.d/01-workload-pinning-default.conf
+        user: {}
+    systemd: {}
+  extensions: null
+  fips: false
+  kernelArguments: null
+  kernelType: ""
+  osImageURL: ""
diff --git a/...bootstrap/extra-mcp/01_01-worker-cnf-cpu-partitioning_workload_pinning_machineconfig.yaml b/...bootstrap/extra-mcp/01_01-worker-cnf-cpu-partitioning_workload_pinning_machineconfig.yaml
@@ -0,0 +1,42 @@
+apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfig
+metadata:
+  creationTimestamp: null
+  labels:
+    machineconfiguration.openshift.io/role: worker-cnf
+  name: 01-worker-cnf-cpu-partitioning
+spec:
+  baseOSExtensionsContainerImage: ""
+  config:
+    ignition:
+      config:
+        replace:
+          verification: {}
+      proxy: {}
+      security:
+        tls: {}
+      timeouts: {}
+      version: 3.2.0
+    passwd: {}
+    storage:
+      files:
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,CnsKICAibWFuYWdlbWVudCI6IHsKICAgICJjcHVzZXQiOiAiIgogIH0KfQo=
+          verification: {}
+        group: {}
+        mode: 420
+        path: /etc/kubernetes/openshift-workload-pinning
+        user: {}
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,CltjcmlvLnJ1bnRpbWUud29ya2xvYWRzLm1hbmFnZW1lbnRdCmFjdGl2YXRpb25fYW5ub3RhdGlvbiA9ICJ0YXJnZXQud29ya2xvYWQub3BlbnNoaWZ0LmlvL21hbmFnZW1lbnQiCmFubm90YXRpb25fcHJlZml4ID0gInJlc291cmNlcy53b3JrbG9hZC5vcGVuc2hpZnQuaW8iCnJlc291cmNlcyA9IHsgImNwdXNoYXJlcyIgPSAwLCAiY3B1c2V0IiA9ICIiIH0K
+          verification: {}
+        group: {}
+        mode: 420
+        path: /etc/crio/crio.conf.d/01-workload-pinning-default.conf
+        user: {}
+    systemd: {}
+  extensions: null
+  fips: false
+  kernelArguments: null
+  kernelType: ""
+  osImageURL: ""
diff --git a/...put/bootstrap/extra-mcp/01_01-worker-cpu-partitioning_workload_pinning_machineconfig.yaml b/...put/bootstrap/extra-mcp/01_01-worker-cpu-partitioning_workload_pinning_machineconfig.yaml
@@ -0,0 +1,42 @@
+apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfig
+metadata:
+  creationTimestamp: null
+  labels:
+    machineconfiguration.openshift.io/role: worker
+  name: 01-worker-cpu-partitioning
+spec:
+  baseOSExtensionsContainerImage: ""
+  config:
+    ignition:
+      config:
+        replace:
+          verification: {}
+      proxy: {}
+      security:
+        tls: {}
+      timeouts: {}
+      version: 3.2.0
+    passwd: {}
+    storage:
+      files:
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,CnsKICAibWFuYWdlbWVudCI6IHsKICAgICJjcHVzZXQiOiAiIgogIH0KfQo=
+          verification: {}
+        group: {}
+        mode: 420
+        path: /etc/kubernetes/openshift-workload-pinning
+        user: {}
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,CltjcmlvLnJ1bnRpbWUud29ya2xvYWRzLm1hbmFnZW1lbnRdCmFjdGl2YXRpb25fYW5ub3RhdGlvbiA9ICJ0YXJnZXQud29ya2xvYWQub3BlbnNoaWZ0LmlvL21hbmFnZW1lbnQiCmFubm90YXRpb25fcHJlZml4ID0gInJlc291cmNlcy53b3JrbG9hZC5vcGVuc2hpZnQuaW8iCnJlc291cmNlcyA9IHsgImNwdXNoYXJlcyIgPSAwLCAiY3B1c2V0IiA9ICIiIH0K
+          verification: {}
+        group: {}
+        mode: 420
+        path: /etc/crio/crio.conf.d/01-workload-pinning-default.conf
+        user: {}
+    systemd: {}
+  extensions: null
+  fips: false
+  kernelArguments: null
+  kernelType: ""
+  osImageURL: ""