From e2c9dc6664c9004aa8496c451283fbe4a0829ddd Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Tue, 26 Jul 2016 10:30:18 -0400 Subject: [PATCH 1/5] Use anonyous FIFO instead of pipe in os::cmd output test Previously, the output of the last command was searched by `grep -q`. When `grep -q` finds the text it is looking for, it exits immediately with exit code 0. If the command feed- ing `grep -q` with input is still running, it will write to a pipe that has no readers, and Bash will terminate the producer process with a SIGPIPE as this is invalid behavior. Instead of using a `cat | grep` approach for searching in os::cmd, we can therefore use `grep <(file)` approach, which creates an anonymous FIFO to pass the data and therefore guards against this sort of error. Signed-off-by: Steve Kuznetsov --- hack/lib/cmd.sh | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/hack/lib/cmd.sh b/hack/lib/cmd.sh index 2710e06b5e0e..9fdb3dba2329 100644 --- a/hack/lib/cmd.sh +++ b/hack/lib/cmd.sh @@ -193,18 +193,10 @@ function os::cmd::internal::expect_exit_code_run_grep() { local test_result=0 if [[ -n "${grep_args}" ]]; then - test_result=$( os::cmd::internal::run_collecting_output 'os::cmd::internal::get_results | grep -Eq "${grep_args}"'; echo $? ) - + test_result=$( os::cmd::internal::run_collecting_output 'grep -Eq "${grep_args}" <(os::cmd::internal::get_results)'; echo $? ) fi local test_succeeded=$( ${test_eval_func} "${test_result}"; echo $? ) - if (( ! test_succeeded )); then - os::text::print_blue "[DEBUG] Output content test failed. Debugging information follows:" - os::text::print_blue "[DEBUG] \${grep_args}=${grep_args}" - os::text::print_blue "[DEBUG] \${test_result}=${test_result}" - os::text::print_blue "[DEBUG] \${test_eval_func}=${test_eval_func}" - os::text::print_blue "[DEBUG] \${test_succeeded}=${test_succeeded}" - fi local end_time=$(os::cmd::internal::seconds_since_epoch) local time_elapsed=$(echo "scale=3; ${end_time} - ${start_time}" | bc | xargs printf '%5.3f') # in decimal seconds, we need leading zeroes for parsing later @@ -578,7 +570,8 @@ function os::cmd::internal::run_until_text() { local test_succeeded=0 while [ $(date +%s000) -lt $deadline ]; do local cmd_result=$( os::cmd::internal::run_collecting_output "${cmd}"; echo $? ) - local test_result=$( os::cmd::internal::run_collecting_output 'os::cmd::internal::get_results | grep -Eq "${text}"'; echo $? ) + local test_result + test_result=$( os::cmd::internal::run_collecting_output 'grep -Eq "${text}" <(os::cmd::internal::get_results)'; echo $? ) test_succeeded=$( os::cmd::internal::success_func "${test_result}"; echo $? ) if (( test_succeeded )); then From a2191866ae1ae155a3855a26420298edced1a7bd Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Tue, 26 Jul 2016 10:36:22 -0400 Subject: [PATCH 2/5] Ensured that all indentation in hack/lib/cmd.sh used tabs Signed-off-by: Steve Kuznetsov --- hack/lib/cmd.sh | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/hack/lib/cmd.sh b/hack/lib/cmd.sh index 9fdb3dba2329..25289d812f13 100644 --- a/hack/lib/cmd.sh +++ b/hack/lib/cmd.sh @@ -181,7 +181,7 @@ function os::cmd::internal::expect_exit_code_run_grep() { os::test::junit::declare_test_start local name=$(os::cmd::internal::describe_call "${cmd}" "${cmd_eval_func}" "${grep_args}" "${test_eval_func}") - local preamble="Running ${name}..." + local preamble="Running ${name}..." echo "${preamble}" # for ease of parsing, we want the entire declaration on one line, so we replace '\n' with ';' junit_log+=( "${name//$'\n'/;}" ) @@ -197,11 +197,10 @@ function os::cmd::internal::expect_exit_code_run_grep() { fi local test_succeeded=$( ${test_eval_func} "${test_result}"; echo $? ) + local end_time=$(os::cmd::internal::seconds_since_epoch) + local time_elapsed=$(echo "scale=3; ${end_time} - ${start_time}" | bc | xargs printf '%5.3f') # in decimal seconds, we need leading zeroes for parsing later - local end_time=$(os::cmd::internal::seconds_since_epoch) - local time_elapsed=$(echo "scale=3; ${end_time} - ${start_time}" | bc | xargs printf '%5.3f') # in decimal seconds, we need leading zeroes for parsing later - - # clear the preamble so we can print out the success or error message + # clear the preamble so we can print out the success or error message os::text::clear_string "${preamble}" local return_code @@ -479,7 +478,7 @@ function os::cmd::internal::run_until_exit_code() { local description=$(os::cmd::internal::describe_call "${cmd}" "${cmd_eval_func}") local duration_seconds=$(echo "scale=3; $(( duration )) / 1000" | bc | xargs printf '%5.3f') local description="${description}; re-trying every ${interval}s until completion or ${duration_seconds}s" - local preamble="Running ${description}..." + local preamble="Running ${description}..." echo "${preamble}" # for ease of parsing, we want the entire declaration on one line, so we replace '\n' with ';' junit_log+=( "${description//$'\n'/;}" ) @@ -501,8 +500,8 @@ function os::cmd::internal::run_until_exit_code() { local end_time=$(os::cmd::internal::seconds_since_epoch) local time_elapsed=$(echo "scale=9; ${end_time} - ${start_time}" | bc | xargs printf '%5.3f') # in decimal seconds, we need leading zeroes for parsing later - # clear the preamble so we can print out the success or error message - os::text::clear_string "${preamble}" + # clear the preamble so we can print out the success or error message + os::text::clear_string "${preamble}" local return_code if (( cmd_succeeded )); then @@ -559,7 +558,7 @@ function os::cmd::internal::run_until_text() { local description=$(os::cmd::internal::describe_call "${cmd}" "" "${text}" "os::cmd::internal::success_func") local duration_seconds=$(echo "scale=3; $(( duration )) / 1000" | bc | xargs printf '%5.3f') local description="${description}; re-trying every ${interval}s until completion or ${duration_seconds}s" - local preamble="Running ${description}..." + local preamble="Running ${description}..." echo "${preamble}" # for ease of parsing, we want the entire declaration on one line, so we replace '\n' with ';' junit_log+=( "${description//$'\n'/;}" ) From 24f7a87f7d18b8f91058694ca4994f5763867989 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Tue, 26 Jul 2016 12:54:57 -0400 Subject: [PATCH 3/5] Improved Bash idioms in test/cmd/basicresources.sh Signed-off-by: Steve Kuznetsov --- test/cmd/basicresources.sh | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/test/cmd/basicresources.sh b/test/cmd/basicresources.sh index 62b10803cabc..5cc41ce64e77 100755 --- a/test/cmd/basicresources.sh +++ b/test/cmd/basicresources.sh @@ -14,6 +14,7 @@ trap os::test::junit::reconcile_output EXIT set +e oc delete all,templates,secrets,pods,jobs --all oc delete image v1-image + oc delete group patch-group exit 0 ) &>/dev/null @@ -28,20 +29,19 @@ os::test::junit::declare_suite_start "cmd/basicresources/versionreporting" # Test to make sure that we're reporting the correct version information from endpoints and the correct # User-Agent information from our clients regardless of which resources they're trying to access os::build::get_version_vars -os_git_regex="$( escape_regex ${OS_GIT_VERSION%%-*} )" -kube_git_regex="$( escape_regex ${KUBE_GIT_VERSION%%-*} )" -etcd_git_regex="$( escape_regex ${ETCD_GIT_VERSION%%-*} )" -# etcd_git_regex="$( escape_regex ${ETCD_GIT_VERSION%%-*} )" +os_git_regex="$( escape_regex "${OS_GIT_VERSION%%-*}" )" +kube_git_regex="$( escape_regex "${KUBE_GIT_VERSION%%-*}" )" +etcd_git_regex="$( escape_regex "${ETCD_GIT_VERSION%%-*}" )" os::cmd::expect_success_and_text 'oc version' "oc ${os_git_regex}" os::cmd::expect_success_and_text 'oc version' "kubernetes ${kube_git_regex}" os::cmd::expect_success_and_text 'oc version' "features: Basic-Auth" os::cmd::expect_success_and_text 'openshift version' "openshift ${os_git_regex}" os::cmd::expect_success_and_text 'openshift version' "kubernetes ${kube_git_regex}" os::cmd::expect_success_and_text 'openshift version' "etcd ${etcd_git_regex}" -os::cmd::expect_success_and_text 'curl -k ${API_SCHEME}://${API_HOST}:${API_PORT}/version' "${kube_git_regex}" -os::cmd::expect_success_and_text 'curl -k ${API_SCHEME}://${API_HOST}:${API_PORT}/version/openshift' "${os_git_regex}" -os::cmd::expect_success_and_not_text 'curl -k ${API_SCHEME}://${API_HOST}:${API_PORT}/version' "${OS_GIT_COMMIT}" -os::cmd::expect_success_and_not_text 'curl -k ${API_SCHEME}://${API_HOST}:${API_PORT}/version/openshift' "${KUBE_GIT_COMMIT}" +os::cmd::expect_success_and_text "curl -k '${API_SCHEME}://${API_HOST}:${API_PORT}/version'" "${kube_git_regex}" +os::cmd::expect_success_and_text "curl -k '${API_SCHEME}://${API_HOST}:${API_PORT}/version/openshift'" "${os_git_regex}" +os::cmd::expect_success_and_not_text "curl -k '${API_SCHEME}://${API_HOST}:${API_PORT}/version'" "${OS_GIT_COMMIT}" +os::cmd::expect_success_and_not_text "curl -k '${API_SCHEME}://${API_HOST}:${API_PORT}/version/openshift'" "${KUBE_GIT_COMMIT}" # variants I know I have to worry about # 1. oc (kube and openshift resources) @@ -148,7 +148,7 @@ os::cmd::expect_success 'oc get nodes' # subshell so we can unset kubeconfig cfg="${KUBECONFIG}" unset KUBECONFIG - os::cmd::expect_success 'kubectl get nodes --kubeconfig="${cfg}"' + os::cmd::expect_success "kubectl get nodes --kubeconfig='${cfg}'" ) echo "nodes: ok" os::test::junit::declare_suite_end @@ -296,13 +296,12 @@ os::cmd::expect_success 'oc delete all --all' os::test::junit::declare_suite_start "cmd/basicresources/projectadmin" # switch to test user to be sure that default project admin policy works properly -new="$(mktemp -d)/tempconfig" -os::cmd::expect_success "oc config view --raw > $new" -export KUBECONFIG=$new -project=$(oc project -q) +temp_config="$(mktemp -d)/tempconfig" +os::cmd::expect_success "oc config view --raw > '${temp_config}'" +export KUBECONFIG="${temp_config}" os::cmd::expect_success 'oc policy add-role-to-user admin test-user' os::cmd::expect_success 'oc login -u test-user -p anything' -os::cmd::try_until_success 'oc project ${project}' +os::cmd::try_until_success "oc project '$(oc project -q)'" os::cmd::expect_success 'oc run --image=openshift/hello-openshift test' os::cmd::expect_success 'oc run --image=openshift/hello-openshift --generator=run-controller/v1 test2' @@ -338,7 +337,7 @@ echo "delete all: ok" os::test::junit::declare_suite_end # service accounts should not be allowed to request new projects -os::cmd::expect_failure_and_text "oc new-project --token="$( oc sa get-token builder )" will-fail" 'Error from server: You may not request a new project via this API' +os::cmd::expect_failure_and_text "oc new-project --token='$( oc sa get-token builder )' will-fail" 'Error from server: You may not request a new project via this API' # test oc projects os::cmd::expect_failure_and_text 'oc projects test_arg' 'no arguments' @@ -353,9 +352,7 @@ echo 'projects command ok' os::test::junit::declare_suite_start "cmd/basicresources/patch" # Validate patching works correctly -oc login -u system:admin -# Clean up group if needed to be re-entrant -oc delete group patch-group || true +os::cmd::expect_success 'oc login -u system:admin' group_json='{"kind":"Group","apiVersion":"v1","metadata":{"name":"patch-group"}}' os::cmd::expect_success "echo '${group_json}' | oc create -f -" os::cmd::expect_success_and_text 'oc get group patch-group -o yaml' 'users: null' From 2f9ba180402778684d360ff62ced174553f97bc1 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Tue, 26 Jul 2016 14:06:47 -0400 Subject: [PATCH 4/5] Improved Bash idioms in test/cmd/builds.sh Signed-off-by: Steve Kuznetsov --- test/cmd/builds.sh | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/cmd/builds.sh b/test/cmd/builds.sh index 4e336ee8abc5..4e1ca8bcba16 100755 --- a/test/cmd/builds.sh +++ b/test/cmd/builds.sh @@ -26,7 +26,7 @@ os::test::junit::declare_suite_start "cmd/builds" os::cmd::expect_success 'oc new-build centos/ruby-22-centos7 https://github.com/openshift/ruby-hello-world.git' os::cmd::expect_success 'oc get bc/ruby-hello-world' -os::cmd::expect_success 'cat "${OS_ROOT}/images/origin/Dockerfile" | oc new-build -D - --name=test' +os::cmd::expect_success "cat '${OS_ROOT}/images/origin/Dockerfile' | oc new-build -D - --name=test" os::cmd::expect_success 'oc get bc/test' template='{{with .spec.output.to}}{{.kind}} {{.name}}{{end}}' @@ -115,13 +115,12 @@ os::cmd::expect_success_and_text 'oc describe buildConfigs ruby-sample-build' "$ os::cmd::expect_success_and_text 'oc describe buildConfigs ruby-sample-build' "Webhook GitHub" os::cmd::expect_success_and_text 'oc describe buildConfigs ruby-sample-build' "${url}/oapi/v1/namespaces/${project}/buildconfigs/ruby-sample-build/webhooks/secret101/generic" os::cmd::expect_success_and_text 'oc describe buildConfigs ruby-sample-build' "Webhook Generic" -os::cmd::expect_success 'oc start-build --list-webhooks='all' ruby-sample-build' +os::cmd::expect_success 'oc start-build --list-webhooks=all ruby-sample-build' os::cmd::expect_success_and_text 'oc start-build --list-webhooks=all bc/ruby-sample-build' 'generic' os::cmd::expect_success_and_text 'oc start-build --list-webhooks=all ruby-sample-build' 'github' os::cmd::expect_success_and_text 'oc start-build --list-webhooks=github ruby-sample-build' 'secret101' os::cmd::expect_failure 'oc start-build --list-webhooks=blah' -webhook=$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1 | head -n 1) -os::cmd::expect_success "oc start-build --from-webhook=${webhook}" +os::cmd::expect_success "oc start-build --from-webhook='$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1 | head -n 1)'" os::cmd::expect_success 'oc get builds' os::cmd::expect_success 'oc delete all -l build=docker' echo "buildConfig: ok" @@ -159,9 +158,9 @@ os::test::junit::declare_suite_start "cmd/builds/start-build" os::cmd::expect_success 'oc create -f test/integration/testdata/test-buildcli.json' # a build for which there is not an upstream tag in the corresponding imagerepo, so # the build should use the image field as defined in the buildconfig -started=$(oc start-build ruby-sample-build-invalidtag) +started="$(oc start-build ruby-sample-build-invalidtag)" os::cmd::expect_success_and_text "oc describe build ${started}" 'centos/ruby-22-centos7$' -frombuild=$(oc start-build --from-build="${started}") +frombuild="$(oc start-build --from-build="${started}")" os::cmd::expect_success_and_text "oc describe build ${frombuild}" 'centos/ruby-22-centos7$' os::cmd::expect_failure_and_text "oc start-build ruby-sample-build-invalid-tag --from-dir=. --from-build=${started}" "Cannot use '--from-build' flag with binary builds" os::cmd::expect_failure_and_text "oc start-build ruby-sample-build-invalid-tag --from-file=. --from-build=${started}" "Cannot use '--from-build' flag with binary builds" From 6badc53ca3cf8c43ba807cc7a1abb1ca487020b6 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Tue, 26 Jul 2016 15:00:13 -0400 Subject: [PATCH 5/5] Improved Bash idioms in test/cmd/images.sh Signed-off-by: Steve Kuznetsov --- test/cmd/images.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cmd/images.sh b/test/cmd/images.sh index 654c4efe4f9b..2d40bab40fe1 100755 --- a/test/cmd/images.sh +++ b/test/cmd/images.sh @@ -66,7 +66,7 @@ os::cmd::expect_success 'oc create -f test/fixtures/mysql-image-stream-mapping.y os::cmd::expect_success_and_text 'oc get istag/test:new --template="{{ index .image.dockerImageMetadata.Config.Entrypoint 0 }}"' "docker-entrypoint.sh" os::cmd::expect_success_and_text 'oc get istag/test:new -o jsonpath={.image.metadata.name}' 'sha256:b2f400f4a5e003b0543decf61a0a010939f3fba07bafa226f11ed7b5f1e81237' # reference should point to the current repository, and that repository should match the reported dockerImageRepository for pushes -repository=$(oc get is/test -o jsonpath={.status.dockerImageRepository}) +repository="$(oc get is/test -o jsonpath={.status.dockerImageRepository})" os::cmd::expect_success_and_text 'oc get istag/test:new -o jsonpath={.image.dockerImageReference}' "^$repository@sha256:b2f400f4a5e003b0543decf61a0a010939f3fba07bafa226f11ed7b5f1e81237" os::cmd::expect_success_and_text 'oc get istag/test:new -o jsonpath={.image.dockerImageReference}' "/$project/test@sha256:b2f400f4a5e003b0543decf61a0a010939f3fba07bafa226f11ed7b5f1e81237"