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

Transition KubeVirt platform to using persistent storage for root volume #1298

Closed

Conversation

davidvossel
Copy link
Contributor

This PR transitions the KubeVirt platform away from using ephemeral storage and towards using persistent storage. Persistent storage allows the nodes hosted in KubeVirt VMs to be restarted/recovered without losing state.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 21, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: davidvossel
To complete the pull request process, please assign sjenning after the PR has been reviewed.
You can assign the PR to them by writing /assign @sjenning 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

@davidvossel
Copy link
Contributor Author

@rmohr I added you as a co-author to this commit. This is a stripped down version of your pr #994 that doesn't auto-detect the rhcos image (since that's still incomplete). I'm getting asked to start supporting persistent storage, so it looks like we need this step a little earlier than the auto detection of rhcos.

@rmohr
Copy link
Contributor

rmohr commented Apr 22, 2022

/lgtm

looks great.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2022
@davidvossel
Copy link
Contributor Author

/retest

This should pass now that openshift/release#28041 has merged

@davidvossel davidvossel force-pushed the kubevirt-persistent-storage-v1 branch from 79474fa to 9be683e Compare April 22, 2022 14:19
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2022
@davidvossel
Copy link
Contributor Author

I had to update the PR to fix a go.mod issue that caused a validation check to fail.

@davidvossel davidvossel force-pushed the kubevirt-persistent-storage-v1 branch from 9be683e to 37419fb Compare April 22, 2022 17:37
@davidvossel
Copy link
Contributor Author

e2e tests failed because I didn't properly set the volume size in the e2e test function.

@davidvossel davidvossel force-pushed the kubevirt-persistent-storage-v1 branch from 37419fb to 386074a Compare April 22, 2022 18:14
@davidvossel
Copy link
Contributor Author

ci passes now

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2022
@rmohr
Copy link
Contributor

rmohr commented Apr 25, 2022

/lgtm

but seems to need another rebase.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 25, 2022
@davidvossel davidvossel force-pushed the kubevirt-persistent-storage-v1 branch from 386074a to 5722e18 Compare April 25, 2022 14:07
@openshift-ci openshift-ci bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 25, 2022
@rmohr
Copy link
Contributor

rmohr commented Apr 25, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 25, 2022
Signed-off-by: David Vossel <davidvossel@gmail.com>

Co-authored-by: rmohr <rmohr@redhat.com>
@davidvossel davidvossel force-pushed the kubevirt-persistent-storage-v1 branch from 5722e18 to 6a365d8 Compare April 26, 2022 18:27
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 26, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 26, 2022

New changes are detected. LGTM label has been removed.

@davidvossel
Copy link
Contributor Author

/hold

I might close this in favor of some new work that encompasses this plus a revised nodepool kubevirt platform api

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 26, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 26, 2022

@davidvossel: 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/e2e-kubevirt-gcp 6a365d8 link false /test e2e-kubevirt-gcp
ci/prow/capi-provider-agent-sanity 6a365d8 link false /test capi-provider-agent-sanity

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.

@davidvossel
Copy link
Contributor Author

closed in favor of #1314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants