Adding support for docker-storage-setup on overlay #6524
Conversation
openshift-ci-robot
added
the
size/M
| group: root | ||
| mode: 0664 | ||
| when: | ||
| - container_runtime_run_docker_storage_setup | default(False) |
There was a problem hiding this comment.
get rid of the bool, let's check the variable that sets to storage type.
There was a problem hiding this comment.
This doesn't really give us a mechanism to turn on or off the storage setup, especially if we default the variable to something.
container_runtime_docker_storage_setup_type=overlay
Should we not specify the variable and then it will not be defined? Or default it to ''?
| mode: 0664 | ||
| when: | ||
| - container_runtime_run_docker_storage_setup | default(False) | ||
| - container_runtime_docker_storage_driver == 'overlay2' |
There was a problem hiding this comment.
I think we should change this from _driver to _type. This will allow us greater flexibility to control more types of storage scenarios. For instance, if a storage driver can be used in a variety of ways (overlay2 on top of gluster) or what have you.
| CONTAINER_ROOT_LV_NAME="docker-root-lv" | ||
| CONTAINER_ROOT_LV_SIZE="100%FREE" | ||
| CONTAINER_ROOT_LV_MOUNT_PATH="{{ docker_storage_path }}" | ||
| EXTRA_STORAGE_OPTIONS="--storage-opt overlay2.override_kernel_check=true --storage-opt overlay2.size={{ docker_storage_size }} --graph={{ docker_storage_path}} " |
There was a problem hiding this comment.
We should try to make these all variables instead of hard-coding.
extra_storage_options should take a list of options.
eso:
- '--storage-opt overlay2.override_kernel_check=true'
- "--graph={{ docker_storage_path}}"
This will be easier for the user to override, and we don't have to implement any fancy logic other than joining the list items.
|
Functionality looks fine. I'll defer to @michaelgugino on naming/etc... However, some of this looks to be similar to #5216 |
kwoodson
force-pushed
the
docker_storage_setup_overlay
branch
2 times, most recently
from
9221cf1
to
69c0417
Compare
|
/retest |
These seem highly unrelated. |
kwoodson
force-pushed
the
docker_storage_setup_overlay
branch
from
69c0417
to
2b22e81
Compare
|
/retest |
kwoodson
force-pushed
the
docker_storage_setup_overlay
branch
from
2b22e81
to
5c6af56
Compare
|
@sdodson will require a cherry pick once merged. |
|
/lgtm |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
@kwoodson: The following tests failed, say
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. |
|
flakes |
|
/cherrypick release-3.8 |
|
@kwoodson: failed to push cherry-picked changes in Github: exit status 1 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. |
No description provided.