From 10c64a2a2a6b5ecde378a860270d4579b3a5acad Mon Sep 17 00:00:00 2001 From: Yuva Shankar <11082310+yuva29@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:43:32 +0000 Subject: [PATCH] [CP 362+363+366] cleanup DevicePluginArguments, consolidate driver version, port validation Cherry-pick of pensando PRs #362, #363, #366 to rocm release-v1.2.1: - #362: Remove unused DevicePluginArguments field from CRD - #363: Consolidate default driver version to 1.117.5-a-147, use explicit versions in E2E tests - #366: Add port range validation (1-65535) to spec.metricsExporter.port Co-Authored-By: Claude Opus 4 (1M context) --- api/v1alpha1/networkconfig_types.go | 10 +++----- api/v1alpha1/zz_generated.deepcopy.go | 7 ------ config/crd/bases/amd.com_networkconfigs.yaml | 11 +++------ ...etwork-operator.clusterserviceversion.yaml | 17 ++++---------- helm-charts-k8s/crds/networkconfig-crd.yaml | 11 +++------ internal/deviceplugin/deviceplugin.go | 8 +------ internal/kmmmodule/kmmmodule.go | 3 +-- internal/utils.go | 23 ++++++++----------- tests/e2e/driver_test.go | 6 ++--- tests/e2e/suite.go | 2 +- 10 files changed, 30 insertions(+), 68 deletions(-) diff --git a/api/v1alpha1/networkconfig_types.go b/api/v1alpha1/networkconfig_types.go index 7977d877..59e14b51 100644 --- a/api/v1alpha1/networkconfig_types.go +++ b/api/v1alpha1/networkconfig_types.go @@ -57,12 +57,6 @@ type DevicePluginSpec struct { // +optional DevicePluginTolerations []v1.Toleration `json:"devicePluginTolerations,omitempty"` - // device plugin arguments is used to pass supported flags and their values while starting device plugin daemonset - // supported flag values: {"resource_naming_strategy": {"single", "mixed"}} - //+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="DevicePluginArguments",xDescriptors={"urn:alm:descriptor:com.amd.networkconfigs:devicePluginArguments"} - // +optional - DevicePluginArguments map[string]string `json:"devicePluginArguments,omitempty"` - // node labeller image //+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="NodeLabellerImage",xDescriptors={"urn:alm:descriptor:com.amd.networkconfigs:nodeLabellerImage"} // +optional @@ -229,7 +223,7 @@ type DriverSpec struct { AMDNetworkInstallerRepoURL string `json:"AMDNetworkInstallerRepoURL,omitempty"` // version of the drivers source code, can be used as part of image of dockerfile source image - // default value for different OS is: ubuntu: 1.117.1-a-42, coreOS: 1.117.1-a-42 + // default value for different OS is: ubuntu: 1.117.5-a-147, coreOS: 1.117.5-a-147 //+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Version",xDescriptors={"urn:alm:descriptor:com.amd.NetworkConfigs:version"} // +optional Version string `json:"version,omitempty"` @@ -501,6 +495,8 @@ type MetricsExporterSpec struct { // Port is the internal port used for in-cluster and node access to pull metrics from the metrics-exporter (default 5001). //+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Port",xDescriptors={"urn:alm:descriptor:com.amd.networkconfigs:port"} // +kubebuilder:default=5001 + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=65535 Port int32 `json:"port,omitempty"` // ServiceType service type for metrics, clusterIP/NodePort, clusterIP by default diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c4940b61..f19a971e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -185,13 +185,6 @@ func (in *DevicePluginSpec) DeepCopyInto(out *DevicePluginSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.DevicePluginArguments != nil { - in, out := &in.DevicePluginArguments, &out.DevicePluginArguments - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } if in.NodeLabellerTolerations != nil { in, out := &in.NodeLabellerTolerations, &out.NodeLabellerTolerations *out = make([]v1.Toleration, len(*in)) diff --git a/config/crd/bases/amd.com_networkconfigs.yaml b/config/crd/bases/amd.com_networkconfigs.yaml index 43e9a0ca..2b9bc02a 100644 --- a/config/crd/bases/amd.com_networkconfigs.yaml +++ b/config/crd/bases/amd.com_networkconfigs.yaml @@ -195,13 +195,6 @@ spec: devicePlugin: description: device plugin properties: - devicePluginArguments: - additionalProperties: - type: string - description: |- - device plugin arguments is used to pass supported flags and their values while starting device plugin daemonset - supported flag values: {"resource_naming_strategy": {"single", "mixed"}} - type: object devicePluginImage: description: device plugin image pattern: ^([a-z0-9]+(?:[._-][a-z0-9]+)*(:[0-9]+)?)(/[a-z0-9]+(?:[._-][a-z0-9]+)*)*(?::[a-z0-9._-]+)?(?:@[a-zA-Z0-9]+:[a-f0-9]+)?$ @@ -594,7 +587,7 @@ spec: version: description: |- version of the drivers source code, can be used as part of image of dockerfile source image - default value for different OS is: ubuntu: 1.117.1-a-42, coreOS: 1.117.1-a-42 + default value for different OS is: ubuntu: 1.117.5-a-147, coreOS: 1.117.5-a-147 type: string type: object metricsExporter: @@ -657,6 +650,8 @@ spec: node access to pull metrics from the metrics-exporter (default 5001). format: int32 + maximum: 65535 + minimum: 1 type: integer prometheus: description: Prometheus configuration for metrics exporter diff --git a/config/manifests/bases/amd-network-operator.clusterserviceversion.yaml b/config/manifests/bases/amd-network-operator.clusterserviceversion.yaml index 45e00374..269d4d03 100644 --- a/config/manifests/bases/amd-network-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/amd-network-operator.clusterserviceversion.yaml @@ -9,7 +9,7 @@ metadata: description: |- Operator responsible for deploying AMD Network kernel drivers, device plugin, node labeller and device metrics exporter For more information, visit [documentation](https://instinct.docs.amd.com/projects/network-operator/en/latest/) - devicePluginImage: docker.io/rocm/k8s-network-device-plugin:v0.0.1 + devicePluginImage: docker.io/rocm/k8s-network-device-plugin:v1.2.1 features.operators.openshift.io/disconnected: "true" features.operators.openshift.io/fips-compliant: "false" features.operators.openshift.io/proxy-aware: "true" @@ -17,8 +17,8 @@ metadata: features.operators.openshift.io/token-auth-aws: "false" features.operators.openshift.io/token-auth-azure: "false" features.operators.openshift.io/token-auth-gcp: "false" - metricsExporterImage: docker.io/rocm/device-metrics-exporter:nic-v0.0.1 - nodelabellerImage: docker.io/rocm/k8s-network-node-labeller:v0.0.1 + metricsExporterImage: docker.io/rocm/device-metrics-exporter:nic-v1.2.1 + nodelabellerImage: docker.io/rocm/k8s-network-node-labeller:v1.2.1 operatorframework.io/cluster-monitoring: "true" operatorframework.io/suggested-namespace: openshift-amd-network operators.openshift.io/valid-subscription: '[]' @@ -144,13 +144,6 @@ spec: path: devicePlugin x-descriptors: - urn:alm:descriptor:com.amd.NetworkConfigs:devicePlugin - - description: 'device plugin arguments is used to pass supported flags and - their values while starting device plugin daemonset supported flag values: - {"resource_naming_strategy": {"single", "mixed"}}' - displayName: DevicePluginArguments - path: devicePlugin.devicePluginArguments - x-descriptors: - - urn:alm:descriptor:com.amd.networkconfigs:devicePluginArguments - description: device plugin image displayName: DevicePluginImage path: devicePlugin.devicePluginImage @@ -391,8 +384,8 @@ spec: x-descriptors: - urn:alm:descriptor:com.amd.networkconfigs:useSourceImage - description: 'version of the drivers source code, can be used as part of image - of dockerfile source image default value for different OS is: ubuntu: 1.117.1-a-42, - coreOS: 1.117.1-a-42' + of dockerfile source image default value for different OS is: ubuntu: 1.117.5-a-147, + coreOS: 1.117.5-a-147' displayName: Version path: driver.version x-descriptors: diff --git a/helm-charts-k8s/crds/networkconfig-crd.yaml b/helm-charts-k8s/crds/networkconfig-crd.yaml index e48090ec..dce6a7b3 100644 --- a/helm-charts-k8s/crds/networkconfig-crd.yaml +++ b/helm-charts-k8s/crds/networkconfig-crd.yaml @@ -9,7 +9,7 @@ metadata: labels: app.kubernetes.io/component: amd-network app.kubernetes.io/part-of: amd-network - helm.sh/chart: network-operator-charts-v1.2.0 + helm.sh/chart: network-operator-charts-v1.2.1 app.kubernetes.io/name: network-operator-charts app.kubernetes.io/instance: amd-network app.kubernetes.io/version: "dev" @@ -203,13 +203,6 @@ spec: devicePlugin: description: device plugin properties: - devicePluginArguments: - additionalProperties: - type: string - description: |- - device plugin arguments is used to pass supported flags and their values while starting device plugin daemonset - supported flag values: {"resource_naming_strategy": {"single", "mixed"}} - type: object devicePluginImage: description: device plugin image pattern: ^([a-z0-9]+(?:[._-][a-z0-9]+)*(:[0-9]+)?)(/[a-z0-9]+(?:[._-][a-z0-9]+)*)*(?::[a-z0-9._-]+)?(?:@[a-zA-Z0-9]+:[a-f0-9]+)?$ @@ -663,6 +656,8 @@ spec: description: Port is the internal port used for in-cluster and node access to pull metrics from the metrics-exporter (default 5001). format: int32 + maximum: 65535 + minimum: 1 type: integer prometheus: description: Prometheus configuration for metrics exporter diff --git a/internal/deviceplugin/deviceplugin.go b/internal/deviceplugin/deviceplugin.go index 78778d82..aeeae01b 100644 --- a/internal/deviceplugin/deviceplugin.go +++ b/internal/deviceplugin/deviceplugin.go @@ -30,7 +30,7 @@ import ( const ( defaultInitContainerImage = "busybox:1.36" - defaultDevicePluginImage = "docker.io/rocm/k8s-network-device-plugin:v1.2.0" + defaultDevicePluginImage = "docker.io/rocm/k8s-network-device-plugin:v1.2.1" defaultDevicePluginConfigMap = "amd-network-operator-device-plugin-config" devicePluginSAName = "amd-network-operator-device-plugin" DevicePluginName = "device-plugin" @@ -120,12 +120,6 @@ func GenerateCommonDevicePluginSpec(nwConfig *amdv1alpha1.NetworkConfig, isOpenS dpOut.MainContainer.IsHostNetwork = true dpOut.MainContainer.Command = []string{} - var commandArgs string - for key, val := range specIn.DevicePluginArguments { - commandArgs += " -" + key + "=" + val - } - //dpOut.MainContainer.Command = []string{"sh", "-c", commandArgs} - hostPathDirectory := v1.HostPathDirectory hostPathDirectoryOrCreate := v1.HostPathDirectoryOrCreate dpOut.MainContainer.Envs = []v1.EnvVar{ diff --git a/internal/kmmmodule/kmmmodule.go b/internal/kmmmodule/kmmmodule.go index c0eaf0e7..802a97f9 100644 --- a/internal/kmmmodule/kmmmodule.go +++ b/internal/kmmmodule/kmmmodule.go @@ -66,7 +66,6 @@ const ( defaultOcDriversImageTemplate = "image-registry.openshift-image-registry.svc:5000/$MOD_NAMESPACE/amdnetwork_kmod" // start local registry image-registry:5000 in k8s defaultDriversImageTemplate = "image-registry:5000/$MOD_NAMESPACE/amdnetwork_kmod" - defaultOcDriversVersion = "1.117.5-a-56" defaultInstallerRepoURL = "https://repo.radeon.com" defaultInitContainerImage = "busybox:1.36" defaultSourceImageRepo = "docker.io/rocm/amdainic-driver" @@ -397,7 +396,7 @@ func getKM(nwConfig *amdv1alpha1.NetworkConfig, node v1.Node, inTreeModuleToRemo if isOpenShift { if driversVersion == "" { - driversVersion = defaultOcDriversVersion + driversVersion = utils.DefaultOcDriversVersion } if driversImage == "" { driversImage = defaultOcDriversImageTemplate diff --git a/internal/utils.go b/internal/utils.go index 870c244d..6a01f44b 100644 --- a/internal/utils.go +++ b/internal/utils.go @@ -32,15 +32,12 @@ import ( ) const ( - defaultOcDriversVersion = "1.117.1-a-42" - defaultUbuntuDriversVersion = "1.117.1-a-42" + DefaultOcDriversVersion = "1.117.5-a-147" + DefaultUbuntuDriversVersion = "1.117.5-a-147" openShiftNodeLabel = "node.openshift.io/os_id" NodeFeatureLabelAmdNic = "feature.node.kubernetes.io/amd-nic" NodeFeatureLabelAmdVNic = "feature.node.kubernetes.io/amd-vnic" - ResourceNamingStrategyFlag = "resource_naming_strategy" - SingleStrategy = "single" - MixedStrategy = "mixed" - DefaultUtilsImage = "docker.io/rocm/network-operator-utils:v1.2.0" + DefaultUtilsImage = "docker.io/rocm/network-operator-utils:v1.2.1" // worker pod related constants KindNetworkConfig = "NetworkConfig" @@ -77,27 +74,27 @@ func GetDefaultDriversVersion(node v1.Node) (string, error) { var defaultDriverversionsMappers = map[string]func(fullImageStr string) (string, error){ "ubuntu": UbuntuDefaultDriverVersionsMapper, "rhel": func(f string) (string, error) { - return defaultOcDriversVersion, nil + return DefaultOcDriversVersion, nil }, "redhat": func(f string) (string, error) { - return defaultOcDriversVersion, nil + return DefaultOcDriversVersion, nil }, "red hat": func(f string) (string, error) { - return defaultOcDriversVersion, nil + return DefaultOcDriversVersion, nil }, } func UbuntuDefaultDriverVersionsMapper(fullImageStr string) (string, error) { if strings.Contains(fullImageStr, "20.04") { - return defaultUbuntuDriversVersion, nil + return DefaultUbuntuDriversVersion, nil } if strings.Contains(fullImageStr, "22.04") { - return defaultUbuntuDriversVersion, nil + return DefaultUbuntuDriversVersion, nil } if strings.Contains(fullImageStr, "24.04") { - return defaultUbuntuDriversVersion, nil + return DefaultUbuntuDriversVersion, nil } - return "", fmt.Errorf("invalid ubuntu version, should be one of [20.04, 22.04]") + return "", fmt.Errorf("invalid ubuntu version, should be one of [20.04, 22.04, 24.04]") } func HasNodeLabelKey(node v1.Node, labelKey string) bool { diff --git a/tests/e2e/driver_test.go b/tests/e2e/driver_test.go index e31a6fd0..829b6bfe 100644 --- a/tests/e2e/driver_test.go +++ b/tests/e2e/driver_test.go @@ -34,7 +34,7 @@ func (s *E2ESuite) TestDriverInstallDefault(c *C) { logger.Infof("create %v", s.cfgName) netCfg := s.getNetworkConfig() netCfg.Spec.Selector = vnicselector - netCfg.Spec.Driver.Version = "" + netCfg.Spec.Driver.Version = "1.117.1-a-42" s.createNetworkConfig(netCfg, c) s.verifyOperandReadiness(c, netCfg) s.verifyNodeDriverVersionLabel(netCfg, c) @@ -138,7 +138,7 @@ func (s *E2ESuite) TestParallelUpgrade(c *C) { }{ { name: "default version to specific version", - fromVersion: "", + fromVersion: "1.117.1-a-42", toVersion: "1.117.1-a-63", upgradePolicy: v1alpha1.DriverUpgradePolicySpec{ Enable: boolPtr(true), @@ -149,7 +149,7 @@ func (s *E2ESuite) TestParallelUpgrade(c *C) { { name: "specific version to default version", fromVersion: "1.117.1-a-63", - toVersion: "", + toVersion: "1.117.1-a-42", upgradePolicy: v1alpha1.DriverUpgradePolicySpec{ Enable: boolPtr(true), RebootRequired: boolPtr(false), diff --git a/tests/e2e/suite.go b/tests/e2e/suite.go index 37aa3f8e..628e9bdc 100644 --- a/tests/e2e/suite.go +++ b/tests/e2e/suite.go @@ -56,7 +56,7 @@ var ( helmChart = flag.String("helmchart", "", "helm chart reference") operatorNS = flag.String("namespace", "kube-amd-network", "operator namespace") cfgName = flag.String("networkConfigName", "networkconfig-example", "NetworkConfig name") - driverVersion = flag.String("driverVersion", "1.117.1-a-42", "driver version") + driverVersion = flag.String("driverVersion", "1.117.5-a-147", "driver version") openshift = flag.Bool("openshift", false, "openshift deployment") simEnable = flag.Bool("simEnable", false, "simulate (no hardware)") ciEnv = flag.Bool("ciEnv", false, "CI environment")