Skip to content

Commit 20f84de

Browse files
johnmcollierjohnmcollier
authored
Add support for SourceMapping field in devfiles (#3295)
* Support sourceMapping field in devfiles (kube) * Support sourceMapping for Docker * Update unit test * Fix file sync for Docker Signed-off-by: John Collier <[email protected]> * Integration tests Signed-off-by: John Collier <[email protected]> * Update comment Signed-off-by: John Collier <[email protected]> * Fix unit test Signed-off-by: John Collier <[email protected]> * Fix unit tests Signed-off-by: John Collier <[email protected]> * Fix tests Signed-off-by: John Collier <[email protected]> * Update tests again Signed-off-by: John Collier <[email protected]> * Fix tests Signed-off-by: John Collier <[email protected]> * Fix docker test Signed-off-by: John Collier <[email protected]>
1 parent 3df6f0a commit 20f84de

File tree

16 files changed

+203
-47
lines changed

16 files changed

+203
-47
lines changed

Diff for: ‎pkg/devfile/adapters/common/types.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,11 @@ type SyncParameters struct {
5151
ComponentExists bool
5252
}
5353

54-
// ComponentInfo is a struct that holds information about a component i.e.; pod name, container name
54+
// ComponentInfo is a struct that holds information about a component i.e.; pod name, container name, and source mount (if applicable)
5555
type ComponentInfo struct {
5656
PodName string
5757
ContainerName string
58+
SourceMount string
5859
}
5960

6061
// PushCommandsMap stores the commands to be executed as per their types.

Diff for: ‎pkg/devfile/adapters/docker/component/adapter.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
106106
}
107107

108108
// Find at least one container with the source volume mounted, error out if none can be found
109-
containerID, err := getFirstContainerWithSourceVolume(containers)
109+
containerID, sourceMount, err := getFirstContainerWithSourceVolume(containers)
110110
if err != nil {
111111
return errors.Wrapf(err, "error while retrieving container for odo component %s with a mounted project volume", a.ComponentName)
112112
}
@@ -118,6 +118,7 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
118118
// podChanged is defaulted to false, since docker volume is always present even if container goes down
119119
compInfo := common.ComponentInfo{
120120
ContainerName: containerID,
121+
SourceMount: sourceMount,
121122
}
122123
syncParams := common.SyncParameters{
123124
PushParams: parameters,
@@ -150,16 +151,16 @@ func (a Adapter) DoesComponentExist(cmpName string) bool {
150151
// Because the source volume is shared across all components that need it, we only need to sync once,
151152
// so we only need to find one container. If no container was found, that means there's no
152153
// container to sync to, so return an error
153-
func getFirstContainerWithSourceVolume(containers []types.Container) (string, error) {
154+
func getFirstContainerWithSourceVolume(containers []types.Container) (string, string, error) {
154155
for _, c := range containers {
155156
for _, mount := range c.Mounts {
156-
if mount.Destination == lclient.OdoSourceVolumeMount {
157-
return c.ID, nil
157+
if strings.Contains(mount.Name, lclient.ProjectSourceVolumeName) {
158+
return c.ID, mount.Destination, nil
158159
}
159160
}
160161
}
161162

162-
return "", fmt.Errorf("in order to sync files, odo requires at least one component in a devfile to set 'mountSources: true'")
163+
return "", "", fmt.Errorf("in order to sync files, odo requires at least one component in a devfile to set 'mountSources: true'")
163164
}
164165

165166
// Delete attempts to delete the component with the specified labels, returning an error if it fails

Diff for: ‎pkg/devfile/adapters/docker/component/utils.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@ import (
2626
)
2727

2828
const (
29-
// LocalhostIP is the IP address for localhost
3029
LocalhostIP = "127.0.0.1"
31-
32-
projectSourceVolumeName = "odo-project-source"
3330
)
3431

3532
func (a Adapter) createComponent() (err error) {
@@ -203,7 +200,11 @@ func (a Adapter) startComponent(mounts []mount.Mount, comp versionsCommon.Devfil
203200

204201
// If the component set `mountSources` to true, add the source volume and env CHE_PROJECTS_ROOT to it
205202
if comp.Container.MountSources {
206-
utils.AddVolumeToContainer(a.projectVolumeName, lclient.OdoSourceVolumeMount, &hostConfig)
203+
if comp.Container.SourceMapping != "" {
204+
utils.AddVolumeToContainer(a.projectVolumeName, comp.Container.SourceMapping, &hostConfig)
205+
} else {
206+
utils.AddVolumeToContainer(a.projectVolumeName, lclient.OdoSourceVolumeMount, &hostConfig)
207+
}
207208

208209
if !common.IsEnvPresent(comp.Container.Env, common.EnvCheProjectsRoot) {
209210
envName := common.EnvCheProjectsRoot
@@ -404,7 +405,7 @@ func (a Adapter) createProjectVolumeIfReqd() (string, error) {
404405

405406
if len(projectVols) == 0 {
406407
// A source volume needs to be created
407-
projectVolumeName, err = storage.GenerateVolName(projectSourceVolumeName, componentName)
408+
projectVolumeName, err = storage.GenerateVolName(lclient.ProjectSourceVolumeName, componentName)
408409
if err != nil {
409410
return "", errors.Wrapf(err, "unable to generate project source volume name for component %s", componentName)
410411
}

Diff for: ‎pkg/devfile/adapters/docker/component/utils_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -676,14 +676,14 @@ func TestCreateProjectVolumeIfReqd(t *testing.T) {
676676
name: "Case 1: Volume does not exist",
677677
componentName: "somecomponent",
678678
client: fakeClient,
679-
wantVolumeName: projectSourceVolumeName + "-somecomponent",
679+
wantVolumeName: lclient.ProjectSourceVolumeName + "-somecomponent",
680680
wantErr: false,
681681
},
682682
{
683683
name: "Case 2: Volume exist",
684684
componentName: "test",
685685
client: fakeClient,
686-
wantVolumeName: projectSourceVolumeName + "-test",
686+
wantVolumeName: lclient.ProjectSourceVolumeName + "-test",
687687
wantErr: false,
688688
},
689689
{

Diff for: ‎pkg/devfile/adapters/docker/utils/utils_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ func TestGetComponentContainers(t *testing.T) {
125125
},
126126
Mounts: []types.MountPoint{
127127
{
128+
Name: lclient.ProjectSourceVolumeName,
128129
Destination: lclient.OdoSourceVolumeMount,
129130
},
130131
},

Diff for: ‎pkg/devfile/adapters/kubernetes/component/adapter.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
127127
}
128128

129129
// Find at least one pod with the source volume mounted, error out if none can be found
130-
containerName, err := getFirstContainerWithSourceVolume(pod.Spec.Containers)
130+
containerName, sourceMount, err := getFirstContainerWithSourceVolume(pod.Spec.Containers)
131131
if err != nil {
132132
return errors.Wrapf(err, "error while retrieving container from pod %s with a mounted project volume", podName)
133133
}
@@ -138,6 +138,7 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
138138
compInfo := common.ComponentInfo{
139139
ContainerName: containerName,
140140
PodName: pod.GetName(),
141+
SourceMount: sourceMount,
141142
}
142143
syncParams := adaptersCommon.SyncParameters{
143144
PushParams: parameters,
@@ -424,20 +425,21 @@ func (a Adapter) InitRunContainerSupervisord(containerName, podName string, cont
424425
return
425426
}
426427

427-
// getFirstContainerWithSourceVolume returns the first container that set mountSources: true
428+
// getFirstContainerWithSourceVolume returns the first container that set mountSources: true as well
429+
// as the path to the source volume inside the container.
428430
// Because the source volume is shared across all components that need it, we only need to sync once,
429431
// so we only need to find one container. If no container was found, that means there's no
430432
// container to sync to, so return an error
431-
func getFirstContainerWithSourceVolume(containers []corev1.Container) (string, error) {
433+
func getFirstContainerWithSourceVolume(containers []corev1.Container) (string, string, error) {
432434
for _, c := range containers {
433435
for _, vol := range c.VolumeMounts {
434436
if vol.Name == kclient.OdoSourceVolume {
435-
return c.Name, nil
437+
return c.Name, vol.MountPath, nil
436438
}
437439
}
438440
}
439441

440-
return "", fmt.Errorf("In order to sync files, odo requires at least one component in a devfile to set 'mountSources: true'")
442+
return "", "", fmt.Errorf("In order to sync files, odo requires at least one component in a devfile to set 'mountSources: true'")
441443
}
442444

443445
// Delete deletes the component

Diff for: ‎pkg/devfile/adapters/kubernetes/component/adapter_test.go

+56-20
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,11 @@ func TestCreateOrUpdateComponent(t *testing.T) {
9898

9999
func TestGetFirstContainerWithSourceVolume(t *testing.T) {
100100
tests := []struct {
101-
name string
102-
containers []corev1.Container
103-
want string
104-
wantErr bool
101+
name string
102+
containers []corev1.Container
103+
want string
104+
wantSourcePath string
105+
wantErr bool
105106
}{
106107
{
107108
name: "Case: One container, no volumes",
@@ -110,8 +111,9 @@ func TestGetFirstContainerWithSourceVolume(t *testing.T) {
110111
Name: "test",
111112
},
112113
},
113-
want: "",
114-
wantErr: true,
114+
want: "",
115+
wantSourcePath: "",
116+
wantErr: true,
115117
},
116118
{
117119
name: "Case: One container, no source volume",
@@ -125,8 +127,9 @@ func TestGetFirstContainerWithSourceVolume(t *testing.T) {
125127
},
126128
},
127129
},
128-
want: "",
129-
wantErr: true,
130+
want: "",
131+
wantSourcePath: "",
132+
wantErr: true,
130133
},
131134
{
132135
name: "Case: One container, source volume",
@@ -135,13 +138,15 @@ func TestGetFirstContainerWithSourceVolume(t *testing.T) {
135138
Name: "test",
136139
VolumeMounts: []corev1.VolumeMount{
137140
{
138-
Name: kclient.OdoSourceVolume,
141+
Name: kclient.OdoSourceVolume,
142+
MountPath: kclient.OdoSourceVolumeMount,
139143
},
140144
},
141145
},
142146
},
143-
want: "test",
144-
wantErr: false,
147+
want: "test",
148+
wantSourcePath: kclient.OdoSourceVolumeMount,
149+
wantErr: false,
145150
},
146151
{
147152
name: "Case: One container, multiple volumes",
@@ -153,13 +158,15 @@ func TestGetFirstContainerWithSourceVolume(t *testing.T) {
153158
Name: "test",
154159
},
155160
{
156-
Name: kclient.OdoSourceVolume,
161+
Name: kclient.OdoSourceVolume,
162+
MountPath: kclient.OdoSourceVolumeMount,
157163
},
158164
},
159165
},
160166
},
161-
want: "test",
162-
wantErr: false,
167+
want: "test",
168+
wantSourcePath: kclient.OdoSourceVolumeMount,
169+
wantErr: false,
163170
},
164171
{
165172
name: "Case: Multiple containers, no source volumes",
@@ -176,8 +183,9 @@ func TestGetFirstContainerWithSourceVolume(t *testing.T) {
176183
},
177184
},
178185
},
179-
want: "",
180-
wantErr: true,
186+
want: "",
187+
wantSourcePath: "",
188+
wantErr: true,
181189
},
182190
{
183191
name: "Case: Multiple containers, multiple volumes",
@@ -192,21 +200,49 @@ func TestGetFirstContainerWithSourceVolume(t *testing.T) {
192200
Name: "test",
193201
},
194202
{
195-
Name: kclient.OdoSourceVolume,
203+
Name: kclient.OdoSourceVolume,
204+
MountPath: kclient.OdoSourceVolumeMount,
196205
},
197206
},
198207
},
199208
},
200-
want: "container-two",
201-
wantErr: false,
209+
want: "container-two",
210+
wantSourcePath: kclient.OdoSourceVolumeMount,
211+
wantErr: false,
212+
},
213+
{
214+
name: "Case: Multiple volumes, different source volume path",
215+
containers: []corev1.Container{
216+
{
217+
Name: "test",
218+
},
219+
{
220+
Name: "container-two",
221+
VolumeMounts: []corev1.VolumeMount{
222+
{
223+
Name: "test",
224+
},
225+
{
226+
Name: kclient.OdoSourceVolume,
227+
MountPath: "/some/path",
228+
},
229+
},
230+
},
231+
},
232+
want: "container-two",
233+
wantSourcePath: "/some/path",
234+
wantErr: false,
202235
},
203236
}
204237
for _, tt := range tests {
205-
container, err := getFirstContainerWithSourceVolume(tt.containers)
238+
container, sourcePath, err := getFirstContainerWithSourceVolume(tt.containers)
206239
if container != tt.want {
207240
t.Errorf("expected %s, actual %s", tt.want, container)
208241
}
209242

243+
if sourcePath != tt.wantSourcePath {
244+
t.Errorf("expected %s, actual %s", tt.wantSourcePath, sourcePath)
245+
}
210246
if !tt.wantErr == (err != nil) {
211247
t.Errorf("expected %v, actual %v", tt.wantErr, err)
212248
}

Diff for: ‎pkg/devfile/adapters/kubernetes/utils/utils.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,26 @@ func GetContainers(devfileObj devfileParser.DevfileObj) ([]corev1.Container, err
7575
}
7676

7777
// If `mountSources: true` was set, add an empty dir volume to the container to sync the source to
78+
// Sync to `Container.SourceMapping` if set
7879
if comp.Container.MountSources {
80+
var syncFolder string
81+
if comp.Container.SourceMapping != "" {
82+
syncFolder = comp.Container.SourceMapping
83+
} else {
84+
syncFolder = kclient.OdoSourceVolumeMount
85+
}
86+
7987
container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
8088
Name: kclient.OdoSourceVolume,
81-
MountPath: kclient.OdoSourceVolumeMount,
89+
MountPath: syncFolder,
8290
})
8391

8492
// only add the env if it is not set by the devfile
8593
if !isEnvPresent(container.Env, adaptersCommon.EnvCheProjectsRoot) {
8694
container.Env = append(container.Env,
8795
corev1.EnvVar{
8896
Name: adaptersCommon.EnvCheProjectsRoot,
89-
Value: kclient.OdoSourceVolumeMount,
97+
Value: syncFolder,
9098
})
9199
}
92100
}

Diff for: ‎pkg/lclient/client.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ Please ensure that Docker is currently running on your machine.
2121
// MinDockerAPIVersion is the minimum Docker API version to use
2222
// 1.30 corresponds to Docker 17.05, which should be sufficiently old enough
2323
const (
24-
MinDockerAPIVersion = "1.30" // MinDockerAPIVersion is the minimum Docker API version to use 1.30 corresponds to Docker 17.05, which should be sufficiently old enough to support most systems
25-
DockerStorageDriver = ""
26-
OdoSourceVolumeMount = "/projects"
24+
MinDockerAPIVersion = "1.30" // MinDockerAPIVersion is the minimum Docker API version to use 1.30 corresponds to Docker 17.05, which should be sufficiently old enough to support most systems
25+
DockerStorageDriver = ""
26+
OdoSourceVolumeMount = "/projects"
27+
ProjectSourceVolumeName = "odo-project-source"
2728
)
2829

2930
// DockerClient requires functions called on the docker client package

Diff for: ‎pkg/lclient/containers_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ func TestGetContainersList(t *testing.T) {
170170
},
171171
Mounts: []types.MountPoint{
172172
{
173+
Name: ProjectSourceVolumeName,
173174
Destination: OdoSourceVolumeMount,
174175
},
175176
},

Diff for: ‎pkg/lclient/fakeclient.go

+1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ var mockContainerList = []types.Container{
126126
},
127127
Mounts: []types.MountPoint{
128128
{
129+
Name: "odo-project-source",
129130
Destination: OdoSourceVolumeMount,
130131
},
131132
},

Diff for: ‎pkg/sync/adapter.go

+10-4
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,16 @@ func (a Adapter) pushLocal(path string, files []string, delFiles []string, isFor
140140
s := log.Spinner("Syncing files to the component")
141141
defer s.End(false)
142142

143-
// If there's only one project defined in the devfile, sync to `/projects/project-name`, otherwise sync to /projects
144-
syncFolder, err := getSyncFolder(a.Devfile.Data.GetProjects())
145-
if err != nil {
146-
return errors.Wrapf(err, "unable to sync the files to the component")
143+
// Determine which folder we need to sync to
144+
var syncFolder string
145+
if compInfo.SourceMount != kclient.OdoSourceVolumeMount {
146+
syncFolder = compInfo.SourceMount
147+
} else {
148+
// If there's only one project defined in the devfile, sync to `/projects/project-name`, otherwise sync to /projects
149+
syncFolder, err = getSyncFolder(a.Devfile.Data.GetProjects())
150+
if err != nil {
151+
return errors.Wrapf(err, "unable to determine sync folder")
152+
}
147153
}
148154

149155
if syncFolder != kclient.OdoSourceVolumeMount {

Diff for: ‎pkg/sync/adapter_test.go

+13
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,19 @@ func TestPushLocal(t *testing.T) {
421421
},
422422
wantErr: false,
423423
},
424+
{
425+
name: "Case 6: Source mapping folder set",
426+
client: fakeClient,
427+
path: directory,
428+
files: []string{},
429+
delFiles: []string{},
430+
isForcePush: false,
431+
compInfo: common.ComponentInfo{
432+
ContainerName: "abcd",
433+
SourceMount: "/some/path",
434+
},
435+
wantErr: false,
436+
},
424437
}
425438
for _, tt := range tests {
426439
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)