Skip to content

Commit c6cbe19

Browse files
amisevskamisevsk
committed
Improve volume handling in library to correctly manage mountSources
- Add documentation to various methods as library functions deal with more edge cases - Split package pkg/library/container into multiple files for readability - Improve handling of mountSources to respect sourceMapping, correctly define projects volume - Improve common PVC provisioning to not add PVC if not needed Signed-off-by: Angel Misevski <amisevsk@redhat.com>
1 parent 134181b commit c6cbe19

File tree

5 files changed

+251
-136
lines changed

5 files changed

+251
-136
lines changed

‎pkg/library/constants/constants.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
//
2+
// Copyright (c) 2019-2020 Red Hat, Inc.
3+
// This program and the accompanying materials are made
4+
// available under the terms of the Eclipse Public License 2.0
5+
// which is available at https://www.eclipse.org/legal/epl-2.0/
6+
//
7+
// SPDX-License-Identifier: EPL-2.0
8+
//
9+
// Contributors:
10+
// Red Hat, Inc. - initial API and implementation
11+
//
12+
13+
// Package constants contains constants related to the devfile API spec (e.g. reserved env vars)
14+
package constants
15+
16+
const (
17+
ProjectsRootEnvVar = "PROJECTS_ROOT"
18+
ProjectsSourceEnvVar = "PROJECTS_SOURCE"
19+
ProjectsVolumeName = "projects"
20+
)

‎pkg/library/container/container.go‎

Lines changed: 14 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,33 @@
1212

1313
// Package container contains library functions for converting DevWorkspace Container components to Kubernetes
1414
// components
15+
//
16+
// TODO:
17+
// - Devfile API spec is unclear on how mountSources should be handled -- mountPath is assumed to be /projects
18+
// and volume name is assumed to be "projects"
19+
// see issues:
20+
// - https://github.com/devfile/api/issues/290
21+
// - https://github.com/devfile/api/issues/291
1522
package container
1623

1724
import (
1825
"fmt"
1926

2027
devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
2128
"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
22-
"github.com/devfile/devworkspace-operator/pkg/config"
2329
"github.com/devfile/devworkspace-operator/pkg/library"
2430
"github.com/devfile/devworkspace-operator/pkg/library/lifecycle"
25-
corev1 "k8s.io/api/core/v1"
26-
"k8s.io/apimachinery/pkg/api/resource"
27-
)
28-
29-
const (
30-
ProjectsRootEnvVar = "PROJECTS_ROOT"
31-
ProjectsSourceEnvVar = "PROJECTS_SOURCE"
32-
ProjecstVolumeName = "projects"
3331
)
3432

3533
// GetKubeContainersFromDevfile converts container components in a DevWorkspace into Kubernetes containers.
3634
// If a DevWorkspace container is an init container (i.e. is bound to a preStart event), it will be returned as an
3735
// init container.
3836
//
37+
// This function also provisions volume mounts on containers as follows:
38+
// - Container component's volume mounts are provisioned with the mount path and name specified in the devworkspace
39+
// However, no Volumes are added to the returned PodAdditions at this stage; the volumeMounts above are expected to be
40+
// rewritten as Volumes are added to PodAdditions, in order to support e.g. using one PVC to hold all volumes
41+
//
3942
// Note: Requires DevWorkspace to be flattened (i.e. the DevWorkspace contains no Parent or Components of type Plugin)
4043
func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpec) (*v1alpha1.PodAdditions, error) {
4144
if !library.DevWorkspaceIsFlattened(workspace) {
@@ -56,6 +59,7 @@ func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpe
5659
if err != nil {
5760
return nil, err
5861
}
62+
handleMountSources(k8sContainer, component.Container)
5963
podAdditions.Containers = append(podAdditions.Containers, *k8sContainer)
6064
}
6165

@@ -64,130 +68,9 @@ func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpe
6468
if err != nil {
6569
return nil, err
6670
}
71+
handleMountSources(k8sContainer, container.Container)
6772
podAdditions.InitContainers = append(podAdditions.InitContainers, *k8sContainer)
6873
}
6974

70-
fillDefaultEnvVars(podAdditions, workspace)
71-
7275
return podAdditions, nil
7376
}
74-
75-
func convertContainerToK8s(devfileComponent devworkspace.Component) (*corev1.Container, error) {
76-
if devfileComponent.Container == nil {
77-
return nil, fmt.Errorf("cannot get k8s container from non-container component")
78-
}
79-
devfileContainer := devfileComponent.Container
80-
81-
containerResources, err := devfileResourcesToContainerResources(devfileContainer)
82-
if err != nil {
83-
return nil, err
84-
}
85-
86-
var mountSources bool
87-
if devfileContainer.MountSources == nil {
88-
mountSources = true
89-
} else {
90-
mountSources = *devfileContainer.MountSources
91-
}
92-
93-
container := &corev1.Container{
94-
Name: devfileComponent.Name,
95-
Image: devfileContainer.Image,
96-
Command: devfileContainer.Command,
97-
Args: devfileContainer.Args,
98-
Resources: *containerResources,
99-
Ports: devfileEndpointsToContainerPorts(devfileContainer.Endpoints),
100-
Env: devfileEnvToContainerEnv(devfileContainer.Env),
101-
VolumeMounts: devfileVolumeMountsToContainerVolumeMounts(devfileContainer.VolumeMounts, mountSources),
102-
ImagePullPolicy: corev1.PullPolicy(config.ControllerCfg.GetSidecarPullPolicy()),
103-
}
104-
105-
return container, nil
106-
}
107-
108-
func devfileEndpointsToContainerPorts(endpoints []devworkspace.Endpoint) []corev1.ContainerPort {
109-
var containerPorts []corev1.ContainerPort
110-
for _, endpoint := range endpoints {
111-
containerPorts = append(containerPorts, corev1.ContainerPort{
112-
Name: endpoint.Name,
113-
ContainerPort: int32(endpoint.TargetPort),
114-
Protocol: corev1.ProtocolTCP,
115-
})
116-
}
117-
return containerPorts
118-
}
119-
120-
func devfileResourcesToContainerResources(devfileContainer *devworkspace.ContainerComponent) (*corev1.ResourceRequirements, error) {
121-
// TODO: Handle memory request and CPU when implemented in devfile API
122-
memLimit := devfileContainer.MemoryLimit
123-
if memLimit == "" {
124-
memLimit = config.SidecarDefaultMemoryLimit
125-
}
126-
memLimitQuantity, err := resource.ParseQuantity(memLimit)
127-
if err != nil {
128-
return nil, fmt.Errorf("failed to parse memory limit %q: %w", memLimit, err)
129-
}
130-
return &corev1.ResourceRequirements{
131-
Limits: corev1.ResourceList{
132-
corev1.ResourceMemory: memLimitQuantity,
133-
},
134-
}, nil
135-
}
136-
137-
func devfileVolumeMountsToContainerVolumeMounts(devfileVolumeMounts []devworkspace.VolumeMount, mountSources bool) []corev1.VolumeMount {
138-
var volumeMounts []corev1.VolumeMount
139-
for _, vm := range devfileVolumeMounts {
140-
volumeMounts = append(volumeMounts, corev1.VolumeMount{
141-
Name: vm.Name,
142-
MountPath: vm.Path,
143-
})
144-
}
145-
if mountSources {
146-
volumeMounts = append(volumeMounts, corev1.VolumeMount{
147-
Name: ProjecstVolumeName,
148-
MountPath: config.DefaultProjectsSourcesRoot,
149-
})
150-
}
151-
return volumeMounts
152-
}
153-
154-
func devfileEnvToContainerEnv(devfileEnvVars []devworkspace.EnvVar) []corev1.EnvVar {
155-
var env []corev1.EnvVar
156-
for _, devfileEnv := range devfileEnvVars {
157-
env = append(env, corev1.EnvVar{
158-
Name: devfileEnv.Name,
159-
Value: devfileEnv.Value,
160-
})
161-
}
162-
return env
163-
}
164-
165-
func fillDefaultEnvVars(podAdditions *v1alpha1.PodAdditions, workspace devworkspace.DevWorkspaceTemplateSpec) {
166-
var projectsSource string
167-
if len(workspace.Projects) > 0 {
168-
// TODO: Unclear from devfile spec how this should work when there are multiple projects
169-
projectsSource = fmt.Sprintf("%s/%s", config.DefaultProjectsSourcesRoot, workspace.Projects[0].ClonePath)
170-
} else {
171-
projectsSource = config.DefaultProjectsSourcesRoot
172-
}
173-
174-
// Add devfile reserved env var and legacy env var for Che-Theia
175-
for idx, container := range podAdditions.Containers {
176-
podAdditions.Containers[idx].Env = append(container.Env, corev1.EnvVar{
177-
Name: ProjectsRootEnvVar,
178-
Value: config.DefaultProjectsSourcesRoot,
179-
}, corev1.EnvVar{
180-
Name: ProjectsSourceEnvVar,
181-
Value: projectsSource,
182-
})
183-
}
184-
for idx, container := range podAdditions.InitContainers {
185-
podAdditions.InitContainers[idx].Env = append(container.Env, corev1.EnvVar{
186-
Name: ProjectsRootEnvVar,
187-
Value: config.DefaultProjectsSourcesRoot,
188-
}, corev1.EnvVar{
189-
Name: ProjectsSourceEnvVar,
190-
Value: projectsSource,
191-
})
192-
}
193-
}

‎pkg/library/container/conversion.go‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
//
2+
// Copyright (c) 2019-2020 Red Hat, Inc.
3+
// This program and the accompanying materials are made
4+
// available under the terms of the Eclipse Public License 2.0
5+
// which is available at https://www.eclipse.org/legal/epl-2.0/
6+
//
7+
// SPDX-License-Identifier: EPL-2.0
8+
//
9+
// Contributors:
10+
// Red Hat, Inc. - initial API and implementation
11+
//
12+
13+
package container
14+
15+
import (
16+
"fmt"
17+
18+
"github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
19+
"github.com/devfile/devworkspace-operator/pkg/config"
20+
v1 "k8s.io/api/core/v1"
21+
"k8s.io/apimachinery/pkg/api/resource"
22+
)
23+
24+
func convertContainerToK8s(devfileComponent v1alpha2.Component) (*v1.Container, error) {
25+
if devfileComponent.Container == nil {
26+
return nil, fmt.Errorf("cannot get k8s container from non-container component")
27+
}
28+
devfileContainer := devfileComponent.Container
29+
30+
containerResources, err := devfileResourcesToContainerResources(devfileContainer)
31+
if err != nil {
32+
return nil, err
33+
}
34+
35+
container := &v1.Container{
36+
Name: devfileComponent.Name,
37+
Image: devfileContainer.Image,
38+
Command: devfileContainer.Command,
39+
Args: devfileContainer.Args,
40+
Resources: *containerResources,
41+
Ports: devfileEndpointsToContainerPorts(devfileContainer.Endpoints),
42+
Env: devfileEnvToContainerEnv(devfileContainer.Env),
43+
VolumeMounts: devfileVolumeMountsToContainerVolumeMounts(devfileContainer.VolumeMounts),
44+
ImagePullPolicy: v1.PullPolicy(config.ControllerCfg.GetSidecarPullPolicy()),
45+
}
46+
47+
return container, nil
48+
}
49+
50+
func devfileEndpointsToContainerPorts(endpoints []v1alpha2.Endpoint) []v1.ContainerPort {
51+
var containerPorts []v1.ContainerPort
52+
for _, endpoint := range endpoints {
53+
containerPorts = append(containerPorts, v1.ContainerPort{
54+
// Use meaningless name for port since endpoint.Name does not match requirements for ContainerPort name
55+
Name: fmt.Sprintf("%d-%s", endpoint.TargetPort, endpoint.Protocol),
56+
ContainerPort: int32(endpoint.TargetPort),
57+
Protocol: v1.ProtocolTCP,
58+
})
59+
}
60+
return containerPorts
61+
}
62+
63+
func devfileResourcesToContainerResources(devfileContainer *v1alpha2.ContainerComponent) (*v1.ResourceRequirements, error) {
64+
// TODO: Handle memory request and CPU when implemented in devfile API
65+
memLimit := devfileContainer.MemoryLimit
66+
if memLimit == "" {
67+
memLimit = config.SidecarDefaultMemoryLimit
68+
}
69+
memLimitQuantity, err := resource.ParseQuantity(memLimit)
70+
if err != nil {
71+
return nil, fmt.Errorf("failed to parse memory limit %q: %w", memLimit, err)
72+
}
73+
return &v1.ResourceRequirements{
74+
Limits: v1.ResourceList{
75+
v1.ResourceMemory: memLimitQuantity,
76+
},
77+
}, nil
78+
}
79+
80+
func devfileVolumeMountsToContainerVolumeMounts(devfileVolumeMounts []v1alpha2.VolumeMount) []v1.VolumeMount {
81+
var volumeMounts []v1.VolumeMount
82+
for _, vm := range devfileVolumeMounts {
83+
path := vm.Path
84+
if path == "" {
85+
// Devfile API spec: if path is unspecified, default is to use volume name
86+
path = fmt.Sprintf("/%s", vm.Name)
87+
}
88+
volumeMounts = append(volumeMounts, v1.VolumeMount{
89+
Name: vm.Name,
90+
MountPath: path,
91+
})
92+
}
93+
return volumeMounts
94+
}
95+
96+
func devfileEnvToContainerEnv(devfileEnvVars []v1alpha2.EnvVar) []v1.EnvVar {
97+
var env []v1.EnvVar
98+
for _, devfileEnv := range devfileEnvVars {
99+
env = append(env, v1.EnvVar{
100+
Name: devfileEnv.Name,
101+
Value: devfileEnv.Value,
102+
})
103+
}
104+
return env
105+
}

‎pkg/library/container/mountSources.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//
2+
// Copyright (c) 2019-2020 Red Hat, Inc.
3+
// This program and the accompanying materials are made
4+
// available under the terms of the Eclipse Public License 2.0
5+
// which is available at https://www.eclipse.org/legal/epl-2.0/
6+
//
7+
// SPDX-License-Identifier: EPL-2.0
8+
//
9+
// Contributors:
10+
// Red Hat, Inc. - initial API and implementation
11+
//
12+
13+
package container
14+
15+
import (
16+
devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
17+
"github.com/devfile/devworkspace-operator/pkg/config"
18+
"github.com/devfile/devworkspace-operator/pkg/library/constants"
19+
corev1 "k8s.io/api/core/v1"
20+
)
21+
22+
// HasMountSources evaluates whether project sources should be mounted in the given container component.
23+
// MountSources is by default true for non-plugin components, unless they have dedicatedPod set
24+
// TODO:
25+
// - Support dedicatedPod field
26+
// - Find way to track is container component comes from plugin
27+
func HasMountSources(devfileContainer *devworkspace.ContainerComponent) bool {
28+
var mountSources bool
29+
if devfileContainer.MountSources == nil {
30+
mountSources = true
31+
} else {
32+
mountSources = *devfileContainer.MountSources
33+
}
34+
return mountSources
35+
}
36+
37+
// handleMountSources adds a volumeMount to a container if the corresponding devfile container has
38+
// mountSources enabled.
39+
func handleMountSources(k8sContainer *corev1.Container, devfileContainer *devworkspace.ContainerComponent) {
40+
if !HasMountSources(devfileContainer) {
41+
return
42+
}
43+
sourceMapping := devfileContainer.SourceMapping
44+
if sourceMapping == "" {
45+
// Sanity check -- this value should be defaulted to `/projects` but may not be
46+
// if struct was not processed by k8s
47+
sourceMapping = config.DefaultProjectsSourcesRoot
48+
}
49+
k8sContainer.VolumeMounts = append(k8sContainer.VolumeMounts, corev1.VolumeMount{
50+
Name: constants.ProjectsVolumeName,
51+
MountPath: sourceMapping,
52+
})
53+
k8sContainer.Env = append(k8sContainer.Env, corev1.EnvVar{
54+
Name: constants.ProjectsRootEnvVar,
55+
Value: sourceMapping,
56+
}, corev1.EnvVar{
57+
Name: constants.ProjectsSourceEnvVar,
58+
Value: sourceMapping, // TODO: Unclear how this should be handled in case of multiple projects
59+
})
60+
}

0 commit comments

Comments
 (0)