feat: Support HelmAddon strategy to deploy NFD (#390)

nutanix-cloud-native · Feb 24, 2024 · 34ab8ec · 34ab8ec
1 parent 41a6cb9
commit 34ab8ec
Show file tree

Hide file tree

Showing 35 changed files with 498 additions and 118 deletions.
diff --git a/api/v1alpha1/addon_types.go b/api/v1alpha1/addon_types.go
@@ -80,12 +80,26 @@ func (CNI) VariableSchema() clusterv1.VariableSchema {
 }
 
 // NFD tells us to enable or disable the node feature discovery addon.
-type NFD struct{}
+type NFD struct {
+	// +optional
+	Strategy AddonStrategy `json:"strategy,omitempty"`
+}
 
 func (NFD) VariableSchema() clusterv1.VariableSchema {
 	return clusterv1.VariableSchema{
 		OpenAPIV3Schema: clusterv1.JSONSchemaProps{
 			Type: "object",
+			Properties: map[string]clusterv1.JSONSchemaProps{
+				"strategy": {
+					Description: "Addon strategy used to deploy Node Feature Discovery (NFD) to the workload cluster",
+					Type:        "string",
+					Enum: variables.MustMarshalValuesToEnumJSON(
+						AddonStrategyClusterResourceSet,
+						AddonStrategyHelmAddon,
+					),
+				},
+			},
+			Required: []string{"strategy"},
 		},
 	}
 }

diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go
@@ -4,8 +4,10 @@
 package v1alpha1
 
 const (
-	// CNIVariableName is the external patch variable name.
+	// CNIVariableName is the CNI external patch variable name.
 	CNIVariableName = "cni"
+	// NFDVariableName is the NFD external patch variable name.
+	NFDVariableName = "nfd"
 	// AWSVariableName is the AWS config patch variable name.
 	AWSVariableName = "aws"
 )
diff --git a/charts/capi-runtime-extensions/README.md b/charts/capi-runtime-extensions/README.md
@@ -46,6 +46,9 @@ A Helm chart for capi-runtime-extensions
 | hooks.cni.cilium.crsStrategy.defaultCiliumConfigMap.name | string | `"cilium"` |  |
 | hooks.cni.cilium.helmAddonStrategy.defaultValueTemplateConfigMap.create | bool | `true` |  |
 | hooks.cni.cilium.helmAddonStrategy.defaultValueTemplateConfigMap.name | string | `"default-cilium-cni-helm-values-template"` |  |
+| hooks.nfd.crsStrategy.defaultInstallationConfigMap.name | string | `"node-feature-discovery"` |  |
+| hooks.nfd.helmAddonStrategy.defaultValueTemplateConfigMap.create | bool | `true` |  |
+| hooks.nfd.helmAddonStrategy.defaultValueTemplateConfigMap.name | string | `"default-nfd-helm-values-template"` |  |
 | image.pullPolicy | string | `"IfNotPresent"` |  |
 | image.repository | string | `"ghcr.io/d2iq-labs/capi-runtime-extensions"` |  |
 | image.tag | string | `""` |  |

diff --git a/charts/capi-runtime-extensions/templates/deployment.yaml b/charts/capi-runtime-extensions/templates/deployment.yaml
@@ -34,7 +34,9 @@ spec:
         - --cni.cilium.crs.defaults-namespace={{ .Release.Namespace }}
         - --cni.cilium.helm-addon.defaults-namespace={{ .Release.Namespace }}
         - --cni.cilium.helm-addon.default-values-template-configmap-name={{ .Values.hooks.cni.cilium.helmAddonStrategy.defaultValueTemplateConfigMap.name }}
-        - --nfd.defaults-namespace={{ .Release.Namespace }}
+        - --nfd.crs.defaults-namespace={{ .Release.Namespace }}
+        - --nfd.helm-addon.defaults-namespace={{ .Release.Namespace }}
+        - --nfd.helm-addon.default-values-template-configmap-name={{ .Values.hooks.nfd.helmAddonStrategy.defaultValueTemplateConfigMap.name }}
         {{- range $key, $value := .Values.extraArgs }}
         - --{{ $key }}={{ $value }}
         {{- end }}

diff --git a/charts/capi-runtime-extensions/templates/nfd/manifests/helm-addon-installation.yaml b/charts/capi-runtime-extensions/templates/nfd/manifests/helm-addon-installation.yaml
@@ -0,0 +1,30 @@
+# Copyright 2024 D2iQ, Inc. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+{{- if .Values.hooks.nfd.helmAddonStrategy.defaultValueTemplateConfigMap.create }}
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: '{{ .Values.hooks.nfd.helmAddonStrategy.defaultValueTemplateConfigMap.name }}'
+data:
+  values.yaml: |-
+    master:
+      extraLabelNs:
+        - nvidia.com
+        - beta.amd.com
+        - amd.com
+
+    worker: ### <NFD-WORKER-CONF-START-DO-NOT-REMOVE>
+      config:
+        sources:
+          pci:
+            deviceLabelFields:
+              - "class"
+              - "vendor"
+      tolerations:
+        - effect: NoSchedule
+          key: node-role.kubernetes.io/master
+        - effect: NoSchedule
+          key: node-role.kubernetes.io/control-plane
+    ### <NFD-WORKER-CONF-END-DO-NOT-REMOVE>
+{{- end -}}
diff --git a/charts/capi-runtime-extensions/templates/nfd/manifests/node-feature-discovery-configmap.yaml b/charts/capi-runtime-extensions/templates/nfd/manifests/node-feature-discovery-configmap.yaml
@@ -924,4 +924,4 @@ data:
 kind: ConfigMap
 metadata:
   creationTimestamp: null
-  name: node-feature-discovery
+  name: '{{ .Values.hooks.nfd.crsStrategy.defaultInstallationConfigMap.name }}'
diff --git a/charts/capi-runtime-extensions/values.yaml b/charts/capi-runtime-extensions/values.yaml
@@ -35,6 +35,14 @@ hooks:
         defaultValueTemplateConfigMap:
           create: true
           name: default-cilium-cni-helm-values-template
+  nfd:
+    crsStrategy:
+      defaultInstallationConfigMap:
+        name: node-feature-discovery
+    helmAddonStrategy:
+      defaultValueTemplateConfigMap:
+        create: true
+        name: default-nfd-helm-values-template
 
 deployDefaultClusterClasses: true
 

diff --git a/docs/content/addons/nfd.md b/docs/content/addons/nfd.md
@@ -25,5 +25,6 @@ spec:
       - name: clusterConfig
         value:
           addons:
-            nfd: {}
+            nfd:
+              strategy: HelmAddon
 ```
diff --git a/examples/capi-quick-start/aws-cluster-calico-crs.yaml b/examples/capi-quick-start/aws-cluster-calico-crs.yaml
@@ -28,7 +28,8 @@ spec:
             csi:
               providers:
                 - name: aws-ebs
-            nfd: {}
+            nfd:
+              strategy: ClusterResourceSet
           aws:
             region: us-west-2
     version: ${KUBERNETES_VERSION}

diff --git a/examples/capi-quick-start/aws-cluster-calico-helm-addon.yaml b/examples/capi-quick-start/aws-cluster-calico-helm-addon.yaml
@@ -28,7 +28,8 @@ spec:
             csi:
               providers:
                 - name: aws-ebs
-            nfd: {}
+            nfd:
+              strategy: HelmAddon
           aws:
             region: us-west-2
     version: ${KUBERNETES_VERSION}

diff --git a/examples/capi-quick-start/aws-cluster-cilium-crs.yaml b/examples/capi-quick-start/aws-cluster-cilium-crs.yaml
@@ -28,7 +28,8 @@ spec:
             csi:
               providers:
                 - name: aws-ebs
-            nfd: {}
+            nfd:
+              strategy: ClusterResourceSet
           aws:
             region: us-west-2
     version: ${KUBERNETES_VERSION}

diff --git a/examples/capi-quick-start/aws-cluster-cilium-helm-addon.yaml b/examples/capi-quick-start/aws-cluster-cilium-helm-addon.yaml
@@ -28,7 +28,8 @@ spec:
             csi:
               providers:
                 - name: aws-ebs
-            nfd: {}
+            nfd:
+              strategy: HelmAddon
           aws:
             region: us-west-2
     version: ${KUBERNETES_VERSION}

diff --git a/examples/capi-quick-start/docker-cluster-calico-crs.yaml b/examples/capi-quick-start/docker-cluster-calico-crs.yaml
@@ -25,7 +25,8 @@ spec:
             cni:
               provider: Calico
               strategy: ClusterResourceSet
-            nfd: {}
+            nfd:
+              strategy: ClusterResourceSet
           docker: {}
     version: ${KUBERNETES_VERSION}
     workers:

diff --git a/examples/capi-quick-start/docker-cluster-calico-helm-addon.yaml b/examples/capi-quick-start/docker-cluster-calico-helm-addon.yaml
@@ -25,7 +25,8 @@ spec:
             cni:
               provider: Calico
               strategy: HelmAddon
-            nfd: {}
+            nfd:
+              strategy: HelmAddon
           docker: {}
     version: ${KUBERNETES_VERSION}
     workers:

diff --git a/examples/capi-quick-start/docker-cluster-cilium-crs.yaml b/examples/capi-quick-start/docker-cluster-cilium-crs.yaml
@@ -25,7 +25,8 @@ spec:
             cni:
               provider: Cilium
               strategy: ClusterResourceSet
-            nfd: {}
+            nfd:
+              strategy: ClusterResourceSet
           docker: {}
     version: ${KUBERNETES_VERSION}
     workers:

diff --git a/examples/capi-quick-start/docker-cluster-cilium-helm-addon.yaml b/examples/capi-quick-start/docker-cluster-cilium-helm-addon.yaml
@@ -25,7 +25,8 @@ spec:
             cni:
               provider: Cilium
               strategy: HelmAddon
-            nfd: {}
+            nfd:
+              strategy: HelmAddon
           docker: {}
     version: ${KUBERNETES_VERSION}
     workers:

diff --git a/hack/addons/update-node-feature-discovery-manifests.sh b/hack/addons/update-node-feature-discovery-manifests.sh
@@ -24,7 +24,7 @@ envsubst -no-unset <"${KUSTOMIZE_BASE_DIR}/kustomization.yaml.tmpl" >"${ASSETS_D
 cp "${KUSTOMIZE_BASE_DIR}"/*.yaml "${ASSETS_DIR}"
 kustomize build --enable-helm "${ASSETS_DIR}" >"${ASSETS_DIR}/${FILE_NAME}"
 
-kubectl create configmap node-feature-discovery --dry-run=client --output yaml \
+kubectl create configmap "{{ .Values.hooks.nfd.crsStrategy.defaultInstallationConfigMap.name }}" --dry-run=client --output yaml \
   --from-file "${ASSETS_DIR}/${FILE_NAME}" \
   >"${ASSETS_DIR}/node-feature-discovery-configmap.yaml"
 

diff --git a/hack/examples/bases/aws/calico/crs/kustomization.yaml.tmpl b/hack/examples/bases/aws/calico/crs/kustomization.yaml.tmpl
@@ -24,6 +24,10 @@ patches:
               cni:
                 provider: Calico
                 strategy: ClusterResourceSet
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: ClusterResourceSet
 
 # Delete all ClusterClass related resources to prevent duplicate resources because we use the same resources definition
 # above- we only actually want the Cluster resource to mutate here.

diff --git a/hack/examples/bases/aws/calico/helm-addon/kustomization.yaml.tmpl b/hack/examples/bases/aws/calico/helm-addon/kustomization.yaml.tmpl
@@ -24,3 +24,7 @@ patches:
               cni:
                 provider: Calico
                 strategy: HelmAddon
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: HelmAddon
diff --git a/hack/examples/bases/aws/cilium/crs/kustomization.yaml.tmpl b/hack/examples/bases/aws/cilium/crs/kustomization.yaml.tmpl
@@ -24,6 +24,10 @@ patches:
               cni:
                 provider: Cilium
                 strategy: ClusterResourceSet
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: ClusterResourceSet
 
 # Delete all ClusterClass related resources to prevent duplicate resources because we use the same resources definition
 # above- we only actually want the Cluster resource to mutate here.

diff --git a/hack/examples/bases/aws/cilium/helm-addon/kustomization.yaml.tmpl b/hack/examples/bases/aws/cilium/helm-addon/kustomization.yaml.tmpl
@@ -24,6 +24,10 @@ patches:
               cni:
                 provider: Cilium
                 strategy: HelmAddon
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: HelmAddon
 
 # Delete all ClusterClass related resources to prevent duplicate resources because we use the same resources definition
 # above- we only actually want the Cluster resource to mutate here.

diff --git a/hack/examples/bases/aws/kustomization.yaml.tmpl b/hack/examples/bases/aws/kustomization.yaml.tmpl
@@ -32,9 +32,6 @@ patches:
       path: "/spec/topology/variables/0/value/aws"
       value:
         region: us-west-2
-    - op: "add"
-      path: "/spec/topology/variables/0/value/addons/nfd"
-      value: {}
     - op: "add"
       path: "/spec/topology/variables/0/value/addons/cpi"
       value: {}

diff --git a/hack/examples/bases/docker/calico/crs/kustomization.yaml.tmpl b/hack/examples/bases/docker/calico/crs/kustomization.yaml.tmpl
@@ -24,3 +24,7 @@ patches:
               cni:
                 provider: Calico
                 strategy: ClusterResourceSet
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: ClusterResourceSet
diff --git a/hack/examples/bases/docker/calico/helm-addon/kustomization.yaml.tmpl b/hack/examples/bases/docker/calico/helm-addon/kustomization.yaml.tmpl
@@ -24,3 +24,7 @@ patches:
               cni:
                 provider: Calico
                 strategy: HelmAddon
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: HelmAddon
diff --git a/hack/examples/bases/docker/cilium/crs/kustomization.yaml.tmpl b/hack/examples/bases/docker/cilium/crs/kustomization.yaml.tmpl
@@ -24,3 +24,7 @@ patches:
               cni:
                 provider: Cilium
                 strategy: ClusterResourceSet
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: ClusterResourceSet
diff --git a/hack/examples/bases/docker/cilium/helm-addon/kustomization.yaml.tmpl b/hack/examples/bases/docker/cilium/helm-addon/kustomization.yaml.tmpl
@@ -24,3 +24,7 @@ patches:
               cni:
                 provider: Cilium
                 strategy: HelmAddon
+    - op: "add"
+      path: "/spec/topology/variables/0/value/addons/nfd"
+      value:
+        strategy: HelmAddon
diff --git a/hack/examples/bases/docker/kustomization.yaml.tmpl b/hack/examples/bases/docker/kustomization.yaml.tmpl
@@ -31,9 +31,6 @@ patches:
     - op: "add"
       path: "/spec/topology/variables/0/value/docker"
       value: {}
-    - op: "add"
-      path: "/spec/topology/variables/0/value/addons/nfd"
-      value: {}
     - op: "remove"
       path: "/spec/topology/workers/machinePools"
 - target:

diff --git a/make/addons.mk b/make/addons.mk
@@ -3,7 +3,7 @@
 
 export CALICO_VERSION := $(shell goprintconst -file pkg/handlers/generic/lifecycle/cni/calico/strategy_helmaddon.go -name defaultCalicoHelmChartVersion)
 export CILIUM_VERSION := $(shell goprintconst -file pkg/handlers/generic/lifecycle/cni/cilium/strategy_helmaddon.go -name defaultCiliumHelmChartVersion)
-export NODE_FEATURE_DISCOVERY_VERSION := 0.14.1
+export NODE_FEATURE_DISCOVERY_VERSION := $(shell goprintconst -file pkg/handlers/generic/lifecycle/nfd/strategy_helmaddon.go -name defaultHelmChartVersion)
 export AWS_CSI_SNAPSHOT_CONTROLLER_VERSION := v6.3.0
 export AWS_EBS_CSI_VERSION := v1.25.0
 # a map of AWS CPI versions

diff --git a/pkg/handlers/generic/lifecycle/cni/cilium/strategy_crs.go b/pkg/handlers/generic/lifecycle/cni/cilium/strategy_crs.go
@@ -6,7 +6,6 @@ package cilium
 import (
 	"context"
 	"fmt"
-	"maps"
 
 	"github.com/go-logr/logr"
 	"github.com/spf13/pflag"
@@ -85,8 +84,9 @@ func (s crsStrategy) apply(
 			Namespace: cluster.Namespace,
 			Name:      "cilium-cni-installation-" + cluster.Name,
 		},
+		Data:       defaultCiliumConfigMap.Data,
+		BinaryData: defaultCiliumConfigMap.BinaryData,
 	}
-	cm.Data = maps.Clone(defaultCiliumConfigMap.Data)
 
 	if err := client.ServerSideApply(ctx, s.client, cm); err != nil {
 		return fmt.Errorf(
@@ -95,7 +95,7 @@ func (s crsStrategy) apply(
 		)
 	}
 
-	if err := utils.EnsureCRSForClusterFromConfigMaps(ctx, cm.Name, s.client, &req.Cluster, cm); err != nil {
+	if err := utils.EnsureCRSForClusterFromConfigMaps(ctx, cm.Name, s.client, cluster, cm); err != nil {
 		return fmt.Errorf(
 			"failed to apply Cilium CNI installation ClusterResourceSet: %w",
 			err,

diff --git a/pkg/handlers/generic/lifecycle/handlers.go b/pkg/handlers/generic/lifecycle/handlers.go
@@ -22,7 +22,7 @@ import (
 type Handlers struct {
 	calicoCNIConfig *calico.CNIConfig
 	ciliumCNIConfig *cilium.CNIConfig
-	nfdConfig       *nfd.NFDConfig
+	nfdConfig       *nfd.Config
 	ebsConfig       *awsebs.AWSEBSConfig
 	awsCPIConfig    *awscpi.AWSCPIConfig
 }
@@ -31,7 +31,7 @@ func New() *Handlers {
 	return &Handlers{
 		calicoCNIConfig: &calico.CNIConfig{},
 		ciliumCNIConfig: &cilium.CNIConfig{},
-		nfdConfig:       &nfd.NFDConfig{},
+		nfdConfig:       &nfd.Config{},
 		ebsConfig:       &awsebs.AWSEBSConfig{},
 		awsCPIConfig:    &awscpi.AWSCPIConfig{},
 	}