Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Commit

Permalink
e2e: simplify rollout block cleanup
Browse files Browse the repository at this point in the history 
Each test is supposed to clean up after itself, but sometimes we're
either doing more cleanup than we should or not doing the right way:

1. Namespaced rollout blocks do not need to be cleaned up manually.
Every test tears down the entire namespace, and with it all of its
rollout blocks. Doing this manual clean up before the tear down is
wasteful.

2. Global rollout blocks need to be cleaned up unconditionally. Instead
of doing it after each point where things can error out, we can depend
on `defer` to do it before the func exits, reliably.
  • Loading branch information
@juliogreff @parhamdoustdar
juliogreff authored and parhamdoustdar committed Sep 27, 2019
1 parent 7a0b6e1 commit 1e02648
Showing 1 changed file with 17 additions and 102 deletions.
119 changes: 17 additions & 102 deletions test/e2e/e2e_test.go
View file
Expand Up @@ -154,10 +154,6 @@ func TestNewAppAllIn(t *testing.T) {
f.waitForComplete(rel.GetName()) f.waitForComplete(rel.GetName())
t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", relName, targetReplicas) t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", relName, targetReplicas)
f.checkPods(relName, targetReplicas) f.checkPods(relName, targetReplicas)
err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Delete(newApp.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}
} }


func TestNewAppAllInWithRolloutBlockOverride(t *testing.T) { func TestNewAppAllInWithRolloutBlockOverride(t *testing.T) {
Expand Down Expand Up @@ -202,15 +198,6 @@ func TestNewAppAllInWithRolloutBlockOverride(t *testing.T) {
f.waitForComplete(rel.GetName()) f.waitForComplete(rel.GetName())
t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", relName, targetReplicas) t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", relName, targetReplicas)
f.checkPods(relName, targetReplicas) f.checkPods(relName, targetReplicas)
err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Delete(newApp.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


func TestBlockNewAppWithRolloutBlock(t *testing.T) { func TestBlockNewAppWithRolloutBlock(t *testing.T) {
Expand All @@ -221,7 +208,6 @@ func TestBlockNewAppWithRolloutBlock(t *testing.T) {


targetReplicas := 4 targetReplicas := 4
ns, err := setupNamespace(t.Name()) ns, err := setupNamespace(t.Name())
f := newFixture(ns.GetName(), t)
if err != nil { if err != nil {
t.Fatalf("could not create namespace %s: %q", ns.GetName(), err) t.Fatalf("could not create namespace %s: %q", ns.GetName(), err)
} }
Expand All @@ -232,7 +218,7 @@ func TestBlockNewAppWithRolloutBlock(t *testing.T) {
teardownNamespace(ns.GetName()) teardownNamespace(ns.GetName())
}() }()


rb, err := createRolloutBlock(ns.GetName(), rolloutBlockName) _, err = createRolloutBlock(ns.GetName(), rolloutBlockName)
if err != nil { if err != nil {
t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err) t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err)
} }
Expand All @@ -245,23 +231,8 @@ func TestBlockNewAppWithRolloutBlock(t *testing.T) {
_, err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Create(newApp) _, err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Create(newApp)
if err != nil { if err != nil {
t.Logf("successfully did not create application %q: %q", appName, err) t.Logf("successfully did not create application %q: %q", appName, err)
err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
return return
} }

f.checkPods(appName, 0)
err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Delete(newApp.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


func TestBlockNewAppProgressWithRolloutBlock(t *testing.T) { func TestBlockNewAppProgressWithRolloutBlock(t *testing.T) {
Expand Down Expand Up @@ -293,21 +264,12 @@ func TestBlockNewAppProgressWithRolloutBlock(t *testing.T) {
t.Fatalf("could not create application %q: %q", appName, err) t.Fatalf("could not create application %q: %q", appName, err)
} }


rb, err := createRolloutBlock(ns.GetName(), rolloutBlockName) _, err = createRolloutBlock(ns.GetName(), rolloutBlockName)
if err != nil { if err != nil {
t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err) t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err)
} }


f.checkPods(appName, 0) f.checkPods(appName, 0)
err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Delete(newApp.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


func TestRolloutAllIn(t *testing.T) { func TestRolloutAllIn(t *testing.T) {
Expand Down Expand Up @@ -429,11 +391,6 @@ func TestRolloutAllInWithRolloutBlockOverride(t *testing.T) {
f.waitForComplete(contender.GetName()) f.waitForComplete(contender.GetName())
t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", contender.GetName(), targetReplicas) t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", contender.GetName(), targetReplicas)
f.checkPods(contender.GetName(), targetReplicas) f.checkPods(contender.GetName(), targetReplicas)

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


func testNewApplicationVanguard(targetReplicas int, t *testing.T) { func testNewApplicationVanguard(targetReplicas int, t *testing.T) {
Expand Down Expand Up @@ -540,11 +497,6 @@ func testNewApplicationVanguardWithRolloutBlockOverride(targetReplicas int, t *t
t.Logf("checking that release %q has %d pods (strategy step %d aka %q)", relName, expectedCapacity, i, step.Name) t.Logf("checking that release %q has %d pods (strategy step %d aka %q)", relName, expectedCapacity, i, step.Name)
f.checkPods(relName, expectedCapacity) f.checkPods(relName, expectedCapacity)
} }

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


// TestNewApplicationVanguardMultipleReplicas tests the creation of a new // TestNewApplicationVanguardMultipleReplicas tests the creation of a new
Expand Down Expand Up @@ -754,7 +706,7 @@ func TestNewApplicationBlockStrategyBackwards(t *testing.T) {
} }


t.Logf("created a new rollout block object %q", rolloutBlockName) t.Logf("created a new rollout block object %q", rolloutBlockName)
rb, err := createRolloutBlock(ns.GetName(), rolloutBlockName) _, err = createRolloutBlock(ns.GetName(), rolloutBlockName)
if err != nil { if err != nil {
t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err) t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err)
} }
Expand All @@ -772,11 +724,6 @@ func TestNewApplicationBlockStrategyBackwards(t *testing.T) {


t.Logf("checking that release %q still has %d pods (strategy step %d aka %q)", relName, expectedCapacity, 1, step.Name) t.Logf("checking that release %q still has %d pods (strategy step %d aka %q)", relName, expectedCapacity, 1, step.Name)
f.checkPods(relName, int(expectedCapacity)) f.checkPods(relName, int(expectedCapacity))

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


func TestRolloutMovingStrategyBackwards(t *testing.T) { func TestRolloutMovingStrategyBackwards(t *testing.T) {
Expand Down Expand Up @@ -928,7 +875,7 @@ func TestRolloutBlockMovingStrategyBackwards(t *testing.T) {
f.checkPods(incumbentName, int(expectedIncumbentCapacity)) f.checkPods(incumbentName, int(expectedIncumbentCapacity))


t.Logf("created a new rollout block object %q", rolloutBlockName) t.Logf("created a new rollout block object %q", rolloutBlockName)
rb, err := createRolloutBlock(ns.GetName(), rolloutBlockName) _, err = createRolloutBlock(ns.GetName(), rolloutBlockName)
if err != nil { if err != nil {
t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err) t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err)
} }
Expand All @@ -953,11 +900,6 @@ func TestRolloutBlockMovingStrategyBackwards(t *testing.T) {


f.checkPods(contenderName, int(expectedContenderCapacity)) f.checkPods(contenderName, int(expectedContenderCapacity))
f.checkPods(incumbentName, int(expectedIncumbentCapacity)) f.checkPods(incumbentName, int(expectedIncumbentCapacity))

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


// TestNewApplicationAbort emulates a brand new application rollout. // TestNewApplicationAbort emulates a brand new application rollout.
Expand Down Expand Up @@ -1183,16 +1125,6 @@ func TestNewRolloutBlockAddOverrides(t *testing.T) {
fmt.Sprintf("%s/%s", newApp.GetNamespace(), newApp.GetName()), fmt.Sprintf("%s/%s", newApp.GetNamespace(), newApp.GetName()),
fmt.Sprintf("%s/%s", newApp.GetNamespace(), relName), fmt.Sprintf("%s/%s", newApp.GetNamespace(), relName),
) )

err = shipperClient.ShipperV1alpha1().Applications(namespace).Delete(newApp.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}

err = shipperClient.ShipperV1alpha1().RolloutBlocks(namespace).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


func TestNewGlobalRolloutBlockAddOverrides(t *testing.T) { func TestNewGlobalRolloutBlockAddOverrides(t *testing.T) {
Expand Down Expand Up @@ -1222,6 +1154,12 @@ func TestNewGlobalRolloutBlockAddOverrides(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("could not refetch rollout block: %q", err) t.Fatalf("could not refetch rollout block: %q", err)
} }
defer func() {
err = shipperClient.ShipperV1alpha1().RolloutBlocks(rb.GetNamespace()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
}()


if len(rb.Status.Overrides.Application) > 0 || len(rb.Status.Overrides.Release) > 0 { if len(rb.Status.Overrides.Application) > 0 || len(rb.Status.Overrides.Release) > 0 {
t.Fatalf("rollout block has unexpected overrides: %v", rb) t.Fatalf("rollout block has unexpected overrides: %v", rb)
Expand Down Expand Up @@ -1253,16 +1191,6 @@ func TestNewGlobalRolloutBlockAddOverrides(t *testing.T) {
fmt.Sprintf("%s/%s", newApp.GetNamespace(), newApp.GetName()), fmt.Sprintf("%s/%s", newApp.GetNamespace(), newApp.GetName()),
fmt.Sprintf("%s/%s", newApp.GetNamespace(), relName), fmt.Sprintf("%s/%s", newApp.GetNamespace(), relName),
) )

err = shipperClient.ShipperV1alpha1().RolloutBlocks(rb.GetNamespace()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}

err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Delete(newApp.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}
} }


func TestNewRolloutBlockRemoveRelease(t *testing.T) { func TestNewRolloutBlockRemoveRelease(t *testing.T) {
Expand Down Expand Up @@ -1344,16 +1272,6 @@ func TestNewRolloutBlockRemoveRelease(t *testing.T) {
fmt.Sprintf("%s/%s", app.GetNamespace(), app.GetName()), fmt.Sprintf("%s/%s", app.GetNamespace(), app.GetName()),
"", "",
) )

err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Delete(app.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}

err = shipperClient.ShipperV1alpha1().RolloutBlocks(ns.GetName()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
} }


func TestNewGlobalRolloutBlockRemoveRelease(t *testing.T) { func TestNewGlobalRolloutBlockRemoveRelease(t *testing.T) {
Expand Down Expand Up @@ -1386,6 +1304,13 @@ func TestNewGlobalRolloutBlockRemoveRelease(t *testing.T) {
t.Fatalf("could not refetch rollout block: %q", err) t.Fatalf("could not refetch rollout block: %q", err)
} }


defer func() {
err = shipperClient.ShipperV1alpha1().RolloutBlocks(rb.GetNamespace()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}
}()

if len(rb.Status.Overrides.Application) > 0 || len(rb.Status.Overrides.Release) > 0 { if len(rb.Status.Overrides.Application) > 0 || len(rb.Status.Overrides.Release) > 0 {
t.Fatalf("rollout block has unexpected overrides: %v", rb) t.Fatalf("rollout block has unexpected overrides: %v", rb)
} }
Expand Down Expand Up @@ -1436,16 +1361,6 @@ func TestNewGlobalRolloutBlockRemoveRelease(t *testing.T) {
fmt.Sprintf("%s/%s", app.GetNamespace(), app.GetName()), fmt.Sprintf("%s/%s", app.GetNamespace(), app.GetName()),
"", "",
) )

err = shipperClient.ShipperV1alpha1().RolloutBlocks(rb.GetNamespace()).Delete(rb.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE rollout block %q: %q", rolloutBlockName, err)
}

err = shipperClient.ShipperV1alpha1().Applications(testNamespace).Delete(app.GetName(), &metav1.DeleteOptions{})
if err != nil {
t.Fatalf("could not DELETE application %q: %q", appName, err)
}
} }


// TODO(btyler): cover a variety of broken chart cases as soon as we report // TODO(btyler): cover a variety of broken chart cases as soon as we report
Expand Down

0 comments on commit 1e02648

Please sign in to comment.