diff --git a/api/nvidia/v1/clusterpolicy_types.go b/api/nvidia/v1/clusterpolicy_types.go index 16d95220c5..0e787f25dc 100644 --- a/api/nvidia/v1/clusterpolicy_types.go +++ b/api/nvidia/v1/clusterpolicy_types.go @@ -1280,6 +1280,12 @@ type DriverRepoConfigSpec struct { // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="ConfigMap Name" // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:text" ConfigMapName string `json:"configMapName,omitempty"` + + // UseHostSubscription mounts the host RHEL subscription configuration with this custom repo configuration. + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="Use Host RHEL Subscription" + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:booleanSwitch" + UseHostSubscription bool `json:"useHostSubscription,omitempty"` } // DriverCertConfigSpec defines custom certificates configuration for NVIDIA Driver container diff --git a/api/nvidia/v1alpha1/nvidiadriver_types.go b/api/nvidia/v1alpha1/nvidiadriver_types.go index 70aa59b0ff..e365d16952 100644 --- a/api/nvidia/v1alpha1/nvidiadriver_types.go +++ b/api/nvidia/v1alpha1/nvidiadriver_types.go @@ -453,6 +453,12 @@ type DriverRepoConfigSpec struct { // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="ConfigMap Name" // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:text" Name string `json:"name,omitempty"` + + // UseHostSubscription mounts the host RHEL subscription configuration with this custom repo configuration. + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="Use Host RHEL Subscription" + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:booleanSwitch" + UseHostSubscription bool `json:"useHostSubscription,omitempty"` } // DriverLicensingConfigSpec defines licensing server configuration for NVIDIA Driver container diff --git a/bundle/manifests/gpu-operator-certified.clusterserviceversion.yaml b/bundle/manifests/gpu-operator-certified.clusterserviceversion.yaml index 126dd3a028..e7542af68e 100644 --- a/bundle/manifests/gpu-operator-certified.clusterserviceversion.yaml +++ b/bundle/manifests/gpu-operator-certified.clusterserviceversion.yaml @@ -346,6 +346,11 @@ spec: path: imagePullPolicy x-descriptors: - 'urn:alm:descriptor:com.tectonic.ui:imagePullPolicy' + - displayName: Use Host RHEL Subscription + description: Mount the host RHEL subscription configuration with this custom repo configuration. + path: repoConfig.useHostSubscription + x-descriptors: + - 'urn:alm:descriptor:com.tectonic.ui:booleanSwitch' - displayName: UseOpenKernelModules description: 'Enable use of open GPU kernel modules' path: useOpenKernelModules @@ -488,6 +493,11 @@ spec: path: driver.imagePullPolicy x-descriptors: - 'urn:alm:descriptor:com.tectonic.ui:imagePullPolicy' + - displayName: Use Host RHEL Subscription + description: Mount the host RHEL subscription configuration with this custom repo configuration. + path: driver.repoConfig.useHostSubscription + x-descriptors: + - 'urn:alm:descriptor:com.tectonic.ui:booleanSwitch' - displayName: UseOpenKernelModules description: 'Enable use of open GPU kernel modules' path: driver.useOpenKernelModules diff --git a/bundle/manifests/nvidia.com_clusterpolicies.yaml b/bundle/manifests/nvidia.com_clusterpolicies.yaml index e8d0be746c..7943dfc45a 100644 --- a/bundle/manifests/nvidia.com_clusterpolicies.yaml +++ b/bundle/manifests/nvidia.com_clusterpolicies.yaml @@ -1137,6 +1137,10 @@ spec: properties: configMapName: type: string + useHostSubscription: + description: UseHostSubscription mounts the host RHEL subscription + configuration with this custom repo configuration. + type: boolean type: object repository: description: NVIDIA Driver image repository diff --git a/bundle/manifests/nvidia.com_nvidiadrivers.yaml b/bundle/manifests/nvidia.com_nvidiadrivers.yaml index 7ad08f203c..6683e7404e 100644 --- a/bundle/manifests/nvidia.com_nvidiadrivers.yaml +++ b/bundle/manifests/nvidia.com_nvidiadrivers.yaml @@ -834,6 +834,10 @@ spec: properties: name: type: string + useHostSubscription: + description: UseHostSubscription mounts the host RHEL subscription + configuration with this custom repo configuration. + type: boolean type: object repository: description: NVIDIA Driver repository diff --git a/config/crd/bases/nvidia.com_clusterpolicies.yaml b/config/crd/bases/nvidia.com_clusterpolicies.yaml index e8d0be746c..7943dfc45a 100644 --- a/config/crd/bases/nvidia.com_clusterpolicies.yaml +++ b/config/crd/bases/nvidia.com_clusterpolicies.yaml @@ -1137,6 +1137,10 @@ spec: properties: configMapName: type: string + useHostSubscription: + description: UseHostSubscription mounts the host RHEL subscription + configuration with this custom repo configuration. + type: boolean type: object repository: description: NVIDIA Driver image repository diff --git a/config/crd/bases/nvidia.com_nvidiadrivers.yaml b/config/crd/bases/nvidia.com_nvidiadrivers.yaml index 7ad08f203c..6683e7404e 100644 --- a/config/crd/bases/nvidia.com_nvidiadrivers.yaml +++ b/config/crd/bases/nvidia.com_nvidiadrivers.yaml @@ -834,6 +834,10 @@ spec: properties: name: type: string + useHostSubscription: + description: UseHostSubscription mounts the host RHEL subscription + configuration with this custom repo configuration. + type: boolean type: object repository: description: NVIDIA Driver repository diff --git a/config/manifests/bases/gpu-operator.clusterserviceversion.yaml b/config/manifests/bases/gpu-operator.clusterserviceversion.yaml index 45a8e4ff6d..3fb2cf9482 100644 --- a/config/manifests/bases/gpu-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/gpu-operator.clusterserviceversion.yaml @@ -201,6 +201,11 @@ spec: path: driver.imagePullPolicy x-descriptors: - 'urn:alm:descriptor:com.tectonic.ui:imagePullPolicy' + - displayName: Use Host RHEL Subscription + description: Mount the host RHEL subscription configuration with this custom repo configuration. + path: driver.repoConfig.useHostSubscription + x-descriptors: + - 'urn:alm:descriptor:com.tectonic.ui:booleanSwitch' - description: Group Feature Discovery Plugin config displayName: Group Feature Discovery Plugin config path: gfd diff --git a/controllers/object_controls.go b/controllers/object_controls.go index c1bf59e4cf..1a36d64196 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -24,6 +24,7 @@ import ( "path" "path/filepath" "regexp" + "slices" "sort" "strconv" "strings" @@ -3720,12 +3721,15 @@ func transformDriverContainer(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicy if err != nil { return fmt.Errorf("ERROR: failed to create ConfigMap VolumeMounts for custom repo config: %v", err) } + removeConfigMapVolumeAndMounts(podSpec, config.Driver.RepoConfig.ConfigMapName) driverContainer.VolumeMounts = append(driverContainer.VolumeMounts, volumeMounts...) podSpec.Volumes = append(podSpec.Volumes, createConfigMapVolume(config.Driver.RepoConfig.ConfigMapName, itemsToInclude)) } // set any custom ssl key/certificate configuration provided if config.Driver.CertConfig != nil && config.Driver.CertConfig.Name != "" { + sharedRepoConfig := config.Driver.RepoConfig != nil && + config.Driver.RepoConfig.ConfigMapName == config.Driver.CertConfig.Name destinationDir, err := n.getCertConfigPath() if err != nil { return fmt.Errorf("ERROR: failed to get destination directory for custom cert config: %v", err) @@ -3734,8 +3738,13 @@ func transformDriverContainer(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicy if err != nil { return fmt.Errorf("ERROR: failed to create ConfigMap VolumeMounts for custom certs: %w", err) } + if !sharedRepoConfig { + removeConfigMapVolumeAndMounts(podSpec, config.Driver.CertConfig.Name) + } driverContainer.VolumeMounts = append(driverContainer.VolumeMounts, volumeMounts...) - podSpec.Volumes = append(podSpec.Volumes, createConfigMapVolume(config.Driver.CertConfig.Name, itemsToInclude)) + if !sharedRepoConfig { + podSpec.Volumes = append(podSpec.Volumes, createConfigMapVolume(config.Driver.CertConfig.Name, itemsToInclude)) + } } secretName := config.Driver.SecretEnv @@ -3749,8 +3758,9 @@ func transformDriverContainer(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicy // set up subscription entitlements for RHEL(using K8s with a non-CRIO runtime) and SLES if (osID == "rhel" && n.openshift == "" && n.runtime != gpuv1.CRIO) || osID == "sles" || osID == "sl-micro" { pathToVolumeSource := MountPathToVolumeSource{} - if config.Driver.RepoConfig != nil && config.Driver.RepoConfig.ConfigMapName != "" && osID == "rhel" { - n.logger.Info("Skipping host subscription mounts because repoConfig is enabled", "OS", osID) + if config.Driver.RepoConfig != nil && config.Driver.RepoConfig.ConfigMapName != "" && + !config.Driver.RepoConfig.UseHostSubscription && osID == "rhel" { + n.logger.Info("Skipping host subscription mounts because repoConfig is enabled and useHostSubscription is false", "OS", osID) } else { n.logger.Info("Mounting subscriptions into the driver container", "OS", osID) pathToVolumeSource, err = n.getSubscriptionPathsToVolumeSources() @@ -3766,8 +3776,15 @@ func transformDriverContainer(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicy } sort.Strings(mountPaths) - for num, mountPath := range mountPaths { - volMountSubscriptionName := fmt.Sprintf("subscription-config-%d", num) + removeSubscriptionMountsAndVolumes(driverContainer, podSpec) + + usedVolumeNames := make(map[string]struct{}, len(podSpec.Volumes)+len(mountPaths)) + for _, volume := range podSpec.Volumes { + usedVolumeNames[volume.Name] = struct{}{} + } + volumeIndex := 0 + for _, mountPath := range mountPaths { + volMountSubscriptionName := nextSubscriptionVolumeName(usedVolumeNames, &volumeIndex) volMountSubscription := corev1.VolumeMount{ Name: volMountSubscriptionName, @@ -3795,6 +3812,60 @@ func transformDriverContainer(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicy return nil } +// removeConfigMapVolumeAndMounts removes an operator-managed ConfigMap volume and its mounts before rebuilding it. +func removeConfigMapVolumeAndMounts(podSpec *corev1.PodSpec, configMapName string) { + configMapVolumeNames := map[string]struct{}{} + podSpec.Volumes = slices.DeleteFunc(podSpec.Volumes, func(volume corev1.Volume) bool { + if volume.Name != configMapName || volume.ConfigMap == nil || volume.ConfigMap.Name != configMapName { + return false + } + configMapVolumeNames[volume.Name] = struct{}{} + return true + }) + for containerIndex := range podSpec.Containers { + podSpec.Containers[containerIndex].VolumeMounts = slices.DeleteFunc(podSpec.Containers[containerIndex].VolumeMounts, func(volumeMount corev1.VolumeMount) bool { + _, found := configMapVolumeNames[volumeMount.Name] + return found + }) + } +} + +// nextSubscriptionVolumeName returns an unused subscription volume name and records it as used. +func nextSubscriptionVolumeName(usedVolumeNames map[string]struct{}, volumeIndex *int) string { + for { + volumeName := fmt.Sprintf("%s%d", consts.SubscriptionVolumeNamePrefix, *volumeIndex) + *volumeIndex += 1 + if _, found := usedVolumeNames[volumeName]; found { + continue + } + usedVolumeNames[volumeName] = struct{}{} + return volumeName + } +} + +// removeSubscriptionMountsAndVolumes removes host subscription volumes and their mounts before rebuilding them. +// It removes only volumes with the subscription prefix that are backed by hostPath, then removes only +// mounts that reference those volumes. ConfigMap-backed volumes with the same prefix are preserved. +func removeSubscriptionMountsAndVolumes(driverContainer *corev1.Container, podSpec *corev1.PodSpec) { + subscriptionVolumeNames := map[string]struct{}{} + podSpec.Volumes = slices.DeleteFunc(podSpec.Volumes, func(volume corev1.Volume) bool { + if !isHostSubscriptionVolume(volume) { + return false + } + subscriptionVolumeNames[volume.Name] = struct{}{} + return true + }) + driverContainer.VolumeMounts = slices.DeleteFunc(driverContainer.VolumeMounts, func(volumeMount corev1.VolumeMount) bool { + _, found := subscriptionVolumeNames[volumeMount.Name] + return found + }) +} + +// isHostSubscriptionVolume reports whether a volume is managed for host subscriptions. +func isHostSubscriptionVolume(volume corev1.Volume) bool { + return strings.HasPrefix(volume.Name, consts.SubscriptionVolumeNamePrefix) && volume.HostPath != nil +} + func createSecretEnvReference(ctx context.Context, ctrlClient client.Client, secretName string, namespace string, container *corev1.Container) error { envFrom := container.EnvFrom diff --git a/controllers/transforms_test.go b/controllers/transforms_test.go index 8931298a48..978bd9f731 100644 --- a/controllers/transforms_test.go +++ b/controllers/transforms_test.go @@ -19,7 +19,6 @@ package controllers import ( "path" "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -4688,18 +4687,25 @@ func TestTransformDriverSubscriptionMounts(t *testing.T) { Namespace: "test-ns", }, Data: map[string]string{ - "redhat.repo": "[test-repo]", + "custom.repo": "[test-repo]", }, } - mockClient := fake.NewFakeClient(repoConfigMap) - + certConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cert-config", Namespace: "test-ns"}, + Data: map[string]string{"ca.crt": "test certificate"}, + } testCases := []struct { description string osRelease string osTag string repoConfigEnabled bool + repoConfigName string + certConfigEnabled bool + useHostSubscription bool + transformTwice bool expectSubscriptionMounts bool expectedSubscriptionHostMap map[string]corev1.HostPathType + expectedSubscriptionMounts map[string]string }{ { description: "rhel with repo config skips host subscription mounts", @@ -4718,11 +4724,64 @@ func TestTransformDriverSubscriptionMounts(t *testing.T) { "/etc/yum.repos.d/redhat.repo": corev1.HostPathFile, "/etc/rhsm": corev1.HostPathDirectory, }, + expectedSubscriptionMounts: map[string]string{ + "/etc/pki/entitlement": "/run/secrets/etc-pki-entitlement", + "/etc/yum.repos.d/redhat.repo": "/run/secrets/redhat.repo", + "/etc/rhsm": "/run/secrets/rhsm", + }, + }, + { + description: "rhel with repo config and host subscription mounts host subscription paths", + osRelease: "rhel", + osTag: "rhel8.10", + repoConfigEnabled: true, + useHostSubscription: true, + certConfigEnabled: true, + transformTwice: true, + expectSubscriptionMounts: true, + expectedSubscriptionHostMap: map[string]corev1.HostPathType{ + "/etc/pki/entitlement": corev1.HostPathDirectory, + "/etc/yum.repos.d/redhat.repo": corev1.HostPathFile, + "/etc/rhsm": corev1.HostPathDirectory, + }, + expectedSubscriptionMounts: map[string]string{ + "/etc/pki/entitlement": "/run/secrets/etc-pki-entitlement", + "/etc/yum.repos.d/redhat.repo": "/run/secrets/redhat.repo", + "/etc/rhsm": "/run/secrets/rhsm", + }, + }, + { + description: "rhel with colliding repo config name and host subscription mounts uses unique volume names", + osRelease: "rhel", + osTag: "rhel8.10", + repoConfigEnabled: true, + repoConfigName: "subscription-config-0", + useHostSubscription: true, + transformTwice: true, + expectSubscriptionMounts: true, + expectedSubscriptionHostMap: map[string]corev1.HostPathType{ + "/etc/pki/entitlement": corev1.HostPathDirectory, + "/etc/yum.repos.d/redhat.repo": corev1.HostPathFile, + "/etc/rhsm": corev1.HostPathDirectory, + }, + expectedSubscriptionMounts: map[string]string{ + "/etc/pki/entitlement": "/run/secrets/etc-pki-entitlement", + "/etc/yum.repos.d/redhat.repo": "/run/secrets/redhat.repo", + "/etc/rhsm": "/run/secrets/rhsm", + }, }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { + repoConfigName := "test-repo-config" + if tc.repoConfigName != "" { + repoConfigName = tc.repoConfigName + } + configMap := repoConfigMap.DeepCopy() + configMap.Name = repoConfigName + certConfigName := certConfigMap.Name + mockClient := fake.NewFakeClient(configMap, certConfigMap) ds := NewDaemonset().WithContainer(corev1.Container{Name: "nvidia-driver-ctr"}). WithInitContainer(corev1.Container{Name: "k8s-driver-manager"}) cpSpec := &gpuv1.ClusterPolicySpec{ @@ -4740,7 +4799,13 @@ func TestTransformDriverSubscriptionMounts(t *testing.T) { }, } if tc.repoConfigEnabled { - cpSpec.Driver.RepoConfig = &gpuv1.DriverRepoConfigSpec{ConfigMapName: "test-repo-config"} + cpSpec.Driver.RepoConfig = &gpuv1.DriverRepoConfigSpec{ + ConfigMapName: repoConfigName, + UseHostSubscription: tc.useHostSubscription, + } + } + if tc.certConfigEnabled { + cpSpec.Driver.CertConfig = &gpuv1.DriverCertConfigSpec{Name: certConfigName} } err := TransformDriver(ds.DaemonSet, cpSpec, ClusterPolicyController{ @@ -4752,22 +4817,206 @@ func TestTransformDriverSubscriptionMounts(t *testing.T) { gpuNodeOSTag: tc.osTag, }) require.NoError(t, err) + if tc.transformTwice { + err = TransformDriver(ds.DaemonSet, cpSpec, ClusterPolicyController{ + client: mockClient, + runtime: gpuv1.Containerd, + operatorNamespace: "test-ns", + logger: ctrl.Log.WithName("test"), + gpuNodeOSRelease: tc.osRelease, + gpuNodeOSTag: tc.osTag, + }) + require.NoError(t, err) + } driverContainer := findContainerByName(ds.Spec.Template.Spec.Containers, "nvidia-driver-ctr") require.NotNil(t, driverContainer) + if tc.repoConfigEnabled { + assertCustomRepoConfigForTransform(t, ds.Spec.Template.Spec.Volumes, driverContainer.VolumeMounts, repoConfigName) + } + if tc.certConfigEnabled { + assertCustomCertConfigForTransform(t, ds.Spec.Template.Spec.Volumes, driverContainer.VolumeMounts, certConfigName) + } assertSubscriptionHostPathVolumesForTransform(t, ds.Spec.Template.Spec.Volumes, tc.expectedSubscriptionHostMap) - assert.Equal(t, tc.expectSubscriptionMounts, hasSubscriptionVolumeMountForTransform(driverContainer.VolumeMounts)) + assertSubscriptionVolumeMountPathsForTransform(t, ds.Spec.Template.Spec.Volumes, driverContainer.VolumeMounts, tc.expectedSubscriptionMounts) + assert.Equal(t, tc.expectSubscriptionMounts, hasSubscriptionVolumeMountForTransform(ds.Spec.Template.Spec.Volumes, driverContainer.VolumeMounts)) + assert.Equal(t, len(tc.expectedSubscriptionHostMap), subscriptionVolumeMountCountForTransform(ds.Spec.Template.Spec.Volumes, driverContainer.VolumeMounts)) + assert.Equal(t, len(tc.expectedSubscriptionHostMap), subscriptionVolumeCountForTransform(ds.Spec.Template.Spec.Volumes)) + assertUniqueVolumeNamesForTransform(t, ds.Spec.Template.Spec.Volumes) + assertUniqueVolumeMountPathsForTransform(t, driverContainer.VolumeMounts) }) } } -func hasSubscriptionVolumeMountForTransform(volumeMounts []corev1.VolumeMount) bool { +// assertCustomCertConfigForTransform verifies the custom certificate ConfigMap and its mount. +func assertCustomCertConfigForTransform(t *testing.T, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, configMapName string) { + t.Helper() + + var volume *corev1.Volume + for index := range volumes { + if volumes[index].Name == configMapName { + volume = &volumes[index] + break + } + } + require.NotNil(t, volume, "missing custom certificate ConfigMap volume") + require.NotNil(t, volume.ConfigMap) + assert.Equal(t, configMapName, volume.ConfigMap.Name) + + mountPath := "/etc/pki/ca-trust/extracted/pem/ca.crt" + configMapMounts := 0 + for _, volumeMount := range volumeMounts { + if volumeMount.Name == configMapName && volumeMount.MountPath == mountPath { + configMapMounts++ + } + } + assert.Equal(t, 1, configMapMounts, "expected one custom certificate volume mount %q at %q", configMapName, mountPath) +} + +// assertCustomRepoConfigForTransform verifies the custom repository ConfigMap and its mount. +func assertCustomRepoConfigForTransform(t *testing.T, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, configMapName string) { + t.Helper() + + const ( + mountPath = "/etc/yum.repos.d/custom.repo" + ) + + var configMapVolume *corev1.Volume + for index := range volumes { + if volumes[index].Name == configMapName { + configMapVolume = &volumes[index] + break + } + } + require.NotNil(t, configMapVolume, "missing custom repository ConfigMap volume") + require.NotNil(t, configMapVolume.ConfigMap) + assert.Equal(t, configMapName, configMapVolume.ConfigMap.Name) + + configMapMounts := 0 + for _, volumeMount := range volumeMounts { + if volumeMount.Name == configMapName && volumeMount.MountPath == mountPath { + configMapMounts++ + } + } + assert.Equal(t, 1, configMapMounts, "expected one custom repository volume mount %q at %q", configMapName, mountPath) +} + +// assertUniqueVolumeNamesForTransform verifies that each volume has a distinct name. +func assertUniqueVolumeNamesForTransform(t *testing.T, volumes []corev1.Volume) { + t.Helper() + + volumeNames := map[string]struct{}{} + for _, volume := range volumes { + _, found := volumeNames[volume.Name] + assert.Falsef(t, found, "duplicate volume name %q", volume.Name) + volumeNames[volume.Name] = struct{}{} + } +} + +// assertUniqueVolumeMountPathsForTransform verifies that each volume mount has a distinct destination path. +func assertUniqueVolumeMountPathsForTransform(t *testing.T, volumeMounts []corev1.VolumeMount) { + t.Helper() + + mountPaths := map[string]struct{}{} + for _, volumeMount := range volumeMounts { + _, found := mountPaths[volumeMount.MountPath] + assert.Falsef(t, found, "duplicate volume mount path %q", volumeMount.MountPath) + mountPaths[volumeMount.MountPath] = struct{}{} + } +} + +func TestRemoveSubscriptionMountsAndVolumesPreservesConfigMapVolume(t *testing.T) { + const configMapVolumeName = "subscription-config-custom" + driverContainer := &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{ + {Name: configMapVolumeName, MountPath: "/etc/yum.repos.d/custom.repo"}, + {Name: "subscription-config-0", MountPath: "/run/secrets/redhat.repo"}, + }, + } + podSpec := &corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: configMapVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{}, + }, + }, + { + Name: "subscription-config-0", + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{}, + }, + }, + }, + } + + removeSubscriptionMountsAndVolumes(driverContainer, podSpec) + + require.Len(t, driverContainer.VolumeMounts, 1) + assert.Equal(t, configMapVolumeName, driverContainer.VolumeMounts[0].Name) + require.Len(t, podSpec.Volumes, 1) + assert.Equal(t, configMapVolumeName, podSpec.Volumes[0].Name) +} + +func TestRemoveSubscriptionMountsAndVolumesWithNoResources(t *testing.T) { + driverContainer := &corev1.Container{} + podSpec := &corev1.PodSpec{} + + removeSubscriptionMountsAndVolumes(driverContainer, podSpec) + + assert.Empty(t, driverContainer.VolumeMounts) + assert.Empty(t, podSpec.Volumes) +} + +// hasSubscriptionVolumeMountForTransform reports whether any host subscription volume is mounted. +func hasSubscriptionVolumeMountForTransform(volumes []corev1.Volume, volumeMounts []corev1.VolumeMount) bool { + return subscriptionVolumeMountCountForTransform(volumes, volumeMounts) > 0 +} + +// subscriptionVolumeMountCountForTransform counts mounts managed by the subscription configuration. +func subscriptionVolumeMountCountForTransform(volumes []corev1.Volume, volumeMounts []corev1.VolumeMount) int { + subscriptionVolumeNames := subscriptionVolumeNamesForTransform(volumes) + count := 0 for _, volumeMount := range volumeMounts { - if strings.HasPrefix(volumeMount.Name, "subscription-config-") { - return true + if _, found := subscriptionVolumeNames[volumeMount.Name]; found { + count++ + } + } + return count +} + +// subscriptionVolumeCountForTransform counts volumes managed by the subscription configuration. +func subscriptionVolumeCountForTransform(volumes []corev1.Volume) int { + return len(subscriptionVolumeNamesForTransform(volumes)) +} + +// subscriptionVolumeNamesForTransform returns names of host volumes managed for subscriptions. +func subscriptionVolumeNamesForTransform(volumes []corev1.Volume) map[string]struct{} { + names := map[string]struct{}{} + for _, volume := range volumes { + if isHostSubscriptionVolume(volume) { + names[volume.Name] = struct{}{} + } + } + return names +} + +// assertSubscriptionVolumeMountPathsForTransform verifies that subscription volumes have the expected mount paths. +func assertSubscriptionVolumeMountPathsForTransform(t *testing.T, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, expected map[string]string) { + t.Helper() + + mountsByName := map[string]corev1.VolumeMount{} + for _, volumeMount := range volumeMounts { + mountsByName[volumeMount.Name] = volumeMount + } + for _, volume := range volumes { + if !isHostSubscriptionVolume(volume) { + continue } + volumeMount, found := mountsByName[volume.Name] + require.True(t, found, "missing volume mount for %q", volume.Name) + assert.Equal(t, expected[volume.HostPath.Path], volumeMount.MountPath) } - return false } func assertSubscriptionHostPathVolumesForTransform(t *testing.T, volumes []corev1.Volume, expected map[string]corev1.HostPathType) { @@ -4779,10 +5028,9 @@ func assertSubscriptionHostPathVolumesForTransform(t *testing.T, volumes []corev actual := map[string]corev1.HostPathType{} for _, volume := range volumes { - if !strings.HasPrefix(volume.Name, "subscription-config-") { + if !isHostSubscriptionVolume(volume) { continue } - require.NotNil(t, volume.HostPath) require.NotNil(t, volume.HostPath.Type) actual[volume.HostPath.Path] = *volume.HostPath.Type } diff --git a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml index e8d0be746c..7943dfc45a 100644 --- a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml +++ b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml @@ -1137,6 +1137,10 @@ spec: properties: configMapName: type: string + useHostSubscription: + description: UseHostSubscription mounts the host RHEL subscription + configuration with this custom repo configuration. + type: boolean type: object repository: description: NVIDIA Driver image repository diff --git a/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml b/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml index 7ad08f203c..6683e7404e 100644 --- a/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml +++ b/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml @@ -834,6 +834,10 @@ spec: properties: name: type: string + useHostSubscription: + description: UseHostSubscription mounts the host RHEL subscription + configuration with this custom repo configuration. + type: boolean type: object repository: description: NVIDIA Driver repository diff --git a/deployments/gpu-operator/templates/nvidiadriver.yaml b/deployments/gpu-operator/templates/nvidiadriver.yaml index 262d551dbe..8c9f691c1e 100644 --- a/deployments/gpu-operator/templates/nvidiadriver.yaml +++ b/deployments/gpu-operator/templates/nvidiadriver.yaml @@ -65,6 +65,9 @@ spec: {{- if .Values.driver.repoConfig.configMapName }} repoConfig: name: {{ .Values.driver.repoConfig.configMapName }} + {{- if .Values.driver.repoConfig.useHostSubscription }} + useHostSubscription: true + {{- end }} {{- end }} {{- if .Values.driver.certConfig.name }} certConfig: diff --git a/deployments/gpu-operator/values.yaml b/deployments/gpu-operator/values.yaml index 1bce3fe866..e46f56e7f4 100644 --- a/deployments/gpu-operator/values.yaml +++ b/deployments/gpu-operator/values.yaml @@ -223,6 +223,8 @@ driver: # Private mirror repository configuration repoConfig: configMapName: "" + # Mount the host RHEL subscription configuration with this repo configuration. + useHostSubscription: false # custom ssl key/certificate configuration certConfig: name: "" diff --git a/internal/consts/consts.go b/internal/consts/consts.go index f28c507942..5d3a1f1bbf 100644 --- a/internal/consts/consts.go +++ b/internal/consts/consts.go @@ -69,6 +69,8 @@ const ( DefaultNVIDIADriverName = "default" // NVIDIADriverOwnerLabel is an operator-managed node label used to route each GPU node to one NVIDIADriver. NVIDIADriverOwnerLabel = "nvidia.com/gpu-operator.driver.owner" + // SubscriptionVolumeNamePrefix identifies volumes and mounts managed for host subscriptions. + SubscriptionVolumeNamePrefix = "subscription-config-" // MinimumGDSVersionForOpenRM indicates the minimum GDS version that is supported only with OpenRM driver MinimumGDSVersionForOpenRM = "v2.17.5" diff --git a/internal/state/driver_cleanup_test.go b/internal/state/driver_cleanup_test.go index 61a41b8620..7338d592c1 100644 --- a/internal/state/driver_cleanup_test.go +++ b/internal/state/driver_cleanup_test.go @@ -324,7 +324,7 @@ func TestGetDriverAdditionalConfigsSLESSubscription(t *testing.T) { nodePool{osRelease: "sles", osVersion: "15.5"}, ) require.NoError(t, err) - assert.True(t, hasSubscriptionVolumeMount(configs.VolumeMounts), "expected SLES subscription mounts") + assert.True(t, hasSubscriptionVolumeMount(configs.Volumes, configs.VolumeMounts), "expected SLES subscription mounts") } func TestGetDriverAdditionalConfigsUnsupportedCertOS(t *testing.T) { diff --git a/internal/state/driver_test.go b/internal/state/driver_test.go index 9a06c9718c..a15c94352d 100644 --- a/internal/state/driver_test.go +++ b/internal/state/driver_test.go @@ -118,15 +118,34 @@ func findContainerByName(containers []corev1.Container, name string) *corev1.Con return nil } -func hasSubscriptionVolumeMount(volumeMounts []corev1.VolumeMount) bool { +// hasSubscriptionVolumeMount reports whether any host subscription volume is mounted. +func hasSubscriptionVolumeMount(volumes []corev1.Volume, volumeMounts []corev1.VolumeMount) bool { + subscriptionVolumeNames := map[string]struct{}{} + for _, volume := range volumes { + if isHostSubscriptionVolume(volume) { + subscriptionVolumeNames[volume.Name] = struct{}{} + } + } for _, volumeMount := range volumeMounts { - if strings.HasPrefix(volumeMount.Name, "subscription-config-") { + if _, found := subscriptionVolumeNames[volumeMount.Name]; found { return true } } return false } +// assertUniqueVolumeMountPaths verifies that each volume mount has a distinct destination path. +func assertUniqueVolumeMountPaths(t *testing.T, volumeMounts []corev1.VolumeMount) { + t.Helper() + + mountPaths := map[string]struct{}{} + for _, volumeMount := range volumeMounts { + _, found := mountPaths[volumeMount.MountPath] + assert.Falsef(t, found, "duplicate volume mount path %q", volumeMount.MountPath) + mountPaths[volumeMount.MountPath] = struct{}{} + } +} + func assertSubscriptionHostPathVolumes(t *testing.T, volumes []corev1.Volume, expected map[string]corev1.HostPathType) { t.Helper() @@ -136,10 +155,9 @@ func assertSubscriptionHostPathVolumes(t *testing.T, volumes []corev1.Volume, ex actual := map[string]corev1.HostPathType{} for _, volume := range volumes { - if !strings.HasPrefix(volume.Name, "subscription-config-") { + if !isHostSubscriptionVolume(volume) { continue } - require.NotNil(t, volume.HostPath) require.NotNil(t, volume.HostPath.Type) actual[volume.HostPath.Path] = *volume.HostPath.Type } @@ -628,7 +646,7 @@ func TestDriverAdditionalConfigsSubscriptionMounts(t *testing.T) { Namespace: "test-ns", }, Data: map[string]string{ - "redhat.repo": "[test-repo]", + "custom.repo": "[test-repo]", }, } @@ -636,6 +654,9 @@ func TestDriverAdditionalConfigsSubscriptionMounts(t *testing.T) { description string osRelease string repoConfigEnabled bool + repoConfigName string + kernelConfigName string + useHostSubscription bool expectSubscriptionMounts bool expectedSubscriptionHostMap map[string]corev1.HostPathType }{ @@ -655,19 +676,69 @@ func TestDriverAdditionalConfigsSubscriptionMounts(t *testing.T) { "/etc/rhsm": corev1.HostPathDirectory, }, }, + { + description: "rhel with repo config and host subscription mounts host subscription paths", + osRelease: "rhel", + repoConfigEnabled: true, + useHostSubscription: true, + expectSubscriptionMounts: true, + expectedSubscriptionHostMap: map[string]corev1.HostPathType{ + "/etc/pki/entitlement": corev1.HostPathDirectory, + "/etc/yum.repos.d/redhat.repo": corev1.HostPathFile, + "/etc/rhsm": corev1.HostPathDirectory, + }, + }, + { + description: "rhel with colliding kernel config name uses unique subscription volume names", + osRelease: "rhel", + kernelConfigName: "subscription-config-0", + expectSubscriptionMounts: true, + expectedSubscriptionHostMap: map[string]corev1.HostPathType{ + "/etc/pki/entitlement": corev1.HostPathDirectory, + "/etc/yum.repos.d/redhat.repo": corev1.HostPathFile, + "/etc/rhsm": corev1.HostPathDirectory, + }, + }, + { + description: "rhel with colliding repo config name uses unique subscription volume names", + osRelease: "rhel", + repoConfigEnabled: true, + repoConfigName: "subscription-config-0", + useHostSubscription: true, + expectSubscriptionMounts: true, + expectedSubscriptionHostMap: map[string]corev1.HostPathType{ + "/etc/pki/entitlement": corev1.HostPathDirectory, + "/etc/yum.repos.d/redhat.repo": corev1.HostPathFile, + "/etc/rhsm": corev1.HostPathDirectory, + }, + }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { + repoConfigName := "test-repo-config" + if tc.repoConfigName != "" { + repoConfigName = tc.repoConfigName + } + configMap := repoConfigMap.DeepCopy() + configMap.Name = repoConfigName + kernelConfigMap := repoConfigMap.DeepCopy() + kernelConfigMap.Name = tc.kernelConfigName stateDriver := &stateDriver{ stateSkel: stateSkel{ - client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(repoConfigMap).Build(), + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(configMap, kernelConfigMap).Build(), namespace: "test-ns", }, } driver := &nvidiav1alpha1.NVIDIADriver{} if tc.repoConfigEnabled { - driver.Spec.RepoConfig = &nvidiav1alpha1.DriverRepoConfigSpec{Name: "test-repo-config"} + driver.Spec.RepoConfig = &nvidiav1alpha1.DriverRepoConfigSpec{ + Name: repoConfigName, + UseHostSubscription: tc.useHostSubscription, + } + } + if tc.kernelConfigName != "" { + driver.Spec.KernelModuleConfig = &nvidiav1alpha1.KernelModuleConfigSpec{Name: tc.kernelConfigName} } configs, err := stateDriver.getDriverAdditionalConfigs( @@ -678,12 +749,77 @@ func TestDriverAdditionalConfigsSubscriptionMounts(t *testing.T) { ) require.NoError(t, err) + if tc.repoConfigEnabled && tc.useHostSubscription { + assertCustomRepoConfig(t, configs.Volumes, configs.VolumeMounts, repoConfigName) + } assertSubscriptionHostPathVolumes(t, configs.Volumes, tc.expectedSubscriptionHostMap) - assert.Equal(t, tc.expectSubscriptionMounts, hasSubscriptionVolumeMount(configs.VolumeMounts)) + assert.Equal(t, tc.expectSubscriptionMounts, hasSubscriptionVolumeMount(configs.Volumes, configs.VolumeMounts)) + assertUniqueVolumeMountPaths(t, configs.VolumeMounts) + assertUniqueVolumeNames(t, configs.Volumes) }) } } +func TestDriverAdditionalConfigsSharedRepoAndCertConfig(t *testing.T) { + const name = "shared-config" + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test-ns"}, Data: map[string]string{"custom.repo": "[test]", "ca.crt": "cert"}} + s := &stateDriver{stateSkel: stateSkel{client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(cm).Build(), namespace: "test-ns"}} + driver := &nvidiav1alpha1.NVIDIADriver{Spec: nvidiav1alpha1.NVIDIADriverSpec{ + RepoConfig: &nvidiav1alpha1.DriverRepoConfigSpec{Name: name, UseHostSubscription: true}, + CertConfig: &nvidiav1alpha1.DriverCertConfigSpec{Name: name}, + }} + configs, err := s.getDriverAdditionalConfigs(context.Background(), driver, testClusterInfo{runtime: consts.Containerd}, nodePool{osRelease: "rhel", osVersion: "rhel"}) + require.NoError(t, err) + volume := findVolumeByName(configs.Volumes, name) + require.NotNil(t, volume) + require.NotNil(t, volume.ConfigMap) + assert.Contains(t, volume.ConfigMap.Items, corev1.KeyToPath{Key: "custom.repo", Path: "custom.repo"}) + assert.Contains(t, volume.ConfigMap.Items, corev1.KeyToPath{Key: "ca.crt", Path: "ca.crt"}) + mountPaths := map[string]struct{}{} + for _, volumeMount := range configs.VolumeMounts { + if volumeMount.Name == name { + mountPaths[volumeMount.MountPath] = struct{}{} + } + } + assert.Contains(t, mountPaths, "/etc/yum.repos.d/custom.repo") + assert.Contains(t, mountPaths, "/etc/pki/ca-trust/extracted/pem/ca.crt") + assertUniqueVolumeNames(t, configs.Volumes) + assertUniqueVolumeMountPaths(t, configs.VolumeMounts) +} + +// assertCustomRepoConfig verifies the custom repository ConfigMap volume and its mount. +func assertCustomRepoConfig(t *testing.T, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, configMapName string) { + t.Helper() + + const ( + mountPath = "/etc/yum.repos.d/custom.repo" + fileName = "custom.repo" + ) + + volume := findVolumeByName(volumes, configMapName) + require.NotNil(t, volume, "missing custom repository ConfigMap volume") + require.NotNil(t, volume.ConfigMap) + assert.Equal(t, configMapName, volume.ConfigMap.Name) + assert.Contains(t, volume.ConfigMap.Items, corev1.KeyToPath{Key: fileName, Path: fileName}) + + volumeMount := findVolumeMountByName(volumeMounts, configMapName) + require.NotNil(t, volumeMount, "missing custom repository volume mount") + assert.Equal(t, mountPath, volumeMount.MountPath) + assert.Equal(t, fileName, volumeMount.SubPath) +} + +// assertUniqueVolumeNames verifies that each volume has a distinct name. +func assertUniqueVolumeNames(t *testing.T, volumes []corev1.Volume) { + t.Helper() + + volumeNames := map[string]struct{}{} + for _, volume := range volumes { + _, found := volumeNames[volume.Name] + assert.Falsef(t, found, "duplicate volume name %q", volume.Name) + volumeNames[volume.Name] = struct{}{} + } +} + func TestDriverPrecompiledLibModules(t *testing.T) { const ( libModulesVolumeName = "lib-modules" diff --git a/internal/state/driver_volumes.go b/internal/state/driver_volumes.go index 34f60ea609..df99b7ed1b 100644 --- a/internal/state/driver_volumes.go +++ b/internal/state/driver_volumes.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "sort" + "strings" corev1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" @@ -159,6 +160,7 @@ func (s *stateDriver) getDriverAdditionalConfigs(ctx context.Context, cr *v1alph // set any custom ssl key/certificate configuration provided if cr.Spec.IsCertConfigEnabled() { + sharedRepoConfig := cr.Spec.IsRepoConfigEnabled() && cr.Spec.RepoConfig.Name == cr.Spec.CertConfig.Name destinationDir, err := getCertConfigPath(pool.osRelease) if err != nil { return nil, fmt.Errorf("ERROR: failed to get destination directory for custom repo config: %w", err) @@ -169,7 +171,9 @@ func (s *stateDriver) getDriverAdditionalConfigs(ctx context.Context, cr *v1alph return nil, fmt.Errorf("ERROR: failed to create ConfigMap VolumeMounts for custom certs: %w", err) } additionalCfgs.VolumeMounts = append(additionalCfgs.VolumeMounts, volumeMounts...) - additionalCfgs.Volumes = append(additionalCfgs.Volumes, createConfigMapVolume(cr.Spec.CertConfig.Name, itemsToInclude)) + if !sharedRepoConfig { + additionalCfgs.Volumes = append(additionalCfgs.Volumes, createConfigMapVolume(cr.Spec.CertConfig.Name, itemsToInclude)) + } } runtime, err := info.GetContainerRuntime() @@ -189,8 +193,8 @@ func (s *stateDriver) getDriverAdditionalConfigs(ctx context.Context, cr *v1alph // mounting host RHSM paths (/etc/pki/entitlement, redhat.repo, /etc/rhsm): they may // be missing or not directories on minimal nodes, and are not needed when packages // come only from the mounted repo ConfigMap. - if cr.Spec.IsRepoConfigEnabled() && pool.osRelease == "rhel" { - logger.Info("Skipping host subscription mounts because repoConfig is enabled", "OS", pool.osVersion) + if cr.Spec.IsRepoConfigEnabled() && !cr.Spec.RepoConfig.UseHostSubscription && pool.osRelease == "rhel" { + logger.Info("Skipping host subscription mounts because repoConfig is enabled and useHostSubscription is false", "OS", pool.osVersion) } else { logger.Info("Mounting subscriptions into the driver container", "OS", pool.osVersion) pathToVolumeSource, err = getSubscriptionPathsToVolumeSources(pool.osRelease) @@ -206,8 +210,16 @@ func (s *stateDriver) getDriverAdditionalConfigs(ctx context.Context, cr *v1alph } sort.Strings(mountPaths) - for num, mountPath := range mountPaths { - volMountSubscriptionName := fmt.Sprintf("subscription-config-%d", num) + usedVolumeNames := make(map[string]struct{}, len(additionalCfgs.Volumes)+len(mountPaths)) + for _, volume := range additionalCfgs.Volumes { + usedVolumeNames[volume.Name] = struct{}{} + } + if cr.Spec.IsKernelModuleConfigEnabled() { + usedVolumeNames[cr.Spec.KernelModuleConfig.Name] = struct{}{} + } + volumeIndex := 0 + for _, mountPath := range mountPaths { + volMountSubscriptionName := nextSubscriptionVolumeName(usedVolumeNames, &volumeIndex) volMountSubscription := corev1.VolumeMount{ Name: volMountSubscriptionName, @@ -326,6 +338,24 @@ func (s *stateDriver) getDriverAdditionalConfigs(ctx context.Context, cr *v1alph return additionalCfgs, nil } +// nextSubscriptionVolumeName returns an unused subscription volume name and records it as used. +func nextSubscriptionVolumeName(usedVolumeNames map[string]struct{}, volumeIndex *int) string { + for { + volumeName := fmt.Sprintf("%s%d", consts.SubscriptionVolumeNamePrefix, *volumeIndex) + *volumeIndex += 1 + if _, found := usedVolumeNames[volumeName]; found { + continue + } + usedVolumeNames[volumeName] = struct{}{} + return volumeName + } +} + +// isHostSubscriptionVolume reports whether a volume is managed for host subscriptions. +func isHostSubscriptionVolume(volume corev1.Volume) bool { + return strings.HasPrefix(volume.Name, consts.SubscriptionVolumeNamePrefix) && volume.HostPath != nil +} + // getRepoConfigPath returns the standard OS specific path for repository configuration files func getRepoConfigPath(os string) (string, error) { if path, ok := RepoConfigPathMap[os]; ok {