Support creating OpenStack Cinder registry volume #6713
Conversation
openshift-ci-robot
added
the
size/L
openshift-merge-robot
added
needs-rebase
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@sdodson the CI seems to be stuck at the Red Hat CI and fedora/27/atomic jobs for days. I've tried |
|
@rh-atomic-bot bot, test pull request |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@michaelgugino can I have a review? Please pay special attention to |
|
@tomassedovic: GitHub didn't allow me to request PR reviews from the following users: bogdando, tzumainn. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn 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. |
| # TODO(shadower): create the registry and PV Cinder volumes if specified | ||
| # and include the `prepare-and-format-cinder-volume` tasks to set it up | ||
| - name: Create the Cinder volume for OpenShift Registry | ||
| include_tasks: create-registry-volume.yml |
There was a problem hiding this comment.
Is the prepare-and-format-cinder-volume not a thing any more?
There was a problem hiding this comment.
Nope, turns out it's no longer necessary. OpenShift will format the volumes as needed.
| attached to anything | ||
| 2. After the OpenShift installation finished, run `openstack volume list` again | ||
| * The `Status` of the volume you created should be `in-use` | ||
| * The volume should be attached to an infra node |
There was a problem hiding this comment.
let's note something for the multi-infra nodes deployments
There was a problem hiding this comment.
Doesn't the "an infra node" imply this? The cinder volume will be attached to the infra node running the registry. That's it.
If you think that's not clear enough, I'll try to word it better.
There was a problem hiding this comment.
I meant how should this work, when we have multiple infra nodes provisioned
There was a problem hiding this comment.
Right, okay. In that case you can only have 1 registry pod running on 1 infra node and the volume will be attached to that node. I'll update the docs.
tzumainn
left a comment
tzumainn
left a comment
There was a problem hiding this comment.
Looks good (with one small optional wording fix). I wasn't able to test this to completion due to other issues in my environment, but I did verify that the registry volume was created as expected.
| `ANSIBLE_LOOKUP_PLUGINS=openshift-ansible-contrib/lookup_plugins` environment | ||
| variable. | ||
| 1. After the provisioning phase is complete, run `openstack volume list` | ||
| * A new volume with be created, with the name and size you specified |
|
Okay, finally managed to test this to completion - looks good to me! |
| @@ -0,0 +1,11 @@ | |||
| --- | |||
| - set_fact: | |||
There was a problem hiding this comment.
This set_fact is unnecessary.
If you want to create vars for the sake of readability, just add a 'vars:' at the end of the task where they are used.
| - "{{ openshift_openstack_lb_flavor }}" | ||
| - "{{ openshift_openstack_etcd_flavor }}" | ||
|
|
||
| - block: |
There was a problem hiding this comment.
I think this is not a great way to do this. Why not just use the os_volume module, users can specify volume name and volume id if they choose, that module will idempotently create the volume if necessary.
|
The latest commit should address all comments. |
|
/retest |
|
/test gcp |
1 similar comment
|
/test gcp |
openshift-bot
added
the
needs-rebase
This documents how to use an existing Cinder volume for registry as well as letting you create one automaticaly during provisioning.
openshift-bot
removed
the
needs-rebase
|
/retest |
|
@tzumainn CI looking good, can I have a review? |
|
/lgtm |
|
/retest |
tomassedovic
dismissed
michaelgugino’s stale review
Both comments were addressed.
|
@tomassedovic: 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. DetailsInstructions 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. |
9 participants
This adds an option to have the playbooks create (or re-use previously created) Cinder volume for the registry automatically. The OpetStack documentation is updated correspondingly.