Add validation and choices to OpenStack installconfig #766
Conversation
openshift-ci-robot
added
approved
|
@russellb FYI this cleans up a couple of FIXME/TODO's you added, it's the start of a cleanup to add all the missing validations for the different inputs we added. @flaper87 and @tomassedovic review welcome if you get a moment, thanks! :) |
hardys
force-pushed
the
openstack_validation
branch
from
0ad5b29 to
e444fe4
Compare
Add validation based on the clouds available in the clouds.yaml. Since we have the list of valid names we can also enable selection of the name when specified interactively.
hardys
force-pushed
the
openstack_validation
branch
from
e444fe4 to
24507b2
Compare
openshift-ci-robot
added
size/XL
hardys
changed the title
hardys
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
/assign |
openshift-ci-robot
added
size/XXL
This checks the available regions and validates the user input, and enables selecting from a list in the interactive case
This checks the available images and validates the user input, and enables selecting from a list in the interactive case
This checks the available networks and validates the user input, and enables selecting from a list in the interactive case
hardys
force-pushed
the
openstack_validation
branch
from
d749a7b to
932b417
Compare
hardys
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
Ok I think this is ready for review now - note that the existing limitation that these interfaces require names not IDs exist (this appears to be due to terraform but I've not fully investigated), so I clarified that in some of the help strings. I pushed as a series of small commits but can squash if folks prefer that. |
| i := 0 | ||
| cloudNames := make([]string, len(clouds)) | ||
| for k := range clouds { | ||
| cloudNames[i] = k |
There was a problem hiding this comment.
Why not just use append instead of keeping with an index?
There was a problem hiding this comment.
Yeah we could do it either way, I read that this approach is slightly more efficient because you don't have to do reallocations, but I'm fine with either. Honestly I'm amazed there's no native .keys() equivalent and I just found this approach while looking for a go alternative.
|
/test e2e-aws |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flaper87, hardys The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@hardys: The following test 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. |
5 participants
Add validation based on the clouds available in the clouds.yaml and related validation for the other options (most of which can be validated by doing an API call to e.g list the regions, images or whatever)
Since we have the list of valid names we can also enable selection of the name when specified interactively.