github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 06:54:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #57474 from Random-Liu/fix-aggregator-test

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 57434, 57221, 57417, 57474, 57481). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Cleanup api service before namespace deletion.

Fixes https://github.com/kubernetes/kubernetes/issues/57486.

https://github.com/kubernetes/kubernetes/pull/57254 helps but is not enough for the fix. After https://github.com/kubernetes/kubernetes/pull/57254, I saw another failure https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-cri-containerd-e2e-ubuntu-gce/1057.

Before, all test after Aggregator test would fail. Now all tests before/after Aggregator tests are passing. However, during the Aggregator test waiting for namespace cleanup, all the other tests are failing. Aggregator test takes 10min to wait for namespace cleanup.

The reason is that, `BeforeEach`s run in LIFO order, `AfterEach`s run in FIFO order. `NewDefaultFramework` inserts an `AfterEach` function to do namespace cleanup.

In the current code, the namespace cleanup `AfterEach` is inserted by `NewDefaultFramework` before `cleanupTest`, it means that the test will try to delete namespace first, and then cleanup the api service. However, as known to us, if api service is not cleaned up, namespace deletion will wait forever.

In this PR, we reverse the `AfterEach` order. A better solution is to put `BeforeEach` and `AfterEach` in an internal `Describe`. However, since we don't have an internal `Describe` in this test, I just reverse the `AfterEach` order directly.

/cc @roycaihw @cheftako @kubernetes/sig-api-machinery-bugs 

**Release note**:

```release-note
none
```
This commit is contained in:
Kubernetes Submit Queue 2017-12-20 17:42:43 -08:00 committed by GitHub
commit 369b7a6131
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 12 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
test/e2e/apimachinery/aggregator.go
View File

@ -53,18 +53,26 @@ var _ = SIGDescribe("Aggregator", func() {
var ns string
var c clientset.Interface
var aggrclient *aggregatorclient.Clientset
// BeforeEachs run in LIFO order, AfterEachs run in FIFO order.
// We want cleanTest to happen before the namespace cleanup AfterEach
// inserted by NewDefaultFramework, so we put this AfterEach in front
// of NewDefaultFramework.
AfterEach(func() {
cleanTest(c, aggrclient, ns)
})
f := framework.NewDefaultFramework("aggregator")
// We want namespace initialization BeforeEach inserted by
// NewDefaultFramework to happen before this, so we put this BeforeEach
// after NewDefaultFramework.
BeforeEach(func() {
c = f.ClientSet
ns = f.Namespace.Name
aggrclient = f.AggregatorClient
})
AfterEach(func() {
cleanTest(c, aggrclient, ns)
})
It("Should be able to support the 1.7 Sample API Server using the current Aggregator", func() {
// Make sure the relevant provider supports Agggregator
framework.SkipUnlessServerVersionGTE(serverAggregatorVersion, f.ClientSet.Discovery())