diff --git a/pkg/provision/storage/commonStorage.go b/pkg/provision/storage/commonStorage.go index 8c1e98427..c95c93bf3 100644 --- a/pkg/provision/storage/commonStorage.go +++ b/pkg/provision/storage/commonStorage.go @@ -30,9 +30,6 @@ import ( "k8s.io/apimachinery/pkg/types" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/pkg/constants" - devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" - "github.com/devfile/devworkspace-operator/pkg/library/overrides" ) // The CommonStorageProvisioner provisions one PVC per namespace and configures all volumes in a workspace @@ -139,76 +136,8 @@ func (p *CommonStorageProvisioner) rewriteContainerVolumeMounts( workspace *dw.DevWorkspaceTemplateSpec, restrictedFields []string, ) error { - devfileVolumes := map[string]dw.VolumeComponent{} - - // Construct map of volume name -> volume Component - for _, component := range workspace.Components { - if component.Volume != nil { - if _, exists := devfileVolumes[component.Name]; exists { - return fmt.Errorf("volume component '%s' is defined multiple times", component.Name) - } - devfileVolumes[component.Name] = *component.Volume - } - } - - // Containers in podAdditions may reference e.g. automounted volumes in their volumeMounts, and this is not an error - additionalVolumes := map[string]bool{} - for _, additionalVolume := range podAdditions.Volumes { - additionalVolumes[additionalVolume.Name] = true - } - - // Containers in podAdditions may reference volumes defined in pod overrides, and this is not an error - overridesVolumes, err := overrides.GetVolumesFromOverrides(workspace, restrictedFields) - if err != nil { - return err - } - for volumeName, present := range overridesVolumes { - additionalVolumes[volumeName] = present - } - - // Add implicit projects volume to support mountSources, if needed - if _, exists := devfileVolumes[devfileConstants.ProjectsVolumeName]; !exists { - projectsVolume := dw.VolumeComponent{} - projectsVolume.Size = constants.PVCStorageSize - devfileVolumes[devfileConstants.ProjectsVolumeName] = projectsVolume - } - - // TODO: What should we do when a volume isn't explicitly defined? - rewriteVolumeMounts := func(containers []corev1.Container) error { - for cIdx, container := range containers { - for vmIdx, vm := range container.VolumeMounts { - volume, ok := devfileVolumes[vm.Name] - if !ok { - // Volume is defined outside of the devfile - if additionalVolumes[vm.Name] { - continue - } - // Should never happen as flattened Devfile is validated. - return fmt.Errorf("container '%s' references undefined volume '%s'", container.Name, vm.Name) - } - if !isEphemeral(&volume) { - containers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name) - containers[cIdx].VolumeMounts[vmIdx].Name = pvcName - } - } - } - return nil - } - if err := rewriteVolumeMounts(podAdditions.Containers); err != nil { - return err - } - if err := rewriteVolumeMounts(podAdditions.InitContainers); err != nil { - return err - } - - podAdditions.Volumes = append(podAdditions.Volumes, corev1.Volume{ - Name: pvcName, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: pvcName, - }, - }, - }) - - return nil + return rewriteContainerVolumeMounts(workspaceId, pvcName, podAdditions, workspace, restrictedFields, + func(workspaceId, volumeName string) string { + return fmt.Sprintf("%s/%s", workspaceId, volumeName) + }) } diff --git a/pkg/provision/storage/perWorkspaceStorage.go b/pkg/provision/storage/perWorkspaceStorage.go index bde796b68..477416498 100644 --- a/pkg/provision/storage/perWorkspaceStorage.go +++ b/pkg/provision/storage/perWorkspaceStorage.go @@ -24,8 +24,6 @@ import ( "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/dwerrors" - devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" - "github.com/devfile/devworkspace-operator/pkg/library/overrides" "github.com/devfile/devworkspace-operator/pkg/library/overrides/restrictions" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" "github.com/devfile/devworkspace-operator/pkg/provision/sync" @@ -104,78 +102,10 @@ func (p *PerWorkspaceStorageProvisioner) rewriteContainerVolumeMounts( workspace *dw.DevWorkspaceTemplateSpec, restrictedFields []string, ) error { - devfileVolumes := map[string]dw.VolumeComponent{} - - // Construct map of volume name -> volume Component - for _, component := range workspace.Components { - if component.Volume != nil { - if _, exists := devfileVolumes[component.Name]; exists { - return fmt.Errorf("volume component '%s' is defined multiple times", component.Name) - } - devfileVolumes[component.Name] = *component.Volume - } - } - - // Containers in podAdditions may reference e.g. automounted volumes in their volumeMounts, and this is not an error - additionalVolumes := map[string]bool{} - for _, additionalVolume := range podAdditions.Volumes { - additionalVolumes[additionalVolume.Name] = true - } - - // Containers in podAdditions may reference volumes defined in pod overrides, and this is not an error - overridesVolumes, err := overrides.GetVolumesFromOverrides(workspace, restrictedFields) - if err != nil { - return err - } - for volumeName, present := range overridesVolumes { - additionalVolumes[volumeName] = present - } - - // Add implicit projects volume to support mountSources, if needed - if _, exists := devfileVolumes[devfileConstants.ProjectsVolumeName]; !exists { - projectsVolume := dw.VolumeComponent{} - projectsVolume.Size = constants.PVCStorageSize - devfileVolumes[devfileConstants.ProjectsVolumeName] = projectsVolume - } - - // TODO: What should we do when a volume isn't explicitly defined? - rewriteVolumeMounts := func(containers []corev1.Container) error { - for cIdx, container := range containers { - for vmIdx, vm := range container.VolumeMounts { - volume, ok := devfileVolumes[vm.Name] - if !ok { - // Volume is defined outside of the devfile - if additionalVolumes[vm.Name] { - continue - } - // Should never happen as flattened Devfile is validated. - return fmt.Errorf("container '%s' references undefined volume '%s'", container.Name, vm.Name) - } - if !isEphemeral(&volume) { - containers[cIdx].VolumeMounts[vmIdx].SubPath = vm.Name - containers[cIdx].VolumeMounts[vmIdx].Name = pvcName - } - } - } - return nil - } - if err := rewriteVolumeMounts(podAdditions.Containers); err != nil { - return err - } - if err := rewriteVolumeMounts(podAdditions.InitContainers); err != nil { - return err - } - - podAdditions.Volumes = append(podAdditions.Volumes, corev1.Volume{ - Name: pvcName, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: pvcName, - }, - }, - }) - - return nil + return rewriteContainerVolumeMounts(workspaceId, pvcName, podAdditions, workspace, restrictedFields, + func(_, volumeName string) string { + return volumeName + }) } func getPVCSize(workspace *common.DevWorkspaceWithConfig, namespacedConfig *nsconfig.NamespacedConfig) (*resource.Quantity, error) { diff --git a/pkg/provision/storage/shared.go b/pkg/provision/storage/shared.go index 0982fb94c..90903dfa8 100644 --- a/pkg/provision/storage/shared.go +++ b/pkg/provision/storage/shared.go @@ -36,6 +36,7 @@ import ( "github.com/devfile/devworkspace-operator/pkg/constants" devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" containerlib "github.com/devfile/devworkspace-operator/pkg/library/container" + "github.com/devfile/devworkspace-operator/pkg/library/overrides" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" ) @@ -338,3 +339,91 @@ func checkPVCTerminating(name, namespace string, api sync.ClusterAPI) (bool, err } return pvc.DeletionTimestamp != nil, nil } + +// rewriteContainerVolumeMounts rewrites the VolumeMounts in a set of PodAdditions so that each non-ephemeral +// volume mount becomes a subpath into the provided PVC. The subpath for each mount is computed by subPathFunc, +// allowing callers to implement the different PVC strategies (e.g. workspace-id-prefixed subpaths for the common +// strategy, and bare volume-name subpaths for the per-workspace strategy). +// +// It also adds the PVC-backed k8s Volume to podAdditions to accommodate the rewritten VolumeMounts. +// Must be called at most once per PodAdditions: the PVC-backed Volume is appended unconditionally. +func rewriteContainerVolumeMounts( + workspaceId, pvcName string, + podAdditions *v1alpha1.PodAdditions, + workspace *dw.DevWorkspaceTemplateSpec, + restrictedFields []string, + subPathFunc func(workspaceId, volumeName string) string, +) error { + devfileVolumes := map[string]dw.VolumeComponent{} + + // Construct map of volume name -> volume Component + for _, component := range workspace.Components { + if component.Volume != nil { + if _, exists := devfileVolumes[component.Name]; exists { + return fmt.Errorf("volume component '%s' is defined multiple times", component.Name) + } + devfileVolumes[component.Name] = *component.Volume + } + } + + // Containers in podAdditions may reference e.g. automounted volumes in their volumeMounts, and this is not an error + additionalVolumes := map[string]bool{} + for _, additionalVolume := range podAdditions.Volumes { + additionalVolumes[additionalVolume.Name] = true + } + + // Containers in podAdditions may reference volumes defined in pod overrides, and this is not an error + overridesVolumes, err := overrides.GetVolumesFromOverrides(workspace, restrictedFields) + if err != nil { + return err + } + for volumeName, present := range overridesVolumes { + additionalVolumes[volumeName] = present + } + + // Add implicit projects volume to support mountSources, if needed + if _, exists := devfileVolumes[devfileConstants.ProjectsVolumeName]; !exists { + projectsVolume := dw.VolumeComponent{} + projectsVolume.Size = constants.PVCStorageSize + devfileVolumes[devfileConstants.ProjectsVolumeName] = projectsVolume + } + + // TODO: What should we do when a volume isn't explicitly defined? + rewriteVolumeMounts := func(containers []corev1.Container) error { + for cIdx, container := range containers { + for vmIdx, vm := range container.VolumeMounts { + volume, ok := devfileVolumes[vm.Name] + if !ok { + // Volume is defined outside of the devfile + if additionalVolumes[vm.Name] { + continue + } + // Should never happen as flattened Devfile is validated. + return fmt.Errorf("container '%s' references undefined volume '%s'", container.Name, vm.Name) + } + if !isEphemeral(&volume) { + containers[cIdx].VolumeMounts[vmIdx].SubPath = subPathFunc(workspaceId, vm.Name) + containers[cIdx].VolumeMounts[vmIdx].Name = pvcName + } + } + } + return nil + } + if err := rewriteVolumeMounts(podAdditions.Containers); err != nil { + return err + } + if err := rewriteVolumeMounts(podAdditions.InitContainers); err != nil { + return err + } + + podAdditions.Volumes = append(podAdditions.Volumes, corev1.Volume{ + Name: pvcName, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: pvcName, + }, + }, + }) + + return nil +}