provision kubelet config file for GCE instead of deprecated flags #62183

mtaufen · 2018-04-05T23:03:59Z

Many Kubelet flags are now deprecated in favor of the versioned config file format. This PR adopts the versioned config file format in our cluster turn-up scripts.

cluster/kube-up.sh now provisions a Kubelet config file for GCE via the metadata server. This file is installed by the corresponding GCE init scripts.

mtaufen

Making changes in stages. Will try echo/redirect again on my second pass.

mtaufen · 2018-04-10T17:08:07Z

cluster/gce/gci/configure.sh

+  local -r dest="$1"
+  echo "Downloading Kubelet config file, if it exists"
+  # Fetch kubelet config file from GCE metadata server.
+  (umask 700;


mtaufen · 2018-04-10T17:12:47Z

cluster/gce/gci/configure.sh

+      http://metadata.google.internal/computeMetadata/v1/instance/attributes/kubelet-config; then
+    # only write to the final location if curl succeeds
+    mv "${tmp_kubelet_config}" "${dest}"
+  elif [[ REQUIRE_METADATA_KUBELET_CONFIG_FILE == "true" ]]; then


mtaufen · 2018-04-10T17:13:54Z

cluster/gce/gci/configure.sh

+  fi
+  )
+}
+
 function download-kube-master-certs {
  # Fetch kube-env from GCE metadata server.
  (umask 700;


Changed to 077

mtaufen · 2018-04-10T17:15:40Z

cluster/gce/util.sh

@@ -531,16 +533,98 @@ function build-node-labels {
  echo $node_labels
 }

+# yaml-map-string-stringarray converts the encoded structure to yaml format, and echoes the result
+# under the provided name. If the encoded structure is empty, echoes nothing.
+# 1: item separator (e.g. ','')


curse my editor's quote behavior

mtaufen · 2018-04-10T17:15:47Z

cluster/gce/util.sh

@@ -531,16 +533,98 @@ function build-node-labels {
  echo $node_labels
 }

+# yaml-map-string-stringarray converts the encoded structure to yaml format, and echoes the result


mtaufen · 2018-04-10T18:20:22Z

cluster/gce/util.sh

+  done
+  # only output if there is a non-empty map
+  if [ ${#map[@]} -gt 0 ]; then
+    cat >>$file <<EOF


mtaufen · 2018-04-10T18:20:33Z

cluster/gce/util.sh

+  done
+  # only output if there is a non-empty map
+  if [ ${#map[@]} -gt 0 ]; then
+    cat >>$file <<EOF


mtaufen · 2018-04-10T18:20:45Z

cluster/gce/util.sh

+      (IFS=,
+       for v in ${map[$k]}; do
+        cat >>$file <<EOF
+    - $(yaml-quote ${v})


mtaufen · 2018-04-10T18:21:13Z

cluster/gce/util.sh

+  local file=$2
+
+  rm -f ${file}
+  cat >>$file <<EOF


mtaufen · 2018-04-10T18:21:29Z

cluster/gce/util.sh

+       cat >>$file <<EOF
+staticPodURL: $(yaml-quote ${MANIFEST_URL})
+EOF
+    write-yaml-map-string-stringarray ',' ':' staticPodURLHeader ${MANIFEST_URL_HEADER} $file


done w/ double quote
(stage 2)

filbranden

Looking good! You need to remove the ${file} from the list of arguments in the functions... You're not passing it when calling them, so I think just not taking them as argument is enough already...

filbranden · 2018-04-10T22:00:37Z

cluster/gce/util.sh

@@ -531,16 +533,98 @@ function build-node-labels {
  echo $node_labels
 }

+# yaml-map-string-stringarray converts the encoded structure to yaml format, and echoes the result


Looks great!

You're taking a ${file} argument here but you don't use it anywhere. Just remove it (and shift the other two up.)

filbranden · 2018-04-10T22:01:05Z

cluster/gce/util.sh

+  if [[ ${#map[@]} -gt 0 ]]; then
+    echo "${name}:"
+    for k in "${!map[@]}"; do
+      echo "  ${k}"


echo " ${k}:" with a colon at the end?

nice catch, thanks

filbranden · 2018-04-10T22:02:40Z

cluster/gce/util.sh

+      declare -a values
+      IFS="${item_sep}" read -ra values <<<"${map[$k]}"
+      for v in "${values[@]}"; do
+        echo "    - $(yaml-quote "${v}")"


nitpick: I'd use an assignment to avoid the nested quotes:

declare quoted_value quoted_value=$(yaml-quote "${v}") echo " - ${quoted_value}"

Note that this doesn't work!

declare quoted_value=$(yaml-quote "${v}")

Because if yaml-quote fails (returns non-zero) then that gets ignored since neither errexit or pipefail is triggered... 😞

filbranden · 2018-04-10T22:03:02Z

cluster/gce/util.sh

+  fi
+}
+
+# yaml-map-string-string converts the encoded structure to yaml format, and echoes the result


Same here, ${file} should be gone...

filbranden · 2018-04-10T22:03:25Z

cluster/gce/util.sh

+    echo "${name}:"
+    for k in "${!map[@]}"; do
+      if [[ "${quote_val_string}" == "true" ]]; then
+        echo "  ${k}: $(yaml-quote ${map[$k]})"


See my comment above about storing yaml-quote result on a variable...

filbranden · 2018-04-10T22:39:24Z

LGTM 👍

mikedanese · 2018-04-11T21:01:08Z

cmd/kubelet/app/server.go

@@ -230,6 +230,7 @@ HTTP server: The kubelet can also listen for HTTP and respond to a simple API
 			}

 			// run the kubelet
+			glog.V(5).Infof("KubeletConfiguration: %#v", kubeletServer.KubeletConfiguration)


still need this?

I've used this in a number of debugging scenarios, I figure we might as well leave it at v=5.

mikedanese · 2018-04-11T21:02:05Z

/approve

mikedanese · 2018-04-11T21:03:20Z

cluster/gce/gci/configure-helper.sh

@@ -2406,6 +2407,12 @@ fi

 source "${KUBE_HOME}/kube-env"

+
+if [ -f "${KUBE_HOME}/kubelet-config.yaml" ]; then


move this into start-kubelet right before you use it?

This PR extends the client-side startup scripts to provision a Kubelet config file instead of legacy flags. This PR also extends the master/node init scripts to install this config file from the GCE metadata server, and provide the --config argument to the Kubelet.

mtaufen · 2018-04-13T21:19:23Z

/status approved-for-milestone

k8s-github-robot · 2018-04-13T21:19:51Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@maisem @mikedanese @mtaufen

Pull Request Labels

sig/cluster-lifecycle sig/node: Pull Request will be escalated to these SIGs if needed.
priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
kind/feature: New functionality.

Help

mikedanese · 2018-04-13T21:25:34Z

/lgtm

k8s-ci-robot · 2018-04-13T21:25:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikedanese, mtaufen

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:

~~cluster/gce/OWNERS~~ [mikedanese]
~~cmd/kubelet/OWNERS~~ [mikedanese]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-github-robot · 2018-04-13T23:01:11Z

Automatic merge from submit-queue (batch tested with PRs 62486, 62471, 62183). If you want to cherry-pick this change to another branch, please follow the instructions here.

mtaufen added this to the v1.11 milestone Apr 5, 2018

mtaufen self-assigned this Apr 5, 2018

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 5, 2018

k8s-ci-robot requested review from bowei and vishh April 5, 2018 23:04

k8s-github-robot added the milestone/needs-approval label Apr 5, 2018

mtaufen force-pushed the gce-kc-metadata branch 14 times, most recently from 2b2b3fd to e0423a1 Compare April 9, 2018 22:32

mtaufen changed the title ~~WIP provision kubelet config file instead of deprecated flags~~ provision kubelet config file for GCE instead of deprecated flags Apr 9, 2018

mtaufen commented Apr 10, 2018

View reviewed changes

mtaufen force-pushed the gce-kc-metadata branch from 349b11f to 9649267 Compare April 10, 2018 18:55

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2018

filbranden reviewed Apr 10, 2018

View reviewed changes

mtaufen force-pushed the gce-kc-metadata branch from 02b1cc0 to 0328e10 Compare April 10, 2018 22:11

mtaufen force-pushed the gce-kc-metadata branch 4 times, most recently from c70ef83 to 570f6f0 Compare April 11, 2018 00:16

mikedanese reviewed Apr 11, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 11, 2018

mikedanese reviewed Apr 11, 2018

View reviewed changes

mtaufen force-pushed the gce-kc-metadata branch from 570f6f0 to 2baf00a Compare April 12, 2018 00:42

k8s-github-robot added milestone/removed and removed milestone/needs-approval labels Apr 13, 2018

k8s-github-robot removed this from the v1.11 milestone Apr 13, 2018

mtaufen force-pushed the gce-kc-metadata branch from 2baf00a to 420edc7 Compare April 13, 2018 20:09

mtaufen added this to the v1.11 milestone Apr 13, 2018

k8s-ci-robot added the status/approved-for-milestone label Apr 13, 2018

mtaufen removed the milestone/removed label Apr 13, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 13, 2018

k8s-github-robot merged commit a5f2655 into kubernetes:master Apr 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provision kubelet config file for GCE instead of deprecated flags #62183

provision kubelet config file for GCE instead of deprecated flags #62183

mtaufen commented Apr 5, 2018 •

edited

Loading

mtaufen left a comment

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

filbranden left a comment

filbranden Apr 10, 2018

filbranden Apr 10, 2018

mtaufen Apr 10, 2018

filbranden Apr 10, 2018

mtaufen Apr 10, 2018

mtaufen Apr 10, 2018

filbranden Apr 10, 2018

filbranden Apr 10, 2018

filbranden commented Apr 10, 2018

mikedanese Apr 11, 2018

mtaufen Apr 13, 2018

mikedanese commented Apr 11, 2018

mikedanese Apr 11, 2018

mtaufen commented Apr 13, 2018

k8s-github-robot commented Apr 13, 2018

mikedanese commented Apr 13, 2018

k8s-ci-robot commented Apr 13, 2018

k8s-github-robot commented Apr 13, 2018

		@@ -2406,6 +2407,12 @@ fi

		source "${KUBE_HOME}/kube-env"


		if [ -f "${KUBE_HOME}/kubelet-config.yaml" ]; then

provision kubelet config file for GCE instead of deprecated flags #62183

provision kubelet config file for GCE instead of deprecated flags #62183

Conversation

mtaufen commented Apr 5, 2018 • edited Loading

mtaufen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filbranden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filbranden commented Apr 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikedanese commented Apr 11, 2018

Choose a reason for hiding this comment

mtaufen commented Apr 13, 2018

k8s-github-robot commented Apr 13, 2018

mikedanese commented Apr 13, 2018

k8s-ci-robot commented Apr 13, 2018

k8s-github-robot commented Apr 13, 2018

mtaufen commented Apr 5, 2018 •

edited

Loading