Repository navigation
fix: allow RHEL host subscriptions with custom driver repo config #2890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ import ( | |
| "path" | ||
| "path/filepath" | ||
| "regexp" | ||
| "slices" | ||
| "sort" | ||
| "strconv" | ||
| "strings" | ||
|
|
@@ -3723,12 +3724,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) | ||
|
|
@@ -3737,8 +3741,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 | ||
|
|
@@ -3752,8 +3761,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" { | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| 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() | ||
|
|
@@ -3769,8 +3779,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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC this is to ensure if TransformDriver is called twice we don't add the same volumes twice. But I am not sure if this is practically possible with our code currently, We seem to call with an empty manifest always. Also if we want to address the idempotency issue with TransformDriver, shouldn't we do this for other volumes as well ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is defensive handling just in case a future change accidentally transforms the same object twice. This is not needed by the current reconcile code. We can keep the scope change minimal and do the defensive handling for all volumes if needed in a future change, so just keep this PR focused on the subscription fix.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think I would prefer that since right now it is not possible to hit this issue and the fix is partial right now |
||
|
|
||
| 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, | ||
|
|
@@ -3798,6 +3815,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 | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.