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

API groups e2e tests remove controllers #3564

Merged

Conversation

codegold79
Copy link
Contributor

@codegold79 codegold79 commented Mar 11, 2021

Summary of changes

  • removed controllers and webhooks unnecessary for testing API group version feature
  • replaced many of the sleeps with --wait and other more deterministic blocking methods

Does your change fix a particular issue?

Fixes #3532

Please indicate you've done the following:

@codegold79
Copy link
Contributor Author

/kind changelog-not-required

@github-actions github-actions bot added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Mar 11, 2021
@codegold79 codegold79 changed the title Api groups e2e tests remove controllers API groups e2e tests remove controllers Mar 11, 2021
@codegold79
Copy link
Contributor Author

@ashish-amarnath I have reason to believe a bug was introduced by Velero recently somewhere.
For my API Group Versions e2e tests, I created two images: current one (should be the same as main as of today, March 11, 2021) and an older one from when I first published my API Groups PR (published Feb 4, 2021). The older image consistently passes. The newest image always fails. I’m currently on the search to identify the bug.

@codegold79
Copy link
Contributor Author

codegold79 commented Mar 12, 2021

These tests currently do not pass because PR #3125 removed API group version feature changes. PR #3571 bug issue was created to address the issue. In the meantime, in order to get tests to pass, build a local Velero version at a commit that is before the merge of PR #3125.

@codegold79 codegold79 marked this pull request as ready for review March 12, 2021 16:51
@codegold79 codegold79 mentioned this pull request Mar 12, 2021
@dsu-igeek dsu-igeek added this to In progress in v1.6.0 via automation Mar 13, 2021
@dsu-igeek dsu-igeek added this to the v1.6.0 milestone Mar 13, 2021
@dsu-igeek dsu-igeek added this to In progress in v1.7.0 via automation Mar 13, 2021
@dsu-igeek dsu-igeek removed this from In progress in v1.6.0 Mar 13, 2021
@dsu-igeek dsu-igeek modified the milestones: v1.6.0, v1.7.0 Mar 13, 2021
@codegold79 codegold79 marked this pull request as draft March 25, 2021 23:31
@codegold79
Copy link
Contributor Author

I put this PR in DRAFT status as it has merge conflicts.

@codegold79 codegold79 marked this pull request as ready for review April 1, 2021 18:36
@codegold79 codegold79 marked this pull request as draft April 1, 2021 18:37
@carlisia carlisia removed the request for review from jenting April 15, 2021 22:28
@dsu-igeek dsu-igeek removed this from In progress in v1.7.0 Apr 21, 2021
@dsu-igeek dsu-igeek added this to In progress in Velero 1.7.0 via automation Apr 21, 2021
@codegold79 codegold79 force-pushed the api-groups-e2e-tests-remove-controllers branch from 331fc4f to 78ac41a Compare April 27, 2021 21:14
@codegold79 codegold79 marked this pull request as ready for review April 27, 2021 21:30
Signed-off-by: F. Gold <fgold@vmware.com>
@codegold79 codegold79 force-pushed the api-groups-e2e-tests-remove-controllers branch from 78ac41a to 40cd94b Compare June 3, 2021 20:28
@codegold79
Copy link
Contributor Author

@dsu-igeek @zubron, this PR is again ready for review after I fixed merge conflicts introduced when PR #3726 was merged.

Copy link
Contributor

@dsu-igeek dsu-igeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of small changes, thanks!

test/e2e/enable_api_group_versions_test.go Outdated Show resolved Hide resolved
Signed-off-by: F. Gold <fgold@vmware.com>
@codegold79
Copy link
Contributor Author

Looks like merge conflicts were introduced with PR #3764 less than a week ago. I'll have to address these.

Signed-off-by: F. Gold <fgold@vmware.com>
Signed-off-by: F. Gold <fgold@vmware.com>
@codegold79 codegold79 force-pushed the api-groups-e2e-tests-remove-controllers branch from cc2d736 to caa4529 Compare June 15, 2021 17:23
Signed-off-by: F. Gold <fgold@vmware.com>
@carlisia carlisia self-requested a review June 15, 2021 18:11
Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR lgtm, it's basically a removal of a bunch of resources. The failure I got was about generating the data, maybe a timeout.

Next: I'm going to run what we have on the main branch and see if that passes.

@codegold79
Copy link
Contributor Author

codegold79 commented Jun 15, 2021

This PR lgtm, it's basically a removal of a bunch of resources. The failure I got was about generating the data, maybe a timeout.

Next: I'm going to run what we have on the main branch and see if that passes.

Thanks @carlisia for running the tests. I'll stand by and see if I can make any changes to help get the tests to pass.

@carlisia
Copy link
Contributor

@codegold79
Copy link
Contributor Author

@codegold79 it got worse: https://snippets.cacher.io/snippet/137ff753cef67263363a

Oh no! Kibishii tests are really struggling :(

@carlisia carlisia removed the request for review from ashish-amarnath June 17, 2021 06:54
Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests run successfully on AWS.

All the chocking reported previously was mostly due to race conditions from running all tests together and from all tests installing Velero in the same namespace.

Note: I fixed the race conditions (🎉) and will open a PR for that tomorrow. No changes were made to this test after the race conditions and they pass successfully.

Velero 1.7.0 automation moved this from In progress to Reviewer approved Jun 17, 2021
Copy link
Contributor

@dsu-igeek dsu-igeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dsu-igeek dsu-igeek merged commit c21b661 into vmware-tanzu:main Jun 17, 2021
Velero 1.7.0 automation moved this from Reviewer approved to Done Jun 17, 2021
zubron pushed a commit to zubron/velero that referenced this pull request Jun 21, 2021
* Remove controllers and sleeps in API groups e2e tests

Signed-off-by: F. Gold <fgold@vmware.com>

* Print command in AfterEach(...) and check error

Signed-off-by: F. Gold <fgold@vmware.com>

* Make change ahead of PR3764 changes in main

Signed-off-by: F. Gold <fgold@vmware.com>

* Update go.{mod,sum} files

Signed-off-by: F. Gold <fgold@vmware.com>

* Run make update

Signed-off-by: F. Gold <fgold@vmware.com>
ywk253100 pushed a commit to ywk253100/velero that referenced this pull request Jun 29, 2021
* Remove controllers and sleeps in API groups e2e tests

Signed-off-by: F. Gold <fgold@vmware.com>

* Print command in AfterEach(...) and check error

Signed-off-by: F. Gold <fgold@vmware.com>

* Make change ahead of PR3764 changes in main

Signed-off-by: F. Gold <fgold@vmware.com>

* Update go.{mod,sum} files

Signed-off-by: F. Gold <fgold@vmware.com>

* Run make update

Signed-off-by: F. Gold <fgold@vmware.com>
gyaozhou pushed a commit to gyaozhou/velero-read that referenced this pull request May 14, 2022
* Remove controllers and sleeps in API groups e2e tests

Signed-off-by: F. Gold <fgold@vmware.com>

* Print command in AfterEach(...) and check error

Signed-off-by: F. Gold <fgold@vmware.com>

* Make change ahead of PR3764 changes in main

Signed-off-by: F. Gold <fgold@vmware.com>

* Update go.{mod,sum} files

Signed-off-by: F. Gold <fgold@vmware.com>

* Run make update

Signed-off-by: F. Gold <fgold@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file has-e2e-tests kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Flaky e2e tests
4 participants