-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Allow hosts with hyphen in name #6529
Allow hosts with hyphen in name #6529
Conversation
confirmed this PR works fine on my local env. /lgtm |
@@ -238,7 +238,7 @@ def ips(start_address, end_address): | |||
return [ip_address(ip).exploded for ip in range(start, end + 1)] | |||
|
|||
for host in hosts: | |||
if '-' in host and not host.startswith('-'): | |||
if '-' in host and not (host.startswith('-') or host[0].isalpha()): |
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.
Took me some time to figure out that if the first char is a letter, it has to be a host, not a range. This is okay
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floryut, mattymo 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 |
* 'master' of https://github.com/kubernetes-sigs/kubespray: Allow hosts with hyphen in name (kubernetes-sigs#6529) Update apiserver-audit-policy.yaml.j2 (kubernetes-sigs#6526) add master_volume_type variable (kubernetes-sigs#6524) Remove unused variable (kubernetes-sigs#6522) Add new cilium options for native routing (kubernetes-sigs#6519) Fixed Kubespray container-engine/docker role to populate docker.service (kubernetes-sigs#6518) Fix cilium_deploy_additionally with kubeadm etcd (kubernetes-sigs#6514) improve Cilium metrics support (kubernetes-sigs#6513)
What type of PR is this?
/kind bug
What this PR does / why we need it:
Hostname with hyphen should be allowed
Which issue(s) this PR fixes:
Fixes #6509
Special notes for your reviewer:
Tests ok
Does this PR introduce a user-facing change?: