Skip to content
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.

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

e2e framework: replace custom cleanup support with Ginkgo v2 mechanisms #111998

Merged
merged 7 commits into from Sep 9, 2022
Merged

e2e framework: replace custom cleanup support with Ginkgo v2 mechanisms #111998

merged 7 commits into from Sep 9, 2022

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Aug 24, 2022 •
edited

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

In contrast to ginkgo.AfterEach, ginkgo.DeferCleanup runs the callback in
first-in-last-out order. Using it makes the following test code work as
expected:

f := framework.NewDefaultFramework("some test")

ginkgo.AfterEach(func() {
    // do something with f.ClientSet
})

Previously, f.ClientSet was already set to nil by the framework's cleanup code.

Fixes: #111979

Does this PR introduce a user-facing change?

e2e: tests can now register callbacks with ginkgo.BeforeEach/AfterEach/DeferCleanup directly after creating a framework instance and are guaranteed that their code is called after the framework is initialized and before it gets cleaned up. ginkgo.DeferCleanup replaces f.AddAfterEach and AddCleanupAction which got removed to simplify the framework.

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 24, 2022
@k8s-ci-robot
Copy link
Contributor

@pohly: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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 k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 24, 2022
@k8s-ci-robot k8s-ci-robot added area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 24, 2022
@pohly
Copy link
Contributor Author

pohly commented Aug 26, 2022

I'm going to take this into #112043.

/close

@k8s-ci-robot
Copy link
Contributor

@pohly: Closed this PR.

In response to this:

I'm going to take this into #112043.

/close

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.

@pohly
Copy link
Contributor Author

pohly commented Aug 26, 2022

/reopen

I need these changes for #112043 but it might be better to review and merge them separately.

@k8s-ci-robot
Copy link
Contributor

@pohly: Reopened this PR.

In response to this:

/reopen

I need these changes for #112043 but it might be better to review and merge them separately.

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 k8s-ci-robot reopened this Aug 26, 2022
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/storage Categorizes an issue or PR as relevant to SIG Storage. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Aug 26, 2022
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2022
k8s-publishing-bot pushed a commit to kubernetes/api that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
e6114e9 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/client-go that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
ab0bfda 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/component-base that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
4c3fcb5 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/component-helpers that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
87b1367 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/apiserver that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
cf6c663 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/kube-aggregator that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
d529e33 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/sample-apiserver that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
2cf88af 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/sample-controller that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
e28fbce 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/apiextensions-apiserver that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
6b7d12b 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/metrics that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
0451dbf 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/cli-runtime that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
ea15d7a 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/sample-cli-plugin that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
b94a07a 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/kube-proxy that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
9e1a287 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/kubelet that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
892f43e 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/kube-scheduler that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
5152377 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/controller-manager that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
579acb2 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/cloud-provider that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
11f92cf 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/kube-controller-manager that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
6d23d3d 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/cluster-bootstrap that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
12b60f8 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/csi-translation-lib that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
0940ea3 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/mount-utils that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
49dffa2 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/legacy-cloud-providers that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
9539e71 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/kubectl that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
2a43ef2 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
k8s-publishing-bot pushed a commit to kubernetes/pod-security-admission that referenced this pull request Sep 10, 2022
@pohly @k8s-publishing-bot
build: update to klog v2.80.1
7cbae0a 
The fix for kubernetes/klog#348 is required before
kubernetes/kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.

Kubernetes-commit: 60d92dd96a4ba3873dd9e061c3e332c16c213e30
@pohly pohly mentioned this pull request Sep 12, 2022
Closed
@Rajalakshmi-Girish Rajalakshmi-Girish mentioned this pull request Sep 13, 2022
Closed
danielye11 pushed a commit to danielye11/kubernetes that referenced this pull request Sep 21, 2022
@pohly @danielye11
build: update to klog v2.80.1
39135d6 
The fix for kubernetes/klog#348 is required before
kubernetes#111998 can be merged because the
way how a unit test in that PR uses klog triggers the data race.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mauriciopoppe mauriciopoppe mauriciopoppe left review comments

@aojea aojea aojea left review comments

@SataQiu SataQiu SataQiu left review comments

@fejta fejta Awaiting requested review from fejta

Assignees

@aojea aojea

@SataQiu SataQiu

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.26
Development

Successfully merging this pull request may close these issues.

e2e: use DeferCleanup for Framework instance
6 participants
@pohly @k8s-ci-robot @aojea @dims @mauriciopoppe @SataQiu