New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add e2e tests for Finalizer #833

Merged

k8s-ci-robot merged 1 commit into kubernetes:master from skmatti:e2e-finalizer

Aug 28, 2019

Contributor

skmatti commented Aug 23, 2019

This adds the following e2e tests for Finalizer

Ingress creation and deletion.
Create Ingress, change Ingress class and delete Ingress.

k8s-ci-robot added cncf-cla: yes needs-ok-to-test labels

Contributor

k8s-ci-robot commented Aug 23, 2019

Hi @skmatti. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot requested review from freehan and MrHohn

August 23, 2019 00:33

k8s-ci-robot added the size/L label

Contributor Author

skmatti commented Aug 23, 2019

/assign mrhohn

k8s-ci-robot assigned MrHohn

Member

MrHohn commented Aug 23, 2019

/ok-to-test

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels

MrHohn reviewed

View reviewed changes

cmd/e2e-test/finalizer_test.go Outdated Show resolved Hide resolved

cmd/e2e-test/finalizer_test.go Outdated Show resolved Hide resolved

cmd/e2e-test/finalizer_test.go Outdated

    
              		if err != nil {

              			t.Fatalf("error waiting for Ingress to stabilize: %v", err)

              		}

              		t.Logf("GCLB resources createdd (%s/%s)", s.Namespace, ing.Name)

Member

MrHohn Aug 23, 2019

nit: created

cmd/e2e-test/finalizer_test.go Outdated Show resolved Hide resolved

pkg/e2e/helpers.go Outdated Show resolved Hide resolved

cmd/e2e-test/finalizer_test.go Outdated Show resolved Hide resolved

skmatti force-pushed the e2e-finalizer branch from 8546d8b to 2367719 Compare

August 23, 2019 16:35


          Add e2e tests for Finalizer

177b806

skmatti force-pushed the e2e-finalizer branch from 2367719 to 177b806 Compare

August 23, 2019 16:37

MrHohn approved these changes

View reviewed changes

Member

MrHohn left a comment

/lgtm
/approve

cmd/e2e-test/finalizer_test.go

+              		}
+              		t.Logf("Finalizer for Ingress (%s/%s) deleted", s.Namespace, ing.Name)
+              		if err := e2e.WaitForIngressDeletion(ctx, gclb, s, ing, deleteOptions); err != nil {

Member

MrHohn Aug 28, 2019

minor: We might not need to wait for deletion one more time (WaitForFinalizerDeletion already does something similar)? The ingress object will be cleaned up along with the namespace deletion at the end by the test framework.

k8s-ci-robot added the lgtm label

Contributor

k8s-ci-robot commented Aug 28, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrHohn, skmatti

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [MrHohn]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the approved label

Member

MrHohn commented Aug 28, 2019

/hold

k8s-ci-robot added the do-not-merge/hold label

Member

MrHohn commented Aug 28, 2019

Will hold a bit until we enable finalizer from in the test manifest.

Member

MrHohn commented Aug 28, 2019

Had some offline discussion with @skmatti, we think it is fine to let this in --- it will not run because of the testcase filter anyway.
/hold cancel

k8s-ci-robot removed the do-not-merge/hold label

k8s-ci-robot merged commit ee19757 into kubernetes:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved cncf-cla: yes lgtm ok-to-test size/L