diff --git a/api/v1alpha1/addon_types.go b/api/v1alpha1/addon_types.go index abc48c4d9..cfc0187cc 100644 --- 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 index f05b39324..a4454d305 100644 --- 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 index efb9b256e..8d20fcfb7 100644 --- 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 index e318af76c..d6e9d6c73 100644 --- 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 new file mode 100644 index 000000000..6f3cc6859 --- /dev/null +++ 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: ### + config: + sources: + pci: + deviceLabelFields: + - "class" + - "vendor" + tolerations: + - effect: NoSchedule + key: node-role.kubernetes.io/master + - effect: NoSchedule + key: node-role.kubernetes.io/control-plane + ### +{{- 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 index 797b1cf64..fef2b2e6b 100644 --- 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 index e3090ef79..9b904a2fd 100644 --- 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 index cfcb9b51e..69b08f4f5 100644 --- 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 index fc214e70a..082d577cc 100644 --- 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 index ad124f375..5e0e5270d 100644 --- 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 index 3680364cf..6367afc5c 100644 --- 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 index 07de5aec2..c11cc20c6 100644 --- 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 index c32022770..873f8463d 100644 --- 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 index 2558972e1..fedefeb44 100644 --- 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 index 6fd5580c6..c50a5ebcf 100644 --- 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 index 220e357d4..664bbcdf0 100644 --- 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 index 9117cfe17..2ad783bc4 100755 --- 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 index 324d05589..33d6b8977 100644 --- 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 index 0020bff96..1c95b2e2b 100644 --- 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 index 5cbc0e6ca..cb1ccba42 100644 --- 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 index c874cfc9d..55c4c6045 100644 --- 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 index be7ded9aa..0fe8e819b 100644 --- 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 index c71728d84..d8b31d0be 100644 --- 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 index 5448d7c39..79a3b8ba5 100644 --- 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 index 1cff58c15..5edc53beb 100644 --- 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 index d34a22d4b..6f03a4941 100644 --- 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 index a23f2c13f..613407191 100644 --- 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 index 0ee7a1d98..17984be46 100644 --- 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 index 336c466c3..b70a1083d 100644 --- 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 index 8219a7e28..593e4351d 100644 --- 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{}, } diff --git a/pkg/handlers/generic/lifecycle/nfd/handler.go b/pkg/handlers/generic/lifecycle/nfd/handler.go index 194f46a35..37afa90ba 100644 --- a/pkg/handlers/generic/lifecycle/nfd/handler.go +++ b/pkg/handlers/generic/lifecycle/nfd/handler.go @@ -7,62 +7,55 @@ import ( "context" "fmt" + "github.com/go-logr/logr" "github.com/spf13/pflag" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - capiv1 "sigs.k8s.io/cluster-api/api/v1beta1" runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" ctrl "sigs.k8s.io/controller-runtime" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "github.com/d2iq-labs/capi-runtime-extensions/api/v1alpha1" + commonhandlers "github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers" + "github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers/lifecycle" "github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/variables" - "github.com/d2iq-labs/capi-runtime-extensions/common/pkg/k8s/client" "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/clusterconfig" - "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/lifecycle/utils" ) -type NFDConfig struct { - defaultsNamespace string - defaultNFDConfigMap string +type addonStrategy interface { + apply(context.Context, *runtimehooksv1.AfterControlPlaneInitializedRequest, logr.Logger) error +} + +type Config struct { + crsConfig crsConfig + helmAddonConfig helmAddonConfig +} + +func (c *Config) AddFlags(prefix string, flags *pflag.FlagSet) { + c.crsConfig.AddFlags(prefix+".crs", flags) + c.helmAddonConfig.AddFlags(prefix+".helm-addon", flags) } type DefaultNFD struct { client ctrlclient.Client - config *NFDConfig + config *Config variableName string // points to the global config variable variablePath []string // path of this variable on the global config variable } -func (n *NFDConfig) AddFlags(prefix string, flags *pflag.FlagSet) { - flags.StringVar( - &n.defaultsNamespace, - prefix+".defaults-namespace", - corev1.NamespaceDefault, - "namespace location of ConfigMap used to deploy Node Feature Discovery (NFD).", - ) - flags.StringVar( - &n.defaultNFDConfigMap, - prefix+".default-nfd-configmap-name", - "node-feature-discovery", - "name of the ConfigMap used to deploy Node Feature Discovery (NFD)", - ) -} - -const ( - variableName = "nfd" +var ( + _ commonhandlers.Named = &DefaultNFD{} + _ lifecycle.AfterControlPlaneInitialized = &DefaultNFD{} ) func New( c ctrlclient.Client, - cfg *NFDConfig, + cfg *Config, ) *DefaultNFD { return &DefaultNFD{ client: c, config: cfg, variableName: clusterconfig.MetaVariableName, - variablePath: []string{"addons", variableName}, + variablePath: []string{"addons", v1alpha1.NFDVariableName}, } } @@ -81,77 +74,51 @@ func (n *DefaultNFD) AfterControlPlaneInitialized( "cluster", clusterKey, ) + varMap := variables.ClusterVariablesToVariablesMap(req.Cluster.Spec.Topology.Variables) - _, found, err := variables.Get[v1alpha1.NFD](varMap, n.variableName, n.variablePath...) + cniVar, found, err := variables.Get[v1alpha1.NFD](varMap, n.variableName, n.variablePath...) if err != nil { + log.Error( + err, + "failed to read NFD variable from cluster definition", + ) resp.SetStatus(runtimehooksv1.ResponseStatusFailure) - log.Error(err, "failed to get NFD variable") + resp.SetMessage( + fmt.Sprintf("failed to read NFD variable from cluster definition: %v", + err, + ), + ) return } - // If the variable isn't there or disabled we can ignore it. if !found { - log.V(4).Info( - "Skipping NFD handler. Not specified in cluster config.", - ) + log.Info("Skipping NFD handler, cluster does not specify request NFDaddon deployment") return } - cm, err := n.ensureNFDConfigMapForCluster(ctx, &req.Cluster) - if err != nil { + var strategy addonStrategy + switch cniVar.Strategy { + case v1alpha1.AddonStrategyClusterResourceSet: + strategy = crsStrategy{ + config: n.config.crsConfig, + client: n.client, + } + case v1alpha1.AddonStrategyHelmAddon: + strategy = helmAddonStrategy{ + config: n.config.helmAddonConfig, + client: n.client, + } + default: resp.SetStatus(runtimehooksv1.ResponseStatusFailure) - log.Error(err, "failed to apply NFD ConfigMap for cluster") + resp.SetMessage(fmt.Sprintf("unknown NFD addon deployment strategy %q", cniVar.Strategy)) return } - err = utils.EnsureCRSForClusterFromConfigMaps( - ctx, - cm.Name+"-"+req.Cluster.Name, - n.client, - &req.Cluster, - cm, - ) - if err != nil { + + if err := strategy.apply(ctx, req, log); err != nil { resp.SetStatus(runtimehooksv1.ResponseStatusFailure) - log.Error(err, "failed to apply NFD ClusterResourceSet for cluster") + resp.SetMessage(err.Error()) return } resp.SetStatus(runtimehooksv1.ResponseStatusSuccess) } - -// ensureNFDConfigMapForCluster is a private function that creates a configMap for the cluster. -func (n *DefaultNFD) ensureNFDConfigMapForCluster( - ctx context.Context, - cluster *capiv1.Cluster, -) (*corev1.ConfigMap, error) { - key := ctrlclient.ObjectKey{ - Namespace: n.config.defaultsNamespace, - Name: n.config.defaultNFDConfigMap, - } - cm := &corev1.ConfigMap{} - err := n.client.Get(ctx, key, cm) - if err != nil { - return nil, fmt.Errorf( - "failed to fetch the configmap specified by %v: %w", - n.config, - err, - ) - } - // Base configmap is there now we create one in the cluster namespace if needed. - cmForCluster := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "ConfigMap", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: cluster.Namespace, - Name: n.config.defaultNFDConfigMap, - }, - Data: cm.Data, - } - err = client.ServerSideApply(ctx, n.client, cmForCluster) - if err != nil { - return nil, fmt.Errorf("failed to apply NFD ConfigMap for cluster: %w", err) - } - return cmForCluster, nil -} diff --git a/pkg/handlers/generic/lifecycle/nfd/strategy_crs.go b/pkg/handlers/generic/lifecycle/nfd/strategy_crs.go new file mode 100644 index 000000000..80b647f3e --- /dev/null +++ b/pkg/handlers/generic/lifecycle/nfd/strategy_crs.go @@ -0,0 +1,106 @@ +// Copyright 2024 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nfd + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "github.com/spf13/pflag" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/d2iq-labs/capi-runtime-extensions/common/pkg/k8s/client" + "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/lifecycle/utils" +) + +type crsConfig struct { + defaultsNamespace string + + defaultNFDConfigMap string +} + +func (c *crsConfig) AddFlags(prefix string, flags *pflag.FlagSet) { + flags.StringVar( + &c.defaultsNamespace, + prefix+".defaults-namespace", + corev1.NamespaceDefault, + "namespace of the ConfigMap used to deploy Cilium", + ) + + flags.StringVar( + &c.defaultNFDConfigMap, + prefix+".default-nfd-configmap-name", + "node-feature-discovery", + "name of the ConfigMap used to deploy Node Feature Discovery (NFD)", + ) +} + +type crsStrategy struct { + config crsConfig + + client ctrlclient.Client +} + +func (s crsStrategy) apply( + ctx context.Context, + req *runtimehooksv1.AfterControlPlaneInitializedRequest, + log logr.Logger, +) error { + defaultCM := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: s.config.defaultsNamespace, + Name: s.config.defaultNFDConfigMap, + }, + } + + err := s.client.Get( + ctx, + ctrlclient.ObjectKeyFromObject(defaultCM), + defaultCM, + ) + if err != nil { + return fmt.Errorf("failed to get default NFD ConfigMap: %w", err) + } + + log.Info("Ensuring NFD ConfigMap exists for cluster") + + cluster := &req.Cluster + + cm := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: defaultCM.Name + "-" + cluster.Name, + }, + Data: defaultCM.Data, + BinaryData: defaultCM.BinaryData, + } + + if err := client.ServerSideApply(ctx, s.client, cm); err != nil { + return fmt.Errorf( + "failed to apply NFD installation ConfigMap: %w", + err, + ) + } + + if err := utils.EnsureCRSForClusterFromConfigMaps(ctx, cm.Name, s.client, cluster, cm); err != nil { + return fmt.Errorf( + "failed to apply NFD installation ClusterResourceSet: %w", + err, + ) + } + + return nil +} diff --git a/pkg/handlers/generic/lifecycle/nfd/strategy_helmaddon.go b/pkg/handlers/generic/lifecycle/nfd/strategy_helmaddon.go new file mode 100644 index 000000000..3507e5714 --- /dev/null +++ b/pkg/handlers/generic/lifecycle/nfd/strategy_helmaddon.go @@ -0,0 +1,136 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nfd + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "github.com/spf13/pflag" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + capiv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + caaphv1 "github.com/d2iq-labs/capi-runtime-extensions/api/external/sigs.k8s.io/cluster-api-addon-provider-helm/api/v1alpha1" + "github.com/d2iq-labs/capi-runtime-extensions/common/pkg/k8s/client" +) + +const ( + defaultHelmRepositoryURL = "https://kubernetes-sigs.github.io/node-feature-discovery/charts" + defaultHelmChartVersion = "0.14.1" + defaultHelmChartName = "node-feature-discovery" + defaultHelmReleaseName = "node-feature-discovery" + defaultHelmReleaseNamespace = "node-feature-discovery" +) + +type helmAddonConfig struct { + defaultsNamespace string + defaultValuesTemplateConfigMapName string +} + +func (c *helmAddonConfig) AddFlags(prefix string, flags *pflag.FlagSet) { + flags.StringVar( + &c.defaultsNamespace, + prefix+".defaults-namespace", + corev1.NamespaceDefault, + "namespace of the default Helm values ConfigMap", + ) + + flags.StringVar( + &c.defaultValuesTemplateConfigMapName, + prefix+".default-values-template-configmap-name", + "default-nfd-helm-values-template", + "default values ConfigMap name", + ) +} + +type helmAddonStrategy struct { + config helmAddonConfig + + client ctrlclient.Client +} + +func (s helmAddonStrategy) apply( + ctx context.Context, + req *runtimehooksv1.AfterControlPlaneInitializedRequest, + log logr.Logger, +) error { + log.Info("Retrieving NFD installation values template for cluster") + valuesTemplateConfigMap, err := s.retrieveValuesTemplateConfigMap(ctx) + if err != nil { + return fmt.Errorf( + "failed to retrieve NFD installation values template ConfigMap for cluster: %w", + err, + ) + } + + values := valuesTemplateConfigMap.Data["values.yaml"] + values += fmt.Sprintf(` +image: + tag: v%s-minimal +`, defaultHelmChartVersion) + + hcp := &caaphv1.HelmChartProxy{ + TypeMeta: metav1.TypeMeta{ + APIVersion: caaphv1.GroupVersion.String(), + Kind: "HelmChartProxy", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: req.Cluster.Namespace, + Name: "node-feature-discovery-" + req.Cluster.Name, + }, + Spec: caaphv1.HelmChartProxySpec{ + RepoURL: defaultHelmRepositoryURL, + ChartName: defaultHelmChartName, + ClusterSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{capiv1.ClusterNameLabel: req.Cluster.Name}, + }, + ReleaseNamespace: defaultHelmReleaseNamespace, + ReleaseName: defaultHelmReleaseName, + Version: defaultHelmChartVersion, + ValuesTemplate: values, + }, + } + + if err := controllerutil.SetOwnerReference(&req.Cluster, hcp, s.client.Scheme()); err != nil { + return fmt.Errorf( + "failed to set owner reference on NFD installation HelmChartProxy: %w", + err, + ) + } + + if err := client.ServerSideApply(ctx, s.client, hcp); err != nil { + return fmt.Errorf("failed to apply NFD installation HelmChartProxy: %w", err) + } + + return nil +} + +func (s helmAddonStrategy) retrieveValuesTemplateConfigMap( + ctx context.Context, +) (*corev1.ConfigMap, error) { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: s.config.defaultsNamespace, + Name: s.config.defaultValuesTemplateConfigMapName, + }, + } + configMapObjName := ctrlclient.ObjectKeyFromObject( + configMap, + ) + err := s.client.Get(ctx, configMapObjName, configMap) + if err != nil { + return nil, fmt.Errorf( + "failed to retrieve installation values template ConfigMap %q: %w", + configMapObjName, + err, + ) + } + + return configMap, nil +} diff --git a/pkg/handlers/generic/lifecycle/nfd/variables_test.go b/pkg/handlers/generic/lifecycle/nfd/variables_test.go new file mode 100644 index 000000000..22e35114f --- /dev/null +++ b/pkg/handlers/generic/lifecycle/nfd/variables_test.go @@ -0,0 +1,55 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nfd + +import ( + "testing" + + "k8s.io/utils/ptr" + + "github.com/d2iq-labs/capi-runtime-extensions/api/v1alpha1" + "github.com/d2iq-labs/capi-runtime-extensions/common/pkg/testutils/capitest" + "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/clusterconfig" +) + +func TestVariableValidation(t *testing.T) { + capitest.ValidateDiscoverVariables( + t, + clusterconfig.MetaVariableName, + ptr.To(v1alpha1.GenericClusterConfig{}.VariableSchema()), + false, + clusterconfig.NewVariable, + capitest.VariableTestDef{ + Name: "ClusterResourceSet strategy", + Vals: v1alpha1.GenericClusterConfig{ + Addons: &v1alpha1.Addons{ + NFD: &v1alpha1.NFD{ + Strategy: v1alpha1.AddonStrategyClusterResourceSet, + }, + }, + }, + }, + capitest.VariableTestDef{ + Name: "HelmAddon strategy", + Vals: v1alpha1.GenericClusterConfig{ + Addons: &v1alpha1.Addons{ + NFD: &v1alpha1.NFD{ + Strategy: v1alpha1.AddonStrategyHelmAddon, + }, + }, + }, + }, + capitest.VariableTestDef{ + Name: "invalid strategy", + Vals: v1alpha1.GenericClusterConfig{ + Addons: &v1alpha1.Addons{ + NFD: &v1alpha1.NFD{ + Strategy: "invalid-strategy", + }, + }, + }, + ExpectError: true, + }, + ) +} diff --git a/test/e2e/nfd_helpers.go b/test/e2e/nfd_helpers.go index 24b9402bf..c99bc105d 100644 --- a/test/e2e/nfd_helpers.go +++ b/test/e2e/nfd_helpers.go @@ -7,7 +7,9 @@ package e2e import ( "context" + "fmt" + . "github.com/onsi/ginkgo/v2" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" capiv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -34,15 +36,35 @@ func WaitForNFDToBeReadyInWorkloadCluster( return } - waitForClusterResourceSetToApplyResourcesInCluster( - ctx, - waitForClusterResourceSetToApplyResourcesInClusterInput{ - name: "node-feature-discovery-" + input.WorkloadCluster.Name, - clusterProxy: input.ClusterProxy, - cluster: input.WorkloadCluster, - intervals: input.ClusterResourceSetIntervals, - }, - ) + switch input.NFD.Strategy { + case v1alpha1.AddonStrategyClusterResourceSet: + waitForClusterResourceSetToApplyResourcesInCluster( + ctx, + waitForClusterResourceSetToApplyResourcesInClusterInput{ + name: "node-feature-discovery-" + input.WorkloadCluster.Name, + clusterProxy: input.ClusterProxy, + cluster: input.WorkloadCluster, + intervals: input.ClusterResourceSetIntervals, + }, + ) + case v1alpha1.AddonStrategyHelmAddon: + WaitForHelmReleaseProxyReadyForCluster( + ctx, + WaitForHelmReleaseProxyReadyForClusterInput{ + GetLister: input.ClusterProxy.GetClient(), + Cluster: input.WorkloadCluster, + HelmChartProxyName: "node-feature-discovery-" + input.WorkloadCluster.Name, + }, + input.HelmReleaseIntervals..., + ) + default: + Fail( + fmt.Sprintf( + "Do not know how to wait for Calico using strategy %s to be ready", + input.NFD.Strategy, + ), + ) + } workloadClusterClient := input.ClusterProxy.GetWorkloadCluster( ctx, input.WorkloadCluster.Namespace, input.WorkloadCluster.Name,