Refactor to enable local storage #1313
Conversation
|
Is this change still only for assisted deployments?, because dev-scripts/assisted_deployment.sh Line 303 in 41bafce |
The point of this PR is to make it possible to enable local storage on a dev-scripts deployed cluster independent of the assisted stuff - so someone can run |
|
That makes sense, thanks @hardys |
|
/test e2e-metal-ipi-serial-ovn-ipv6 |
hardys
force-pushed
the
local_storage
branch
from
0676fde to
93b33cd
Compare
Refactor code from assisted_deployment.sh to enable_local_storage.sh so that the local-storage operator can be enabled for deployed clusters independent of the other assisted dependencies. This also now works for pre-release builds by installing LSO from source.
Currently this is wrong, since the primary disk shows up as sda due to using virtio-scsi, but then the single additional disk uses virtio and shows up as vda, not vdb.
hardys
force-pushed
the
local_storage
branch
from
93b33cd to
75a147b
Compare
|
/test e2e-metal-ipi-serial-ovn-ipv6 |
|
/test e2e-metal-ipi-ovn-ipv6 |
|
/test e2e-metal-ipi-serial-ovn-ipv6 |
|
/test e2e-metal-ipi-serial-ovn-ipv6 |
1 similar comment
|
/test e2e-metal-ipi-serial-ovn-ipv6 |
|
/override ci/prow/e2e-metal-ipi-serial-ovn-ipv6 |
|
@hardys: Overrode contexts on behalf of hardys: ci/prow/e2e-metal-ipi-serial-ovn-ipv6 In response to this:
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. |
|
Given that the patch seems ok, wouldn't be recommended to have an assisted related job in the CI dev-scripts repo just to ensure that the change is not breaking anything? |
| oc project openshift-local-storage | ||
| LSO_PATH=${LOCAL_STORAGE_OPERATOR_PATH:-$GOPATH/src/github.com/openshift/local-storage-operator} | ||
| if [ ! -d $LSO_PATH ]; then | ||
| echo "Did not find $LSO_PATH" 1>&2 |
There was a problem hiding this comment.
Follow-up idea: just clone or go get it?
There was a problem hiding this comment.
Yeah good idea, cloning when not present similar to other repos seems like the most consistent option
I checked with @flaper87 and he said the script I'm changing here isn't used in AI CI (which presumably explains why we don't have any gating coverage) - IIUC https://github.com/openshift/assisted-test-infra is used instead, but I admit I'm not at all familiar with exactly how the AI CI jobs are implemented |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dtantsur The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
There are some CI tests that have a dev-scripts base for AI. I'll look into it today |
|
/retest-required |
|
/test e2e-metal-ipi-serial-ipv4 |
|
/retest-required |
2 similar comments
|
/retest-required |
|
/retest-required |
|
/test e2e-metal-ipi-ovn-ipv6 |
1 similar comment
|
/test e2e-metal-ipi-ovn-ipv6 |
|
/retest-required |
|
/retest |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/override ci/prow/e2e-metal-ipi-proxy-ipv6 |
|
@hardys: Overrode contexts on behalf of hardys: ci/prow/e2e-metal-ipi-proxy-ipv4, ci/prow/e2e-metal-ipi-proxy-ipv6 In response to this:
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. |
|
/test e2e-metal-ipi |
|
@hardys: The following tests failed, say
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. |
Refactor code from assisted_deployment.sh to enable_local_storage.sh
so that the local-storage operator can be enabled for deployed clusters
independent of the other assisted dependencies.
This also now works for CI/nightly builds, not only GA by installing the LSO from source.
Also fix the default for
VM_EXTRADISKS_LIST- this is wrong so just specifyingVM_EXTRADISKS=truedoesn't work with this script (the operator ends up looking forvdbbut the disk actually shows up asvda