Skip to content

Commit 1ce9b15

Browse files
mprahlmprahl
committed
Fix the flaky Kubernetes pipeline store tests
Some tests set the viper configuration of POD_NAMESPACE while others didn't and so the order of the tests mattered. This now sets and resets the viper configuration for each test. Signed-off-by: mprahl <[email protected]>
1 parent d6bde85 commit 1ce9b15

File tree

1 file changed

+94
-39
lines changed

1 file changed

+94
-39
lines changed

‎backend/src/apiserver/storage/pipeline_store_kubernetes_test.go‎

Lines changed: 94 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package storage
33
import (
44
"testing"
55

6+
"github.com/golang/glog"
67
api "github.com/kubeflow/pipelines/backend/api/v1beta1/go_client"
78
"github.com/kubeflow/pipelines/backend/src/apiserver/filter"
89
"github.com/kubeflow/pipelines/backend/src/apiserver/list"
@@ -21,6 +22,10 @@ import (
2122
)
2223

2324
func TestListK8sPipelines(t *testing.T) {
25+
podNamespace := viper.Get("POD_NAMESPACE")
26+
viper.Set("POD_NAMESPACE", "Test")
27+
defer viper.Set("POD_NAMESPACE", podNamespace)
28+
2429
store := NewPipelineStoreKubernetes(getClient())
2530

2631
fc := &model.FilterContext{}
@@ -45,6 +50,10 @@ func TestListK8sPipelines(t *testing.T) {
4550
}
4651

4752
func TestListK8sPipelines_WithFilter(t *testing.T) {
53+
podNamespace := viper.Get("POD_NAMESPACE")
54+
viper.Set("POD_NAMESPACE", "Test")
55+
defer viper.Set("POD_NAMESPACE", podNamespace)
56+
4857
store := NewPipelineStoreKubernetes(getClient())
4958

5059
pipeline := &model.Pipeline{
@@ -74,6 +83,10 @@ func TestListK8sPipelines_WithFilter(t *testing.T) {
7483
}
7584

7685
func TestListK8sPipelines_Pagination(t *testing.T) {
86+
podNamespace := viper.Get("POD_NAMESPACE")
87+
viper.Set("POD_NAMESPACE", "Test")
88+
defer viper.Set("POD_NAMESPACE", podNamespace)
89+
7790
store := NewPipelineStoreKubernetes(getClient())
7891

7992
pipeline1 := &model.Pipeline{
@@ -101,12 +114,17 @@ func TestListK8sPipelines_Pagination(t *testing.T) {
101114
require.Equalf(t, pageSize, 3, "List size should not be zero")
102115

103116
options, err1 = list.NewOptionsFromToken(npt, 1)
117+
require.Nil(t, err1, "Failed to create list options: %v")
104118
pipelines, _, _, err3 := store.ListPipelines(&model.FilterContext{}, options)
105119
require.Nil(t, err3, "Failed to list pipelines: %v")
106120
require.Equalf(t, pipelines[0].Name, "Test Pipeline 3", "Pagination failed")
107121
}
108122

109123
func TestListK8sPipelines_Pagination_Descend(t *testing.T) {
124+
podNamespace := viper.Get("POD_NAMESPACE")
125+
viper.Set("POD_NAMESPACE", "Test")
126+
defer viper.Set("POD_NAMESPACE", podNamespace)
127+
110128
store := NewPipelineStoreKubernetes(getClient())
111129

112130
pipeline1 := &model.Pipeline{
@@ -140,6 +158,10 @@ func TestListK8sPipelines_Pagination_Descend(t *testing.T) {
140158
}
141159

142160
func TestListK8sPipelinesV1_Pagination_NameAsc(t *testing.T) {
161+
podNamespace := viper.Get("POD_NAMESPACE")
162+
viper.Set("POD_NAMESPACE", "Test")
163+
defer viper.Set("POD_NAMESPACE", podNamespace)
164+
143165
store := NewPipelineStoreKubernetes(getClient())
144166

145167
pipeline1 := &model.Pipeline{
@@ -173,6 +195,10 @@ func TestListK8sPipelinesV1_Pagination_NameAsc(t *testing.T) {
173195
}
174196

175197
func TestListK8sPipelines_Pagination_LessThanPageSize(t *testing.T) {
198+
podNamespace := viper.Get("POD_NAMESPACE")
199+
viper.Set("POD_NAMESPACE", "Test")
200+
defer viper.Set("POD_NAMESPACE", podNamespace)
201+
176202
store := NewPipelineStoreKubernetes(getClient())
177203

178204
options, err1 := list.NewOptions(&model.Pipeline{}, 10, "", nil)
@@ -186,24 +212,23 @@ func TestListK8sPipelines_Pagination_LessThanPageSize(t *testing.T) {
186212

187213
func TestGetK8sPipeline(t *testing.T) {
188214
// This is important for getting a K8s pipeline
215+
podNamespace := viper.Get("POD_NAMESPACE")
189216
viper.Set("POD_NAMESPACE", "Test")
190-
store := NewPipelineStoreKubernetes(getClient())
217+
defer viper.Set("POD_NAMESPACE", podNamespace)
191218

192-
pipeline := &model.Pipeline{
193-
UUID: DefaultFakePipelineId,
194-
Name: "Test Pipeline 3",
195-
Description: "Test Pipeline 3 Description",
196-
Namespace: "Test",
197-
}
219+
store := NewPipelineStoreKubernetes(getClient())
198220

199-
p, err := store.GetPipeline(pipeline.UUID)
221+
p, err := store.GetPipeline(DefaultFakePipelineIdTwo)
200222
require.Nil(t, err, "Failed to get Pipeline: %v", err)
201-
require.Equal(t, p.UUID, pipeline.UUID)
223+
require.Equal(t, p.UUID, DefaultFakePipelineIdTwo)
202224
}
203225

204226
func TestGetK8sPipeline_NotFoundError(t *testing.T) {
205227
// This is important for getting a K8s pipeline
228+
podNamespace := viper.Get("POD_NAMESPACE")
206229
viper.Set("POD_NAMESPACE", "Test")
230+
defer viper.Set("POD_NAMESPACE", podNamespace)
231+
207232
store := NewPipelineStoreKubernetes(getClient())
208233

209234
pipeline := &model.Pipeline{
@@ -218,6 +243,10 @@ func TestGetK8sPipeline_NotFoundError(t *testing.T) {
218243
}
219244

220245
func TestCreateK8sPipeline(t *testing.T) {
246+
podNamespace := viper.Get("POD_NAMESPACE")
247+
viper.Set("POD_NAMESPACE", "Test")
248+
defer viper.Set("POD_NAMESPACE", podNamespace)
249+
221250
store := NewPipelineStoreKubernetes(getClient())
222251

223252
pipeline := &model.Pipeline{
@@ -235,7 +264,10 @@ func TestCreateK8sPipeline(t *testing.T) {
235264
}
236265

237266
func TestDeleteK8sPipeline(t *testing.T) {
238-
viper.Set("POD_NAMESPACE", "test")
267+
podNamespace := viper.Get("POD_NAMESPACE")
268+
viper.Set("POD_NAMESPACE", "Test")
269+
defer viper.Set("POD_NAMESPACE", podNamespace)
270+
239271
store := NewPipelineStoreKubernetes(getClient())
240272

241273
err := store.DeletePipeline(DefaultFakePipelineId)
@@ -247,22 +279,27 @@ func TestDeleteK8sPipeline(t *testing.T) {
247279
}
248280

249281
func TestCreateK8sPipelineVersion(t *testing.T) {
282+
podNamespace := viper.Get("POD_NAMESPACE")
250283
viper.Set("POD_NAMESPACE", "Test")
284+
defer viper.Set("POD_NAMESPACE", podNamespace)
285+
251286
store := NewPipelineStoreKubernetes(getClient())
252287

253288
pipelineVersion := &model.PipelineVersion{
254289
Name: "Test Pipeline Version",
255-
PipelineId: DefaultFakePipelineId,
290+
PipelineId: DefaultFakePipelineIdTwo,
256291
Description: "Test Pipeline Version Description",
257292
}
258293

259294
_, err := store.CreatePipelineVersion(pipelineVersion)
260295
require.Nil(t, err, "Failed to create PipelineVersion: %v", err)
261-
// require.Equal(t, pv, *pipelineVersion)
262296
}
263297

264298
func TestDeleteK8sPipelineVersion(t *testing.T) {
299+
podNamespace := viper.Get("POD_NAMESPACE")
265300
viper.Set("POD_NAMESPACE", "Test")
301+
defer viper.Set("POD_NAMESPACE", podNamespace)
302+
266303
store := NewPipelineStoreKubernetes(getClient())
267304

268305
err := store.DeletePipelineVersion(DefaultFakePipelineId)
@@ -276,6 +313,10 @@ func TestDeleteK8sPipelineVersion(t *testing.T) {
276313
}
277314

278315
func TestGetK8sPipelineVersion(t *testing.T) {
316+
podNamespace := viper.Get("POD_NAMESPACE")
317+
viper.Set("POD_NAMESPACE", "Test")
318+
defer viper.Set("POD_NAMESPACE", podNamespace)
319+
279320
store := NewPipelineStoreKubernetes(getClient())
280321

281322
pipelineVersion := &model.PipelineVersion{
@@ -290,7 +331,10 @@ func TestGetK8sPipelineVersion(t *testing.T) {
290331
}
291332

292333
func TestGetLatestK8sPipelineVersion(t *testing.T) {
334+
podNamespace := viper.Get("POD_NAMESPACE")
293335
viper.Set("POD_NAMESPACE", "Test")
336+
defer viper.Set("POD_NAMESPACE", podNamespace)
337+
294338
store := NewPipelineStoreKubernetes(getClient())
295339

296340
pipelineVersion, err := store.GetLatestPipelineVersion(DefaultFakePipelineIdTwo)
@@ -299,7 +343,10 @@ func TestGetLatestK8sPipelineVersion(t *testing.T) {
299343
}
300344

301345
func TestGetK8sPipelineVersion_NotFoundError(t *testing.T) {
346+
podNamespace := viper.Get("POD_NAMESPACE")
302347
viper.Set("POD_NAMESPACE", "Test")
348+
defer viper.Set("POD_NAMESPACE", podNamespace)
349+
303350
store := NewPipelineStoreKubernetes(getClient())
304351

305352
_, err := store.GetLatestPipelineVersion(DefaultFakePipelineIdFive)
@@ -308,16 +355,20 @@ func TestGetK8sPipelineVersion_NotFoundError(t *testing.T) {
308355
}
309356

310357
func TestListK8sPipelineVersions_Pagination(t *testing.T) {
358+
podNamespace := viper.Get("POD_NAMESPACE")
359+
viper.Set("POD_NAMESPACE", "Test")
360+
defer viper.Set("POD_NAMESPACE", podNamespace)
361+
311362
store := NewPipelineStoreKubernetes(getClient())
312363

313364
pipelineVersion1 := &model.PipelineVersion{
314365
Name: "Test Pipeline Version 1",
315-
PipelineId: DefaultFakePipelineId,
366+
PipelineId: DefaultFakePipelineIdTwo,
316367
}
317368

318369
pipelineVersion2 := &model.PipelineVersion{
319370
Name: "Test Pipeline Version 2",
320-
PipelineId: DefaultFakePipelineId,
371+
PipelineId: DefaultFakePipelineIdTwo,
321372
}
322373

323374
_, err := store.CreatePipelineVersion(pipelineVersion1)
@@ -326,29 +377,36 @@ func TestListK8sPipelineVersions_Pagination(t *testing.T) {
326377
require.Nil(t, err, "Failed to create PipelineVersion: %v", err)
327378

328379
options, err := list.NewOptions(&model.PipelineVersion{}, 1, "", nil)
380+
require.Nil(t, err, "Failed to create list options")
329381

330-
pipelineVersions, _, npt, err := store.ListPipelineVersions(DefaultFakePipelineId, options)
382+
pipelineVersions, _, npt, err := store.ListPipelineVersions(DefaultFakePipelineIdTwo, options)
331383
require.Nil(t, err, "Failed to list pipeline versions: %v", err)
332384
require.Equalf(t, len(pipelineVersions), 1, "List size should not be zero")
333385
require.NotNil(t, npt, "Npt should not be nil")
334386

335387
options, err = list.NewOptionsFromToken(npt, 1)
388+
require.Nil(t, err, "Failed to create list options")
336389
pipelineVersions, _, _, err = store.ListPipelineVersions(DefaultFakePipelineIdTwo, options)
390+
require.Nil(t, err, "Failed to list pipeline versions: %v", err)
337391
require.Equalf(t, len(pipelineVersions), 1, "List size should not be zero")
338392
require.Equalf(t, pipelineVersions[0].Name, "Test Pipeline Version 3", "Pagination did not work as expected")
339393
}
340394

341395
func TestListK8sPipelineVersions_Pagination_Descend(t *testing.T) {
396+
podNamespace := viper.Get("POD_NAMESPACE")
397+
viper.Set("POD_NAMESPACE", "Test")
398+
defer viper.Set("POD_NAMESPACE", podNamespace)
399+
342400
store := NewPipelineStoreKubernetes(getClient())
343401

344402
pipelineVersion1 := &model.PipelineVersion{
345403
Name: "Test Pipeline Version 1",
346-
PipelineId: DefaultFakePipelineId,
404+
PipelineId: DefaultFakePipelineIdTwo,
347405
}
348406

349407
pipelineVersion2 := &model.PipelineVersion{
350408
Name: "Test Pipeline Version 2",
351-
PipelineId: DefaultFakePipelineId,
409+
PipelineId: DefaultFakePipelineIdTwo,
352410
}
353411

354412
_, err := store.CreatePipelineVersion(pipelineVersion1)
@@ -365,6 +423,10 @@ func TestListK8sPipelineVersions_Pagination_Descend(t *testing.T) {
365423
}
366424

367425
func TestListK8sPipelineVersions_Pagination_LessThanPageSize(t *testing.T) {
426+
podNamespace := viper.Get("POD_NAMESPACE")
427+
viper.Set("POD_NAMESPACE", "Test")
428+
defer viper.Set("POD_NAMESPACE", podNamespace)
429+
368430
store := NewPipelineStoreKubernetes(getClient())
369431

370432
options, err1 := list.NewOptions(&model.Pipeline{}, 10, "", nil)
@@ -377,6 +439,10 @@ func TestListK8sPipelineVersions_Pagination_LessThanPageSize(t *testing.T) {
377439
}
378440

379441
func TestGetK8sPipelineVersionByName(t *testing.T) {
442+
podNamespace := viper.Get("POD_NAMESPACE")
443+
viper.Set("POD_NAMESPACE", "Test")
444+
defer viper.Set("POD_NAMESPACE", podNamespace)
445+
380446
store := NewPipelineStoreKubernetes(getClient())
381447

382448
pipelineVersion, err := store.GetPipelineVersionByName("Test Pipeline Version 3")
@@ -385,7 +451,10 @@ func TestGetK8sPipelineVersionByName(t *testing.T) {
385451
}
386452

387453
func TestListK8sPipelineVersions_WithFilter(t *testing.T) {
454+
podNamespace := viper.Get("POD_NAMESPACE")
388455
viper.Set("POD_NAMESPACE", "Test")
456+
defer viper.Set("POD_NAMESPACE", podNamespace)
457+
389458
store := NewPipelineStoreKubernetes(getClient())
390459

391460
filterProto := &api.Filter{
@@ -408,7 +477,10 @@ func TestListK8sPipelineVersions_WithFilter(t *testing.T) {
408477
}
409478

410479
func TestCreatePipelineAndPipelineVersion(t *testing.T) {
480+
podNamespace := viper.Get("POD_NAMESPACE")
411481
viper.Set("POD_NAMESPACE", "Test")
482+
defer viper.Set("POD_NAMESPACE", podNamespace)
483+
412484
store := NewPipelineStoreKubernetes(getClient())
413485

414486
k8sPipeline := &model.Pipeline{
@@ -426,27 +498,7 @@ func getClient() (client.Client, client.Client) {
426498
scheme := runtime.NewScheme()
427499
err := v2beta1.AddToScheme(scheme)
428500
if err != nil {
429-
}
430-
431-
pipeline := &v2beta1.Pipeline{
432-
ObjectMeta: metav1.ObjectMeta{
433-
UID: DefaultFakePipelineId,
434-
Name: "Test Pipeline",
435-
Namespace: "Test",
436-
},
437-
}
438-
439-
pipeline1 := &v2beta1.Pipeline{
440-
ObjectMeta: metav1.ObjectMeta{
441-
Name: "Test Pipeline 1",
442-
Namespace: "Test",
443-
},
444-
}
445-
pipeline2 := &v2beta1.Pipeline{
446-
ObjectMeta: metav1.ObjectMeta{
447-
Name: "Test Pipeline 2",
448-
Namespace: "Test",
449-
},
501+
glog.Fatalf("Failed to add to scheme: %v", err)
450502
}
451503

452504
pipeline3 := &v2beta1.Pipeline{
@@ -471,6 +523,9 @@ func getClient() (client.Client, client.Client) {
471523
ObjectMeta: metav1.ObjectMeta{
472524
Name: "Test Pipeline Version 1",
473525
Namespace: "Test",
526+
Labels: map[string]string{
527+
"pipelines.kubeflow.org/pipeline-id": DefaultFakePipelineId,
528+
},
474529
},
475530
}
476531

@@ -502,7 +557,7 @@ func getClient() (client.Client, client.Client) {
502557

503558
k8sClient := fake.NewClientBuilder().
504559
WithScheme(scheme).
505-
WithStatusSubresource(pipeline, pipeline1, pipeline2, pipelineVersion, pipelineVersion1, pipelineVersion2).
560+
WithStatusSubresource(pipelineVersion, pipelineVersion1, pipelineVersion2, pipelineVersion3).
506561
WithObjects(pipeline3, pipelineVersion3).
507562
Build()
508563

0 commit comments

Comments
 (0)