🌱 Separate upgrade plan from upgrade test

[jackjii79]

What this PR does / why we need it:

Generalize upgrade test procedure by extracting common procedure as independent flow to be reusable for more complex upgrading test scenarios.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1626

[metal3-io-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zaneb for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[metal3-io-bot]

Hi @jackjii79. Thanks for your PR.

I'm waiting for a metal3-io 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.

[metal3-io-bot]

@jackjii79: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test metal3-bmo-e2e-test-pull
/test metal3-bmo-e2e-test-optional-pull

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.

[tuminoid]

/ok-to-test
/cc @lentzi90

[lentzi90]

Thanks for working on this!

I see many great things here, but I do think it goes a bit too far in what can be changed in the test. If I understand this correctly, it would be possible to even swap out the BMOUpgradePlanner to some custom implementation that could do the upgrade in a completely different way.
What I would like to see is that this part (what BMOUpgradePlanner does) would be the test spec itself, similar to this in CAPI. It would then be called like this.

There seems to be some issue with the code currently. Could you take a look at why the E2E fixture test failed and try to fix that?

[jackjii79]

like this

Thanks for working on this!

I see many great things here, but I do think it goes a bit too far in what can be changed in the test. If I understand this correctly, it would be possible to even swap out the BMOUpgradePlanner to some custom implementation that could do the upgrade in a completely different way. What I would like to see is that this part (what BMOUpgradePlanner does) would be the test spec itself, similar to this in CAPI. It would then be called like this.

There seems to be some issue with the code currently. Could you take a look at why the E2E fixture test failed and try to fix that?

thanks @lentzi90 to the hint, i get a much clearer picture now, will make some updates

[jackjii79]

@lentzi90 refactor is ready to review, however i have some problem setting up the minikube with kvm driver in my local environment to test, so i do not have a chance to run the e2e in my local to give it a shot in advance. The make test-e2e is also not easy to use in my local as some testing logic is trying to post requests to the service and i do not know which service/port might need port forward before the e2e even started.

[jackjii79]

@lentzi90 Should we proceed with this PR ?

[metal3-io-bot]

PR needs rebase.

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.

[lentzi90]

Sorry for the delay. I have a lot of PRs to review...
If you could rebase this I would be happy to take another look!

[metal3-io-bot]

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

Test name	Commit	Details	Required	Rerun command
unit	653f349	link	true	/test unit
manifestlint	653f349	link	true	/test manifestlint
gomod	653f349	link	true	/test gomod
generate	653f349	link	true	/test generate

Full PR test history. Your PR dashboard.

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.

[metal3-io-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale