Skip to content

Commit 765de9f

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 765de9f

File tree

1 file changed

+95
-47
lines changed

1 file changed

+95
-47
lines changed

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

Lines changed: 95 additions & 47 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,38 +212,34 @@ 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")
207-
store := NewPipelineStoreKubernetes(getClient())
230+
defer viper.Set("POD_NAMESPACE", podNamespace)
208231

209-
pipeline := &model.Pipeline{
210-
UUID: DefaultFakePipelineIdFive,
211-
Name: "Test Pipeline 3",
212-
Description: "Test Pipeline 3 Description",
213-
Namespace: "Test",
214-
}
232+
store := NewPipelineStoreKubernetes(getClient())
215233

216-
_, err := store.GetPipeline(pipeline.UUID)
234+
_, err := store.GetPipeline(DefaultFakePipelineIdFive)
217235
require.NotNil(t, err)
218236
}
219237

220238
func TestCreateK8sPipeline(t *testing.T) {
239+
podNamespace := viper.Get("POD_NAMESPACE")
240+
viper.Set("POD_NAMESPACE", "Test")
241+
defer viper.Set("POD_NAMESPACE", podNamespace)
242+
221243
store := NewPipelineStoreKubernetes(getClient())
222244

223245
pipeline := &model.Pipeline{
@@ -235,7 +257,10 @@ func TestCreateK8sPipeline(t *testing.T) {
235257
}
236258

237259
func TestDeleteK8sPipeline(t *testing.T) {
238-
viper.Set("POD_NAMESPACE", "test")
260+
podNamespace := viper.Get("POD_NAMESPACE")
261+
viper.Set("POD_NAMESPACE", "Test")
262+
defer viper.Set("POD_NAMESPACE", podNamespace)
263+
239264
store := NewPipelineStoreKubernetes(getClient())
240265

241266
err := store.DeletePipeline(DefaultFakePipelineId)
@@ -247,22 +272,27 @@ func TestDeleteK8sPipeline(t *testing.T) {
247272
}
248273

249274
func TestCreateK8sPipelineVersion(t *testing.T) {
275+
podNamespace := viper.Get("POD_NAMESPACE")
250276
viper.Set("POD_NAMESPACE", "Test")
277+
defer viper.Set("POD_NAMESPACE", podNamespace)
278+
251279
store := NewPipelineStoreKubernetes(getClient())
252280

253281
pipelineVersion := &model.PipelineVersion{
254282
Name: "Test Pipeline Version",
255-
PipelineId: DefaultFakePipelineId,
283+
PipelineId: DefaultFakePipelineIdTwo,
256284
Description: "Test Pipeline Version Description",
257285
}
258286

259287
_, err := store.CreatePipelineVersion(pipelineVersion)
260288
require.Nil(t, err, "Failed to create PipelineVersion: %v", err)
261-
// require.Equal(t, pv, *pipelineVersion)
262289
}
263290

264291
func TestDeleteK8sPipelineVersion(t *testing.T) {
292+
podNamespace := viper.Get("POD_NAMESPACE")
265293
viper.Set("POD_NAMESPACE", "Test")
294+
defer viper.Set("POD_NAMESPACE", podNamespace)
295+
266296
store := NewPipelineStoreKubernetes(getClient())
267297

268298
err := store.DeletePipelineVersion(DefaultFakePipelineId)
@@ -276,6 +306,10 @@ func TestDeleteK8sPipelineVersion(t *testing.T) {
276306
}
277307

278308
func TestGetK8sPipelineVersion(t *testing.T) {
309+
podNamespace := viper.Get("POD_NAMESPACE")
310+
viper.Set("POD_NAMESPACE", "Test")
311+
defer viper.Set("POD_NAMESPACE", podNamespace)
312+
279313
store := NewPipelineStoreKubernetes(getClient())
280314

281315
pipelineVersion := &model.PipelineVersion{
@@ -290,7 +324,10 @@ func TestGetK8sPipelineVersion(t *testing.T) {
290324
}
291325

292326
func TestGetLatestK8sPipelineVersion(t *testing.T) {
327+
podNamespace := viper.Get("POD_NAMESPACE")
293328
viper.Set("POD_NAMESPACE", "Test")
329+
defer viper.Set("POD_NAMESPACE", podNamespace)
330+
294331
store := NewPipelineStoreKubernetes(getClient())
295332

296333
pipelineVersion, err := store.GetLatestPipelineVersion(DefaultFakePipelineIdTwo)
@@ -299,7 +336,10 @@ func TestGetLatestK8sPipelineVersion(t *testing.T) {
299336
}
300337

301338
func TestGetK8sPipelineVersion_NotFoundError(t *testing.T) {
339+
podNamespace := viper.Get("POD_NAMESPACE")
302340
viper.Set("POD_NAMESPACE", "Test")
341+
defer viper.Set("POD_NAMESPACE", podNamespace)
342+
303343
store := NewPipelineStoreKubernetes(getClient())
304344

305345
_, err := store.GetLatestPipelineVersion(DefaultFakePipelineIdFive)
@@ -308,16 +348,20 @@ func TestGetK8sPipelineVersion_NotFoundError(t *testing.T) {
308348
}
309349

310350
func TestListK8sPipelineVersions_Pagination(t *testing.T) {
351+
podNamespace := viper.Get("POD_NAMESPACE")
352+
viper.Set("POD_NAMESPACE", "Test")
353+
defer viper.Set("POD_NAMESPACE", podNamespace)
354+
311355
store := NewPipelineStoreKubernetes(getClient())
312356

313357
pipelineVersion1 := &model.PipelineVersion{
314358
Name: "Test Pipeline Version 1",
315-
PipelineId: DefaultFakePipelineId,
359+
PipelineId: DefaultFakePipelineIdTwo,
316360
}
317361

318362
pipelineVersion2 := &model.PipelineVersion{
319363
Name: "Test Pipeline Version 2",
320-
PipelineId: DefaultFakePipelineId,
364+
PipelineId: DefaultFakePipelineIdTwo,
321365
}
322366

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

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

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

335380
options, err = list.NewOptionsFromToken(npt, 1)
381+
require.Nil(t, err, "Failed to create list options")
336382
pipelineVersions, _, _, err = store.ListPipelineVersions(DefaultFakePipelineIdTwo, options)
383+
require.Nil(t, err, "Failed to list pipeline versions: %v", err)
337384
require.Equalf(t, len(pipelineVersions), 1, "List size should not be zero")
338385
require.Equalf(t, pipelineVersions[0].Name, "Test Pipeline Version 3", "Pagination did not work as expected")
339386
}
340387

341388
func TestListK8sPipelineVersions_Pagination_Descend(t *testing.T) {
389+
podNamespace := viper.Get("POD_NAMESPACE")
390+
viper.Set("POD_NAMESPACE", "Test")
391+
defer viper.Set("POD_NAMESPACE", podNamespace)
392+
342393
store := NewPipelineStoreKubernetes(getClient())
343394

344395
pipelineVersion1 := &model.PipelineVersion{
345396
Name: "Test Pipeline Version 1",
346-
PipelineId: DefaultFakePipelineId,
397+
PipelineId: DefaultFakePipelineIdTwo,
347398
}
348399

349400
pipelineVersion2 := &model.PipelineVersion{
350401
Name: "Test Pipeline Version 2",
351-
PipelineId: DefaultFakePipelineId,
402+
PipelineId: DefaultFakePipelineIdTwo,
352403
}
353404

354405
_, err := store.CreatePipelineVersion(pipelineVersion1)
@@ -365,6 +416,10 @@ func TestListK8sPipelineVersions_Pagination_Descend(t *testing.T) {
365416
}
366417

367418
func TestListK8sPipelineVersions_Pagination_LessThanPageSize(t *testing.T) {
419+
podNamespace := viper.Get("POD_NAMESPACE")
420+
viper.Set("POD_NAMESPACE", "Test")
421+
defer viper.Set("POD_NAMESPACE", podNamespace)
422+
368423
store := NewPipelineStoreKubernetes(getClient())
369424

370425
options, err1 := list.NewOptions(&model.Pipeline{}, 10, "", nil)
@@ -377,6 +432,10 @@ func TestListK8sPipelineVersions_Pagination_LessThanPageSize(t *testing.T) {
377432
}
378433

379434
func TestGetK8sPipelineVersionByName(t *testing.T) {
435+
podNamespace := viper.Get("POD_NAMESPACE")
436+
viper.Set("POD_NAMESPACE", "Test")
437+
defer viper.Set("POD_NAMESPACE", podNamespace)
438+
380439
store := NewPipelineStoreKubernetes(getClient())
381440

382441
pipelineVersion, err := store.GetPipelineVersionByName("Test Pipeline Version 3")
@@ -385,7 +444,10 @@ func TestGetK8sPipelineVersionByName(t *testing.T) {
385444
}
386445

387446
func TestListK8sPipelineVersions_WithFilter(t *testing.T) {
447+
podNamespace := viper.Get("POD_NAMESPACE")
388448
viper.Set("POD_NAMESPACE", "Test")
449+
defer viper.Set("POD_NAMESPACE", podNamespace)
450+
389451
store := NewPipelineStoreKubernetes(getClient())
390452

391453
filterProto := &api.Filter{
@@ -408,7 +470,10 @@ func TestListK8sPipelineVersions_WithFilter(t *testing.T) {
408470
}
409471

410472
func TestCreatePipelineAndPipelineVersion(t *testing.T) {
473+
podNamespace := viper.Get("POD_NAMESPACE")
411474
viper.Set("POD_NAMESPACE", "Test")
475+
defer viper.Set("POD_NAMESPACE", podNamespace)
476+
412477
store := NewPipelineStoreKubernetes(getClient())
413478

414479
k8sPipeline := &model.Pipeline{
@@ -426,27 +491,7 @@ func getClient() (client.Client, client.Client) {
426491
scheme := runtime.NewScheme()
427492
err := v2beta1.AddToScheme(scheme)
428493
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-
},
494+
glog.Fatalf("Failed to add to scheme: %v", err)
450495
}
451496

452497
pipeline3 := &v2beta1.Pipeline{
@@ -471,6 +516,9 @@ func getClient() (client.Client, client.Client) {
471516
ObjectMeta: metav1.ObjectMeta{
472517
Name: "Test Pipeline Version 1",
473518
Namespace: "Test",
519+
Labels: map[string]string{
520+
"pipelines.kubeflow.org/pipeline-id": DefaultFakePipelineId,
521+
},
474522
},
475523
}
476524

@@ -502,7 +550,7 @@ func getClient() (client.Client, client.Client) {
502550

503551
k8sClient := fake.NewClientBuilder().
504552
WithScheme(scheme).
505-
WithStatusSubresource(pipeline, pipeline1, pipeline2, pipelineVersion, pipelineVersion1, pipelineVersion2).
553+
WithStatusSubresource(pipelineVersion, pipelineVersion1, pipelineVersion2, pipelineVersion3).
506554
WithObjects(pipeline3, pipelineVersion3).
507555
Build()
508556

0 commit comments

Comments
 (0)