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.

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

[WIP] Reproduce v1alpha3=>v1beta1 upgrade issue (CR release-0.10) #5421

Closed
wants to merge 1 commit into from
Closed

[WIP] Reproduce v1alpha3=>v1beta1 upgrade issue (CR release-0.10) #5421

wants to merge 1 commit into from

Conversation

sbueringer
Copy link
Member

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Tries to reproduce kubernetes-sigs/cluster-api-provider-aws#2834

With: kubernetes-sigs/controller-runtime#1695

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 13, 2021
@sbueringer
Copy link
Member Author

/retest

@sbueringer
Copy link
Member Author

sbueringer commented Oct 13, 2021 •
edited
Loading

@vincepri

  • pull-cluster-api-e2e-main looks good (it's actually v1alpha3=>v1beta1 upgrade without leader election)
  • pull-cluster-api-e2e-ipv6-main is expected to fail as v1alpha3 does not support IPv6

Not sure why pull-cluster-api-test-main fails. I think it's related to the CR changes as Fabrizio's PR didn't have this problem.

Note: this PR only tests the upgrade and nothing else (to reproduce the issue we had)

@vincepri
Copy link
Member

@sbueringer It's not clear from the logs what's actually returning from the Start() method, because it's running in a goroutine, seems to be failing after the manager is shutting down?

@sbueringer sbueringer force-pushed the pr-repro-upgrade-issues-mgr-refactor branch from 13fcbb5 to f9e2830 Compare October 14, 2021 09:09
@sbueringer
Copy link
Member Author

sbueringer commented Oct 14, 2021 •
edited
Loading

Rebased to latest commit of the CR PR.

All tests are now running with leader election again

/test pull-cluster-api-e2e-workload-upgrade-1-22-latest-main
/test pull-cluster-api-e2e-full-main

@sbueringer sbueringer force-pushed the pr-repro-upgrade-issues-mgr-refactor branch from f9e2830 to 4eb2da6 Compare October 14, 2021 09:34
@sbueringer
Copy link
Member Author

/test pull-cluster-api-e2e-full-main

@sbueringer
Copy link
Member Author

And now all without leader election:
/test pull-cluster-api-e2e-full-main

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2021
@sbueringer sbueringer force-pushed the pr-repro-upgrade-issues-mgr-refactor branch from 97b4a5f to 2ee4cd1 Compare November 3, 2021 17:08
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign justinsb after the PR has been reviewed.
You can assign the PR to them by writing /assign @justinsb in a comment when ready.

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

Needs approval from an approver in each of these files:

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
Copy link
Contributor

k8s-ci-robot commented Nov 3, 2021 •
edited
Loading

@sbueringer: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-e2e-workload-upgrade-1-22-latest-main f9e2830 link false /test pull-cluster-api-e2e-workload-upgrade-1-22-latest-main

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@sbueringer sbueringer force-pushed the pr-repro-upgrade-issues-mgr-refactor branch from 2ee4cd1 to e5e4984 Compare November 3, 2021 17:13
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2021
@sbueringer
Copy link
Member Author

/test pull-cluster-api-e2e-full-main

@sbueringer sbueringer force-pushed the pr-repro-upgrade-issues-mgr-refactor branch from e5e4984 to 4bdada9 Compare November 3, 2021 17:15
@sbueringer
Copy link
Member Author

/test pull-cluster-api-e2e-full-main

@sbueringer sbueringer mentioned this pull request Nov 3, 2021
Merged
@sbueringer sbueringer changed the title [WIP] Reproduce v1alpha3=>v1beta1 upgrade issue (mgr refactor) [WIP] Reproduce v1alpha3=>v1beta1 upgrade issue (CR release-0.10) Nov 5, 2021
@sbueringer sbueringer force-pushed the pr-repro-upgrade-issues-mgr-refactor branch from 4bdada9 to 4ab91bd Compare November 5, 2021 09:34
@sbueringer
Copy link
Member Author

Testing kubernetes-sigs/controller-runtime#1690 now

/test pull-cluster-api-e2e-full-main

@sbueringer sbueringer mentioned this pull request Nov 5, 2021
Merged
@sbueringer
Copy link
Member Author

/close
Bug has been fixed in CR main and release-0.10.

@sbueringer sbueringer closed this Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enxebre enxebre Awaiting requested review from enxebre

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sbueringer @vincepri @k8s-ci-robot