From 1fe4192b671b1e2c709cfcf6a45762abf6e285c7 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Fri, 9 Feb 2018 13:22:23 +0200 Subject: [PATCH 1/7] hack/grab-profiles.sh: fix typo in variable name. Variable "controller_manager_port" was never updated, because it was misspelled "controller-managerr_port" in assignment. --- hack/grab-profiles.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/grab-profiles.sh b/hack/grab-profiles.sh index a0feecb5047..bf45245ef95 100755 --- a/hack/grab-profiles.sh +++ b/hack/grab-profiles.sh @@ -167,7 +167,7 @@ while true; do >&2 echo "empty argumet to --controller-manager-port flag" exit 1 fi - controller-managerr_port=$1 + controller_manager_port=$1 shift ;; -o|--output) From a0db1dc8c9c5568bd7dd6f852ec447ae74e69fc8 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Fri, 9 Feb 2018 13:29:50 +0200 Subject: [PATCH 2/7] hack/grab-profiles.sh: use double quotes in trap. The SSH_PID variable doesn't get expanded in the trap because of the single quotes. Change to double quotes. This is an example how the change works: $ FOO="bar" $ echo '$FOO' $FOO $ echo "$FOO" bar --- hack/grab-profiles.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/grab-profiles.sh b/hack/grab-profiles.sh index bf45245ef95..3f6a22f4065 100755 --- a/hack/grab-profiles.sh +++ b/hack/grab-profiles.sh @@ -246,8 +246,8 @@ echo "Waiting for tunnel to be created..." kube::util::wait_for_url http://localhost:${tunnel_port}/healthz SSH_PID=$(pgrep -f "/usr/bin/ssh.*${tunnel_port}:localhost:8080") -kube::util::trap_add 'kill $SSH_PID' EXIT -kube::util::trap_add 'kill $SSH_PID' SIGTERM +kube::util::trap_add "kill $SSH_PID" EXIT +kube::util::trap_add "kill $SSH_PID" SIGTERM requested_profiles=$(echo ${requested_profiles} | xargs -n1 | LC_ALL=C sort -u | xargs) profile_components=$(echo ${profile_components} | xargs -n1 | LC_ALL=C sort -u | xargs) From 0cc190cfa0bd110563a4714d93c85ca2ad1723c2 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Fri, 9 Feb 2018 14:14:35 +0200 Subject: [PATCH 3/7] hack/grab-profiles.sh: bash script cleanups. Use double quotes around variables in places where they might be used in globbing. Also replace an "echo | sed" construct with bash variable substitution. You can compare the variable substitution with this example: $ addresses="foo;bar,test" $ for token in $(echo $addresses | sed 's/[,;]/\n/g'); do echo "token: $token"; done token: foo token: bar token: test $ for token in ${addresses//[,;]/' '}; do echo "token: $token" ; done token: foo token: bar token: test --- hack/grab-profiles.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hack/grab-profiles.sh b/hack/grab-profiles.sh index 3f6a22f4065..3635d86a475 100755 --- a/hack/grab-profiles.sh +++ b/hack/grab-profiles.sh @@ -240,18 +240,18 @@ if [[ -z "${requested_profiles}" ]]; then exit 1 fi -gcloud compute ssh "${server_addr}" --ssh-flag=-nN --ssh-flag=-L${tunnel_port}:localhost:8080 & +gcloud compute ssh "${server_addr}" --ssh-flag=-nN --ssh-flag=-L"${tunnel_port}":localhost:8080 & echo "Waiting for tunnel to be created..." -kube::util::wait_for_url http://localhost:${tunnel_port}/healthz +kube::util::wait_for_url http://localhost:"${tunnel_port}"/healthz SSH_PID=$(pgrep -f "/usr/bin/ssh.*${tunnel_port}:localhost:8080") kube::util::trap_add "kill $SSH_PID" EXIT kube::util::trap_add "kill $SSH_PID" SIGTERM -requested_profiles=$(echo ${requested_profiles} | xargs -n1 | LC_ALL=C sort -u | xargs) -profile_components=$(echo ${profile_components} | xargs -n1 | LC_ALL=C sort -u | xargs) -kubelet_addreses=$(echo ${kubelet_addreses} | xargs -n1 | LC_ALL=C sort -u | xargs) +requested_profiles=$(echo "${requested_profiles}" | xargs -n1 | LC_ALL=C sort -u | xargs) +profile_components=$(echo "${profile_components}" | xargs -n1 | LC_ALL=C sort -u | xargs) +kubelet_addreses=$(echo "${kubelet_addreses}" | xargs -n1 | LC_ALL=C sort -u | xargs) echo "requested profiles: ${requested_profiles}" echo "flags for heap profile: ${mem_pprof_flags}" @@ -291,7 +291,7 @@ for component in ${profile_components}; do esac if [[ "${component}" == "kubelet" ]]; then - for node in $(echo ${kubelet_addreses} | sed 's/[,;]/\n/g'); do + for node in ${kubelet_addreses//[,;]/' '}; do grab_profiles_from_component "${requested_profiles}" "${mem_pprof_flags}" "${binary}" "${tunnel_port}" "${path}/${node}" "${output_dir}/${component}" "${timestamp}" done else From 7da195cac26e3333bd9ae3950d4bf7a79d7338e0 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Fri, 9 Feb 2018 13:24:05 +0200 Subject: [PATCH 4/7] hack/grab-profiles.sh: fix typos in error strings and variables. Change "argumet" -> "argument" and "addreses" -> "addresses". --- hack/grab-profiles.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/hack/grab-profiles.sh b/hack/grab-profiles.sh index 3635d86a475..c8ad130aa5a 100755 --- a/hack/grab-profiles.sh +++ b/hack/grab-profiles.sh @@ -51,7 +51,7 @@ KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${KUBE_ROOT}/hack/lib/init.sh" server_addr="" -kubelet_addreses="" +kubelet_addresses="" kubelet_binary="" master_binary="" scheduler_binary="" @@ -97,7 +97,7 @@ while true; do --master-binary) shift if [ -z "$1" ]; then - >&2 echo "empty argumet to --master-binary flag" + >&2 echo "empty argument to --master-binary flag" exit 1 fi master_binary=$1 @@ -111,16 +111,16 @@ while true; do shift profile_components="kubelet ${profile_components}" if [ -z "$1" ]; then - >&2 echo "empty argumet to --kubelet flag" + >&2 echo "empty argument to --kubelet flag" exit 1 fi - kubelet_addreses="$1 $kubelet_addreses" + kubelet_addresses="$1 $kubelet_addresses" shift ;; --kubelet-binary) shift if [ -z "$1" ]; then - >&2 echo "empty argumet to --kubelet-binary flag" + >&2 echo "empty argument to --kubelet-binary flag" exit 1 fi kubelet_binary=$1 @@ -133,7 +133,7 @@ while true; do --scheduler-binary) shift if [ -z "$1" ]; then - >&2 echo "empty argumet to --scheduler-binary flag" + >&2 echo "empty argument to --scheduler-binary flag" exit 1 fi scheduler_binary=$1 @@ -142,7 +142,7 @@ while true; do --scheduler-port) shift if [ -z "$1" ]; then - >&2 echo "empty argumet to --scheduler-port flag" + >&2 echo "empty argument to --scheduler-port flag" exit 1 fi scheduler_port=$1 @@ -155,7 +155,7 @@ while true; do --controller-manager-binary) shift if [ -z "$1" ]; then - >&2 echo "empty argumet to --controller-manager-binary flag" + >&2 echo "empty argument to --controller-manager-binary flag" exit 1 fi controller_manager_binary=$1 @@ -164,7 +164,7 @@ while true; do --controller-manager-port) shift if [ -z "$1" ]; then - >&2 echo "empty argumet to --controller-manager-port flag" + >&2 echo "empty argument to --controller-manager-port flag" exit 1 fi controller_manager_port=$1 @@ -251,7 +251,7 @@ kube::util::trap_add "kill $SSH_PID" SIGTERM requested_profiles=$(echo "${requested_profiles}" | xargs -n1 | LC_ALL=C sort -u | xargs) profile_components=$(echo "${profile_components}" | xargs -n1 | LC_ALL=C sort -u | xargs) -kubelet_addreses=$(echo "${kubelet_addreses}" | xargs -n1 | LC_ALL=C sort -u | xargs) +kubelet_addresses=$(echo "${kubelet_addresses}" | xargs -n1 | LC_ALL=C sort -u | xargs) echo "requested profiles: ${requested_profiles}" echo "flags for heap profile: ${mem_pprof_flags}" @@ -291,7 +291,7 @@ for component in ${profile_components}; do esac if [[ "${component}" == "kubelet" ]]; then - for node in ${kubelet_addreses//[,;]/' '}; do + for node in ${kubelet_addresses//[,;]/' '}; do grab_profiles_from_component "${requested_profiles}" "${mem_pprof_flags}" "${binary}" "${tunnel_port}" "${path}/${node}" "${output_dir}/${component}" "${timestamp}" done else From b6fbe2aee585d078644f6337f84a348e743e209a Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 13 Feb 2018 13:02:06 +0200 Subject: [PATCH 5/7] hack/update-codegen.sh: split string into array robustly. Use "mapfile" and "read" to split the $KUBE_AVAILABLE_GROUP_VERSIONS string into an array using space as the delimiter. This prevents shell from globbing and splitting the string in potentially wrong places. --- hack/update-codegen.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 9c29807b438..bd4b93d4b7c 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -38,7 +38,7 @@ informergen=$(kube::util::find-binary "informer-gen") # that generates the set-gen program. # -GROUP_VERSIONS=(${KUBE_AVAILABLE_GROUP_VERSIONS}) +IFS=" " read -ra GROUP_VERSIONS <<< "$KUBE_AVAILABLE_GROUP_VERSIONS" GV_DIRS=() INTERNAL_DIRS=() for gv in "${GROUP_VERSIONS[@]}"; do From a9905f2ad3b1609c24a29ffd77dd3981ec070d69 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 13 Feb 2018 13:04:48 +0200 Subject: [PATCH 6/7] hack/update-codegen.sh: fix finding items in an array. The current code if ! [[ " ${INTERNAL_DIRS[@]:-} " =~ " ${int_group} " ]]; then is broken because the array is concatenated in [[ .. ]] structure. This means that the match will be done to any substring in the resulting string which just happens to include ${int_group}. Fix this to use a loop instead, and do exact matching. Also make tabs consistent in the for loop. --- hack/update-codegen.sh | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index bd4b93d4b7c..a6d4293fa2a 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -50,20 +50,28 @@ for gv in "${GROUP_VERSIONS[@]}"; do # skip groups that aren't being served, clients for these don't matter - if [[ " ${KUBE_NONSERVER_GROUP_VERSIONS} " == *" ${gv} "* ]]; then - continue - fi + if [[ " ${KUBE_NONSERVER_GROUP_VERSIONS} " == *" ${gv} "* ]]; then + continue + fi GV_DIRS+=("${pkg_dir}") # collect internal groups int_group="${pkg_dir%/*}/" if [[ "${pkg_dir}" = core/* ]]; then - int_group="api/" + int_group="api/" + fi + + found=0 + for dir in "${INTERNAL_DIRS[@]}"; do + if [[ "$dir" = "$int_group" ]]; then + found=1 + break + fi + done + if [[ $found = 0 ]]; then + INTERNAL_DIRS+=("$int_group") fi - if ! [[ " ${INTERNAL_DIRS[@]:-} " =~ " ${int_group} " ]]; then - INTERNAL_DIRS+=("${int_group}") - fi done # delimit by commas for the command GV_DIRS_CSV=$(IFS=',';echo "${GV_DIRS[*]// /,}";IFS=$) From 6beb1ddac3c283fbe710eaeec059b9b435ef4ebc Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 13 Feb 2018 13:17:16 +0200 Subject: [PATCH 7/7] hack/update-codegen.sh: fix finding api names. Use "find -exec" instead of plain "find | xargs" to fix handling of difficult file names (such as those containing spaces). Also, use "mapfile" for creating the array from the output instead of letting the shell split the results into the array. Add double quotes to places where variable handling needs it to prevent splitting and globbing. --- hack/update-codegen.sh | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index a6d4293fa2a..16d19f3a53f 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -82,32 +82,26 @@ INTERNAL_DIRS_CSV=$(IFS=',';echo "${INTERNAL_DIRS[*]// /,}";IFS=$) ${clientgen} --input-base="k8s.io/kubernetes/pkg/apis" --input="${INTERNAL_DIRS_CSV}" "$@" ${clientgen} --output-base "${KUBE_ROOT}/vendor" --output-package="k8s.io/client-go" --clientset-name="kubernetes" --input-base="k8s.io/kubernetes/vendor/k8s.io/api" --input="${GV_DIRS_CSV}" "$@" -listergen_internal_apis=( -$( - cd ${KUBE_ROOT} - find pkg/apis -maxdepth 2 -name types.go | xargs -n1 dirname | sort +mapfile -t listergen_internal_apis < <( + cd "${KUBE_ROOT}" + sort <(find pkg/apis -maxdepth 2 -name types.go -exec dirname {} \;) ) -) -listergen_internal_apis=(${listergen_internal_apis[@]/#/k8s.io/kubernetes/}) +listergen_internal_apis=("${listergen_internal_apis[@]/#/k8s.io/kubernetes/}") listergen_internal_apis_csv=$(IFS=,; echo "${listergen_internal_apis[*]}") ${listergen} --input-dirs "${listergen_internal_apis_csv}" "$@" -listergen_external_apis=( -$( - cd ${KUBE_ROOT}/staging/src - find k8s.io/api -name types.go | xargs -n1 dirname | sort -) +mapfile -t listergen_external_apis < <( + cd "${KUBE_ROOT}/staging/src" + sort <(find k8s.io/api -name types.go -exec dirname {} \;) ) listergen_external_apis_csv=$(IFS=,; echo "${listergen_external_apis[*]}") ${listergen} --output-base "${KUBE_ROOT}/vendor" --output-package "k8s.io/client-go/listers" --input-dirs "${listergen_external_apis_csv}" "$@" -informergen_internal_apis=( -$( - cd ${KUBE_ROOT} - find pkg/apis -maxdepth 2 -name types.go | xargs -n1 dirname | sort +mapfile -t informergen_internal_apis < <( + cd "${KUBE_ROOT}" + sort <(find pkg/apis -maxdepth 2 -name types.go -exec dirname {} \;) ) -) -informergen_internal_apis=(${informergen_internal_apis[@]/#/k8s.io/kubernetes/}) +informergen_internal_apis=("${informergen_internal_apis[@]/#/k8s.io/kubernetes/}") informergen_internal_apis_csv=$(IFS=,; echo "${informergen_internal_apis[*]}") ${informergen} \ --input-dirs "${informergen_internal_apis_csv}" \ @@ -115,12 +109,10 @@ ${informergen} \ --listers-package k8s.io/kubernetes/pkg/client/listers \ "$@" -informergen_external_apis=( -$( - cd ${KUBE_ROOT}/staging/src - # because client-gen doesn't do policy/v1alpha1, we have to skip it too - find k8s.io/api -name types.go | xargs -n1 dirname | sort | grep -v pkg.apis.policy.v1alpha1 -) +mapfile -t informergen_external_apis < <( + cd "${KUBE_ROOT}/staging/src" + # because client-gen doesn't do policy/v1alpha1, we have to skip it too + sort <(find k8s.io/api -name types.go -exec dirname {} \;) | grep -v pkg.apis.policy.v1alpha1 ) informergen_external_apis_csv=$(IFS=,; echo "${informergen_external_apis[*]}")