dind: move to hyperkube and 'openshift start network' #20764
Conversation
openshift-ci-robot
added
sig/networking
size/L
openshift-ci-robot
added
the
approved
|
LGTM |
| if [[ ! -f "${kube_config}" ]]; then | ||
| # use the static node config if it exists | ||
| # TODO: remove when static node configuration is no longer supported | ||
| for f in ${config_dir}/system*.kubeconfig; do |
There was a problem hiding this comment.
I'd add a shopt -s failglob command here, or an additional check after the for loop to make sure $kube_config exists. (I was surprised to find that oc config --config=non-existent-file view --flatten does not fail.)
There was a problem hiding this comment.
@Miciah yeah, I was surprised too. Anyway, added a post-for check.
| @@ -15,7 +15,7 @@ function ovn-kubernetes-master() { | |||
|
|
|||
| local master_config="${config_dir}/master-config.yaml" | |||
| cluster_cidr=$(python -c "import yaml; stream = file('${master_config}', 'r'); y = yaml.load(stream); print y['networkConfig']['clusterNetworks'][0]['cidr']") | |||
| apiserver=$(oc --config="${kube_config}" config view -o custom-columns=server:clusters[0].cluster.server | grep http) | |||
| apiserver=$(grep server ${kube_config} | cut -f 6 -d' ') | |||
There was a problem hiding this comment.
Adding -m1, adding a colon to the pattern, and using awk would make the command more robust:
apiserver=$(grep -m1 server: ${kube_config} | awk '{print $2}')
There was a problem hiding this comment.
apiserver=$(awk '/server:/ { print $2; exit }')
:-)
There was a problem hiding this comment.
@Miciah fixed, thanks.
| # Create the service account for openshift-sdn | ||
| if ! /usr/local/bin/oc --config="${kube_config}" get serviceaccount openshift-sdn >/dev/null 2>&1; then | ||
| /usr/local/bin/oc --config="${kube_config}" create serviceaccount openshift-sdn | ||
| /usr/local/bin/oc adm --config="${kube_config}" policy add-cluster-role-to-user cluster-admin -z openshift-sdn |
There was a problem hiding this comment.
For consistency move the "adm" after "--config...". (If you copied this from elsewhere it's presumably just an artifact of blindly running s/oadm/oc adm/ and not an example of good style.)
(likewise below)
| local kube_config="${config_dir}/admin.kubeconfig" | ||
|
|
||
| local msg="apiserver to become alive" | ||
| os::util::wait-for-condition "${msg}" "is-api-running ${kube_config}" |
There was a problem hiding this comment.
drop ${msg}. (other calls to wait-for-condition below don't use a separate msg var)
| @@ -15,7 +15,7 @@ function ovn-kubernetes-master() { | |||
|
|
|||
| local master_config="${config_dir}/master-config.yaml" | |||
| cluster_cidr=$(python -c "import yaml; stream = file('${master_config}', 'r'); y = yaml.load(stream); print y['networkConfig']['clusterNetworks'][0]['cidr']") | |||
| apiserver=$(oc --config="${kube_config}" config view -o custom-columns=server:clusters[0].cluster.server | grep http) | |||
| apiserver=$(grep server ${kube_config} | cut -f 6 -d' ') | |||
There was a problem hiding this comment.
apiserver=$(awk '/server:/ { print $2; exit }')
:-)
dcbw
force-pushed
the
dind-hyperkube
branch
4 times, most recently
from
638e3ee
to
697c652
Compare
|
@Miciah @danwinship updated; PTAL thanks! |
|
/lgtm |
In preparation for moving to daemonsets, start using hyperkube on nodes instead of the all-in-one. This requires a new node service for openshift-sdn, and we might as well create a new service account for that while we're at it.
Catch up with some recent changes in config reading, fix some bugs, and consolidate some code.
|
@Miciah @danwinship repushed to fix merge conflict, another LGTM would be great thanks! |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, dcbw, Miciah 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 |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Let's win half the battle and do the whole daemonset/image thing as the second part.
@openshift/sig-networking @squeed @danwinship @JacobTanenbaum