-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add OpenStack provider #540
Conversation
cmd/template/cluster/provider/templates/openstack/cluster.yaml.tmpl
Outdated
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/open_stack_machine_template.yaml.tmpl
Outdated
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/open_stack_machine_template.yaml.tmpl
Outdated
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/open_stack_machine_template.yaml.tmpl
Outdated
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/kubeadm_control_plane.yaml.tmpl
Outdated
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/kubeadm_control_plane.yaml.tmpl
Outdated
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/open_stack_machine_template.yaml.tmpl
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/open_stack_cluster.yaml.tmpl
Outdated
Show resolved
Hide resolved
cmd/template/cluster/provider/templates/openstack/open_stack_machine_template.yaml.tmpl
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
What could we do? Can me make the OpenStack related flags hidden, as they are meant to be used by a limited audience only (and may be removed in the future anyway)? This way we wouldn't have negative side effects for people using AWS or Azure only. But that would make the flags harder to use for those few who actually need them. Can we structure the help output in a way that OpenStack-related flags form their own group? That would actually be a big improvement for every user. |
@marians I don't really like the idea of hiding them because I want OpenStack customers to be supported just like AWS and Azure customers. One option would be to add a subcommand under Another option would be to parse the We could also write our own help message instead of using the automatically generated message. One last option is to keep it like this until What do you think @giantswarm/sig-ux? |
For reference, here is the upstream issue: spf13/cobra#1327 I can't find a PR to close that issue. The linked PR 1003 implements grouping of commands, not flags. I would suggest to override the flags help rendering then with our own "hard-coded" string. |
@@ -27,7 +27,20 @@ type ClusterCRsConfig struct { | |||
EKS bool | |||
ExternalSNAT bool | |||
ControlPlaneSubnet string | |||
PodsCIDR string | |||
|
|||
// OpenStack only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all of these options really unique per workload cluster?
Wouldn't it make way more sense to default a bunch of these (e.g. SSHKeyName, ImageName, ...) in the webhook as they are tied to the management cluster?
Additionally we know that customers want to use gitops to manage these clusters, supplying them with good overlays seems more sensible to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dropped SSH key name because we don't want to allow it. Image name will be decided by us (we're managing images), but is dependent on Kubernetes version and may be useful to override. Source UUID is tricky because it depends on the OpenStack project the cluster is deployed into and that is therefore cluster-dependent.
For the alpha (or "pre-alpha") we just want the ability to specify all of the options. The flags won't show up in the automatic CLI help so I don't think it will be a problem in terms of backwards compatibility. Let me know if you disagree @MarcelMue.
cmd/template/cluster/provider/templates/openstack/machine_deployment.yaml.tmpl
Show resolved
Hide resolved
Regarding the general UX concerns, we just discussed that in SIG UX. That should not block this PR in any way. I created this issue to tackle the necessary improvements independently: |
After a team discussion, we decided to simply hide all of the provider-specific flags until we have a better solution. |
^ sorry for the wrong commit - got mixed up with my CLI. |
For https://github.com/giantswarm/giantswarm/issues/19400
Adds support for templating OpenStack clusters.
Adds the following OpenStack-specific flags:
--cloud
--cloud-config
--dns-nameservers
--external-network-id
--failure-domain
--image-name
--node-machine-flavor
--node-cidr
--root-volume-disk-size
--root-volume-source-type
--root-volume-source-uuid
Some of these flags will be dropped in favor of defaulting. Others will become flags common for all providers. For now we need the configurability for testing OpenStack alpha releases.