Skip to content

Commit d3efd51

Browse files
amisevsksleshchenko
andcommitted
Fix needsStorage tests and add test cases for ephemeral volumes
Update pkg/library/storage/testdata/error-unparseable-ephemeral-size.yaml Co-authored-by: Serhii Leshchenko <[email protected]> Signed-off-by: Angel Misevski <[email protected]>
1 parent d2ddaeb commit d3efd51

5 files changed

+218
-48
lines changed

pkg/library/storage/commonStorage_test.go

+85-48
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
package storage
1414

1515
import (
16-
"fmt"
1716
"io/ioutil"
1817
"path/filepath"
1918
"testing"
@@ -54,29 +53,35 @@ func setupControllerCfg() {
5453
config.SetupConfigForTesting(testControllerCfg)
5554
}
5655

57-
func loadTestCaseOrPanic(t *testing.T, testFilename string) testCase {
58-
testPath := filepath.Join("./testdata", testFilename)
59-
bytes, err := ioutil.ReadFile(testPath)
56+
func loadTestCaseOrPanic(t *testing.T, testFilepath string) testCase {
57+
bytes, err := ioutil.ReadFile(testFilepath)
6058
if err != nil {
6159
t.Fatal(err)
6260
}
6361
var test testCase
6462
if err := yaml.Unmarshal(bytes, &test); err != nil {
6563
t.Fatal(err)
6664
}
67-
t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test))
6865
return test
6966
}
7067

71-
func TestRewriteContainerVolumeMounts(t *testing.T) {
72-
tests := []testCase{
73-
loadTestCaseOrPanic(t, "does-nothing-for-no-storage-needed.yaml"),
74-
loadTestCaseOrPanic(t, "projects-volume-overriding.yaml"),
75-
loadTestCaseOrPanic(t, "rewrites-volumes-for-common-pvc-strategy.yaml"),
76-
loadTestCaseOrPanic(t, "error-duplicate-volumes.yaml"),
77-
loadTestCaseOrPanic(t, "error-undefined-volume.yaml"),
78-
loadTestCaseOrPanic(t, "error-undefined-volume-init-container.yaml"),
68+
func loadAllTestCasesOrPanic(t *testing.T, fromDir string) []testCase {
69+
files, err := ioutil.ReadDir(fromDir)
70+
if err != nil {
71+
t.Fatal(err)
7972
}
73+
var tests []testCase
74+
for _, file := range files {
75+
if file.IsDir() {
76+
continue
77+
}
78+
tests = append(tests, loadTestCaseOrPanic(t, filepath.Join(fromDir, file.Name())))
79+
}
80+
return tests
81+
}
82+
83+
func TestRewriteContainerVolumeMounts(t *testing.T) {
84+
tests := loadAllTestCasesOrPanic(t, "testdata")
8085
setupControllerCfg()
8186

8287
for _, tt := range tests {
@@ -97,60 +102,93 @@ func TestRewriteContainerVolumeMounts(t *testing.T) {
97102
}
98103

99104
func TestNeedsStorage(t *testing.T) {
100-
boolFalse := false
101105
boolTrue := true
102106
tests := []struct {
103-
Name string
104-
Explanation string
105-
Components []devworkspace.ComponentUnion
107+
Name string
108+
Explanation string
109+
NeedsStorage bool
110+
Components []devworkspace.Component
106111
}{
107112
{
108-
Name: "Has volume component",
109-
Explanation: "If the devfile has a volume component, it requires storage",
110-
Components: []devworkspace.ComponentUnion{
113+
Name: "Has volume component",
114+
Explanation: "If the devfile has a volume component, it requires storage",
115+
NeedsStorage: true,
116+
Components: []devworkspace.Component{
117+
{
118+
ComponentUnion: devworkspace.ComponentUnion{
119+
Volume: &devworkspace.VolumeComponent{},
120+
},
121+
},
122+
},
123+
},
124+
{
125+
Name: "Has ephemeral volume and does not need storage",
126+
Explanation: "Volumes with ephemeral: true do not require storage",
127+
NeedsStorage: false,
128+
Components: []devworkspace.Component{
111129
{
112-
Volume: &devworkspace.VolumeComponent{},
130+
ComponentUnion: devworkspace.ComponentUnion{
131+
Volume: &devworkspace.VolumeComponent{
132+
Volume: devworkspace.Volume{
133+
Ephemeral: true,
134+
},
135+
},
136+
},
113137
},
114138
},
115139
},
116140
{
117-
Name: "Has container component with volume mounts",
118-
Explanation: "If a devfile container has volumeMounts, it requires storage",
119-
Components: []devworkspace.ComponentUnion{
141+
Name: "Container has mountSources",
142+
Explanation: "If a devfile container has mountSources set, it requires storage",
143+
NeedsStorage: true,
144+
Components: []devworkspace.Component{
120145
{
121-
Container: &devworkspace.ContainerComponent{
122-
Container: devworkspace.Container{
123-
MountSources: &boolFalse,
124-
VolumeMounts: []devworkspace.VolumeMount{
125-
{
126-
Name: "test-volumeMount",
127-
},
146+
ComponentUnion: devworkspace.ComponentUnion{
147+
Container: &devworkspace.ContainerComponent{
148+
Container: devworkspace.Container{
149+
MountSources: &boolTrue,
128150
},
129151
},
130152
},
131153
},
132154
},
133155
},
134156
{
135-
Name: "Container has mountSources",
136-
Explanation: "If a devfile container has mountSources set, it requires storage",
137-
Components: []devworkspace.ComponentUnion{
157+
Name: "Container has mountSources but projects is ephemeral",
158+
Explanation: "When a devfile has an explicit, ephemeral projects volume, containers with mountSources do not need storage",
159+
NeedsStorage: false,
160+
Components: []devworkspace.Component{
161+
{
162+
ComponentUnion: devworkspace.ComponentUnion{
163+
Container: &devworkspace.ContainerComponent{
164+
Container: devworkspace.Container{
165+
MountSources: &boolTrue,
166+
},
167+
},
168+
},
169+
},
138170
{
139-
Container: &devworkspace.ContainerComponent{
140-
Container: devworkspace.Container{
141-
MountSources: &boolTrue,
171+
Name: "projects",
172+
ComponentUnion: devworkspace.ComponentUnion{
173+
Volume: &devworkspace.VolumeComponent{
174+
Volume: devworkspace.Volume{
175+
Ephemeral: true,
176+
},
142177
},
143178
},
144179
},
145180
},
146181
},
147182
{
148-
Name: "Container has implicit mountSources",
149-
Explanation: "If a devfile container does not have mountSources set, the default is true",
150-
Components: []devworkspace.ComponentUnion{
183+
Name: "Container has implicit mountSources",
184+
Explanation: "If a devfile container does not have mountSources set, the default is true",
185+
NeedsStorage: true,
186+
Components: []devworkspace.Component{
151187
{
152-
Container: &devworkspace.ContainerComponent{
153-
Container: devworkspace.Container{},
188+
ComponentUnion: devworkspace.ComponentUnion{
189+
Container: &devworkspace.ContainerComponent{
190+
Container: devworkspace.Container{},
191+
},
154192
},
155193
},
156194
},
@@ -159,13 +197,12 @@ func TestNeedsStorage(t *testing.T) {
159197
for _, tt := range tests {
160198
t.Run(tt.Name, func(t *testing.T) {
161199
workspace := devworkspace.DevWorkspaceTemplateSpec{}
162-
for idx, comp := range tt.Components {
163-
workspace.Components = append(workspace.Components, devworkspace.Component{
164-
Name: fmt.Sprintf("test-component-%d", idx),
165-
ComponentUnion: comp,
166-
})
200+
workspace.Components = tt.Components
201+
if tt.NeedsStorage {
202+
assert.True(t, NeedsStorage(workspace), tt.Explanation)
203+
} else {
204+
assert.False(t, NeedsStorage(workspace), tt.Explanation)
167205
}
168-
assert.True(t, NeedsStorage(workspace), tt.Explanation)
169206
})
170207
}
171208
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
name: "Can make projects volume ephemeral"
2+
3+
input:
4+
workspaceId: "test-workspaceid"
5+
podAdditions:
6+
containers:
7+
- name: testing-container-1
8+
image: testing-image
9+
volumeMounts:
10+
- name: "projects"
11+
mountPath: "/projects-mountpath"
12+
13+
workspace:
14+
components:
15+
- name: testing-container-1
16+
container:
17+
image: testing-image-1
18+
sourceMapping: "/plugins-mountpath"
19+
mountSources: true
20+
- name: projects
21+
volume:
22+
ephemeral: true
23+
24+
output:
25+
podAdditions:
26+
containers:
27+
- name: testing-container-1
28+
image: testing-image
29+
volumeMounts:
30+
- name: projects
31+
mountPath: "/projects-mountpath"
32+
33+
volumes:
34+
- name: projects
35+
emptyDir: {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
name: "Can make projects volume ephemeral"
2+
3+
input:
4+
workspaceId: "test-workspaceid"
5+
podAdditions:
6+
containers:
7+
- name: testing-container-1
8+
image: testing-image
9+
volumeMounts:
10+
- name: "projects"
11+
mountPath: "/projects-mountpath"
12+
13+
workspace:
14+
components:
15+
- name: testing-container-1
16+
container:
17+
image: testing-image-1
18+
sourceMapping: "/plugins-mountpath"
19+
mountSources: true
20+
- name: projects
21+
volume:
22+
ephemeral: true
23+
size: 512Mi
24+
25+
output:
26+
podAdditions:
27+
containers:
28+
- name: testing-container-1
29+
image: testing-image
30+
volumeMounts:
31+
- name: projects
32+
mountPath: "/projects-mountpath"
33+
34+
volumes:
35+
- name: projects
36+
emptyDir:
37+
sizeLimit: 512Mi
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
name: "Can make projects volume ephemeral"
2+
3+
input:
4+
workspaceId: "test-workspaceid"
5+
podAdditions:
6+
containers:
7+
- name: testing-container-1
8+
image: testing-image
9+
volumeMounts:
10+
- name: "projects"
11+
mountPath: "/projects-mountpath"
12+
13+
workspace:
14+
components:
15+
- name: testing-container-1
16+
container:
17+
image: testing-image-1
18+
sourceMapping: "/plugins-mountpath"
19+
mountSources: true
20+
- name: projects
21+
volume:
22+
ephemeral: true
23+
size: 512XX
24+
25+
output:
26+
errRegexp: "failed to parse size for Volume projects.*"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
name: "Handles ephemeral volumes"
2+
3+
input:
4+
workspaceId: "test-workspaceid"
5+
podAdditions:
6+
containers:
7+
- name: testing-container-1
8+
image: testing-image
9+
volumeMounts:
10+
- name: testvol
11+
mountPath: "/projects-mountpath"
12+
13+
workspace:
14+
components:
15+
- name: testing-container-1
16+
container:
17+
image: testing-image-1
18+
mountSources: false
19+
20+
- name: testvol
21+
volume:
22+
ephemeral: true
23+
24+
output:
25+
podAdditions:
26+
containers:
27+
- name: testing-container-1
28+
image: testing-image
29+
volumeMounts:
30+
- name: testvol
31+
mountPath: "/projects-mountpath"
32+
33+
volumes:
34+
- name: testvol
35+
emptyDir: {}

0 commit comments

Comments
 (0)