[upi][vsphere] applicable static IP changes #1480
Conversation
dav1x
commented
Mar 27, 2019
dav1x
commented
- Adds network module to allocate static IP addresses from machine_cidr
- Adds ignition config to set static IP address.
- Add systemd unit file to reboot the VM on firstboot when idle to apply static IP.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dav1x If they are not already assigned, you can assign the PR to them by writing 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 |
openshift-ci-robot
added
the
size/L
abhinavdahiya
changed the title
|
@dav1x: 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. 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. |
| for ETCD in 0 1 2 | ||
| do | ||
|
|
||
| DNS_RECORDS+="$(dig +short etcd-$ETCD.${cluster_domain}) " |
There was a problem hiding this comment.
Do you mind rebuilding on top of #1492? Then we can search for control-plane-{0,1,2}.${cluster_domain} and compute-{0,1,2}.${cluster_domain} instead of etcd.
| w_count=0 | ||
|
|
||
| # check cluster_domain DNS first | ||
| for ETCD in 0 1 2 |
There was a problem hiding this comment.
The iteration should not be hard-coded to 0 1 2. It needs to be based on the number of control-plane instances (and the number of compute instances).
|
|
||
| query = { | ||
| cidr = "${var.machine_cidr}" | ||
| master_count = "${var.master_count}" |
There was a problem hiding this comment.
Use the terms control-plane and compute instead of master and worker.
| then | ||
| worker_ips+="$IPADDR " | ||
| w_count=$((w_count+1)) | ||
| elif [[ $bs_count -eq 1 ]] && [[ $m_count -eq $master_count ]] && [[ $w_count -eq $worker_count ]] |
There was a problem hiding this comment.
This should not be in an elif. It should be its own if. Otherwise, it will need to do an additional ping after finding all of the IP addresses. This would cause it to fail if the last IP address in the block is one that is going to be assigned.
My preference would be to break out of the for loop when we find the last compute IP address. And then put the jq after the for loop.
| w_count=$((w_count+1)) | ||
| elif [[ $bs_count -eq 1 ]] && [[ $m_count -eq $master_count ]] && [[ $w_count -eq $worker_count ]] | ||
| then | ||
| jq -n \ |
There was a problem hiding this comment.
Can we do this without requiring jq?
There was a problem hiding this comment.
Sorry. I glossed over the use of jq earlier in the script. We can leave the jq here.
| } | ||
|
|
||
| output "bootstrap_ip" { | ||
| value = "${split(" ", trimspace(data.external.ping.result.bootstrap_ip))}" |
There was a problem hiding this comment.
I would prefer to keep this as a string instead of a list, unless it is terribly difficult otherwise. Would it work to just omit the split?
|
Closed in favor of : #1502 |
