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

Cleanup upgrade code #3260

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

nunnatsa
Copy link
Collaborator

@nunnatsa nunnatsa commented Jan 5, 2025

What this PR does / why we need it

This PR clean up the upgrade handling code

  • Remove the old CRD remover. This code only handled the v2v CRDs, that were already removed in a few releases. It is now safe to drop this code.
  • Drop the code that removes version 1valpha1 from HCO API. We removed it in many versions already and it is now safe to remove this code.
  • Move the upgradepatches related code to its own package, and refactor to achieve better maintenance and more testable code. Improve the initiation code to get fast fail if the file is corrupted or wrong.

Jira Ticket

None

Release note

None

Drop all the code that deals with the removal of v2v CRDs and resources
during upgrade. After a few versions this code is running on upgrade, we
can be sure there is no installation of v2v we need to handle anymore.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
The v1aplpha1 API version of HCO was removed many versions back, and we
can be sure that by now, it's not exist anymore. The code that removes
this version from the CRD on upgrade is not needed.

This commit droped the code that removes the v1alpha1 API version for
the CRD on upgrade.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
Drop two cases of conditions that are knowns and shouldn't be a
condtions.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
This will ease the maintenace of the upgrade related code and will
cleanup the hyperconverged controller code.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
The crwriter tool generates the HyperConverged CR with its default
values, in one of three formats: yaml (default), json, or go file that
contains a json string.

The tool was added in order to auto generate a file that HCO will use to
validate the upgradePatches.json file on boot, even if the
HyperConverged CR is not deployed yet.

It can be used in other places, like replacing the code in the
manifest-templator tool that generates the deploy/hco.cr.yaml, to be
more explicit, or to deploy the HyperConverged CR on hack/deploy.sh.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
HCO validates the upgradePatches.json file only after the HyperConverged
CR is created, and if it's not valid, HCO exit with error. This is too
late to kill the HCO pod. The pod should be killed on boot, if something
is not right. The reason for this behavior was that HCO need the
HyperConverged CR in order to run the patches on it.

This commit generates a json file of the HyperConverged CR, using the
new crwriter tool, and then use it to validate the upgradePathes.json
file, without reading the actual CR, but use the pre-generated json
file.

Then, this commit moves the validation code to boot.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
@kubevirt-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

1 similar comment
Copy link

openshift-ci bot commented Jan 5, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jan 5, 2025
@kubevirt-bot
Copy link
Contributor

[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 ask for approval from nunnatsa. 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:

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

@nunnatsa nunnatsa marked this pull request as ready for review January 5, 2025 11:44
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 5, 2025
@kubevirt-bot kubevirt-bot requested a review from orenc1 January 5, 2025 11:45
Move the actual upgrade json patch code, from the hyperconverged
controller, to the upgradepatch package, for bettr handling and to
improve testability.

Pre-generate the semver range function on boot, instead of re generate
them each time we are trying to apply the patches.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
Instead of manually use a boolean, use the standard library sync.Once to
only read the upgradePatches.json file once.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
@coveralls
Copy link
Collaborator

coveralls commented Jan 5, 2025

Pull Request Test Coverage Report for Build 12628265283

Details

  • 121 of 157 (77.07%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 71.508%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/upgradepatch/upgradePatches.go 113 149 75.84%
Files with Coverage Reduction New Missed Lines %
controllers/hyperconverged/hyperconverged_controller.go 2 81.75%
Totals Coverage Status
Change from base Build 12580388467: -0.3%
Covered Lines: 6016
Relevant Lines: 8413

💛 - Coveralls

@hco-bot
Copy link
Collaborator

hco-bot commented Jan 5, 2025

hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

In response to this:

hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

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-sigs/prow repository.

@hco-bot
Copy link
Collaborator

hco-bot commented Jan 5, 2025

hco-e2e-upgrade-prev-operator-sdk-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-aws
hco-e2e-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-sno-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-aws

In response to this:

hco-e2e-upgrade-prev-operator-sdk-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-aws
hco-e2e-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-sno-azure

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-sigs/prow repository.

@hco-bot
Copy link
Collaborator

hco-bot commented Jan 5, 2025

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure

In response to this:

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

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-sigs/prow repository.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
Copy link

sonarqubecloud bot commented Jan 6, 2025

@hco-bot
Copy link
Collaborator

hco-bot commented Jan 6, 2025

hco-e2e-upgrade-prev-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure
hco-e2e-upgrade-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure

In response to this:

hco-e2e-upgrade-prev-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure
hco-e2e-upgrade-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure

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-sigs/prow repository.

@hco-bot
Copy link
Collaborator

hco-bot commented Jan 6, 2025

hco-e2e-operator-sdk-gcp lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-aws
hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-aws, ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

In response to this:

hco-e2e-operator-sdk-gcp lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-aws
hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

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-sigs/prow repository.

@hco-bot
Copy link
Collaborator

hco-bot commented Jan 6, 2025

hco-e2e-upgrade-prev-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure

In response to this:

hco-e2e-upgrade-prev-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure

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-sigs/prow repository.

@hco-bot
Copy link
Collaborator

hco-bot commented Jan 6, 2025

hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

In response to this:

hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

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-sigs/prow repository.

Copy link

openshift-ci bot commented Jan 6, 2025

@nunnatsa: 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
ci/prow/hco-e2e-operator-sdk-aws 671419d link true /test hco-e2e-operator-sdk-aws
ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure 671419d link false /test hco-e2e-upgrade-prev-operator-sdk-sno-azure
ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure 671419d link true /test hco-e2e-upgrade-prev-operator-sdk-azure
ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure 671419d link false /test hco-e2e-upgrade-operator-sdk-sno-azure

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note-none Denotes a PR that doesn't merit a release note. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants