Suppress node configuration details during tests by enj · Pull Request #10688 · openshift/origin

enj · 2016-08-27T18:03:26Z

Fixes #7193

enj · 2016-08-27T18:06:05Z

[test]

stevekuznetsov · 2016-08-27T21:21:47Z

I think you need it in hack/test-cmd.sh as it's still not been refactored to use the normal setup process.

stevekuznetsov · 2016-08-29T14:32:18Z

I think you need it in hack/test-cmd.sh

Disregard me ... no nodes in that test.

Instead of using the wait_for_url, though, let's pick up the new patttern with oc get --raw: #10542

stevekuznetsov · 2016-08-30T12:42:27Z

test flake unrelated, these bits look good

enj · 2016-08-30T12:59:54Z

@stevekuznetsov Don't you want me to squash first?

stevekuznetsov · 2016-08-30T13:07:02Z

@enj yeah, squash down and we can merge it

liggitt · 2016-08-30T13:31:57Z

Post 1.3

enj · 2016-09-22T19:06:49Z

@stevekuznetsov merge plz.

liggitt · 2016-09-22T19:09:03Z

hack/util.sh

-	wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1/nodes/${KUBELET_HOST}" "apiserver(nodes): " 0.25 80
+	os::test::junit::declare_suite_start "util/start_os_server"
+	os::cmd::try_until_text "oc get --raw /healthz --config='${MASTER_CONFIG_DIR}/admin.kubeconfig'" 'ok'
+	os::cmd::try_until_text "oc get --raw ${KUBELET_SCHEME}://${KUBELET_HOST}:${KUBELET_PORT}/healthz --config='${MASTER_CONFIG_DIR}/admin.kubeconfig'" 'ok'


using --raw against a kubelet threw errors trying to fetch discovery information from the kubelet for me

Ignoring the rebase flakes the tests seem to be fine.

enj · 2016-09-23T17:04:37Z

re[test] flake #11016

enj · 2016-09-27T15:21:41Z

re[test] flake on #11109 #11058

stevekuznetsov · 2016-10-06T12:01:56Z

@liggitt any last comments or was this good to go?

liggitt · 2016-10-06T19:59:20Z

no other comments from me but I didn't review in detail

stevekuznetsov

Most comments are generic and apply to all diffs.

stevekuznetsov · 2016-10-06T20:26:51Z

hack/lib/start.sh


-	wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz" "apiserver: " 0.25 160
-	wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz/ready" "apiserver(ready): " 0.25 160
+	os::test::junit::declare_suite_start "os::start::master"


We don't use colons as separators in the jUnit suites - we can just have setup/start-master

stevekuznetsov · 2016-10-06T20:27:23Z

hack/lib/start.sh

-	wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz/ready" "apiserver(ready): " 0.25 80
-	wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1/nodes/${KUBELET_HOST}" "apiserver(nodes): " 0.25 80
+	os::test::junit::declare_suite_start "os::start::all_in_one"
+	os::cmd::try_until_text "oc get --raw /healthz --config='${MASTER_CONFIG_DIR}/admin.kubeconfig'" 'ok'


We don't want to lose the poll & timeout info from before.

stevekuznetsov · 2016-10-06T20:28:01Z

hack/util.sh

-	echo "ERROR: gave up waiting for ${url}"
-	echo $(${cmd})
+	echo "ERROR: gave up waiting for ${url}" 1>&2
+	echo $(${cmd}) 1>&2


This can be written:

${cmd} 1>&2

stevekuznetsov · 2016-10-06T20:29:16Z

test/old-start-configs/v1.0.0/test-end-to-end.sh

-wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz" "apiserver: " 0.25 80
-wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz/ready" "apiserver(ready): " 0.25 80
-wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1/nodes/${KUBELET_HOST}" "apiserver(nodes): " 0.25 80
+wait_for_command "oc get --raw ${KUBELET_SCHEME}://${KUBELET_HOST}:${KUBELET_PORT}/healthz --config='${MASTER_CONFIG_DIR}/admin.kubeconfig' | grep ok"


We shouldn't touch these scripts, they should reflect the previous state to enforce backwards compat.

enj · 2016-10-06T22:03:18Z

@stevekuznetsov PTAL

stevekuznetsov

One remaining nit, rest LGTM

stevekuznetsov · 2016-10-07T11:56:12Z

hack/util.sh

-	echo "ERROR: gave up waiting for ${url}"
-	echo $(${cmd})
+	echo "ERROR: gave up waiting for ${url}" 1>&2
+	echo ${cmd} 1>&2


Sorry, not what I meant.

When you do something like:

echo "$( ${commands} )"

You are:

spawning a subshell $()

running ${commands} in that subshell

collecting stdout from it

asking echo to take that and put it into stdout

Instead, if you do simply:

${commands}

You then are:

running the ${commands} and letting them touch std{out,err} as it would

echo ${commands}

Won't actually run the ${commands}.

Signed-off-by: Monis Khan <mkhan@redhat.com>

openshift-bot · 2016-10-07T13:17:32Z

Evaluated for origin test up to b86eca7

stevekuznetsov · 2016-10-07T14:22:28Z

[merge]

openshift-bot · 2016-10-07T14:27:21Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9738/) (Image: devenv-rhel7_5156)

openshift-bot · 2016-10-07T14:27:21Z

Evaluated for origin merge up to b86eca7

openshift-bot · 2016-10-07T15:39:13Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9738/)

enj assigned stevekuznetsov Aug 27, 2016

enj force-pushed the enj/i/node_config_noise/7193 branch from b47cb55 to 15762a3 Compare August 30, 2016 13:17

danmcp added the priority/P2 label Aug 31, 2016

enj added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2016

liggitt reviewed Sep 22, 2016

View reviewed changes

enj force-pushed the enj/i/node_config_noise/7193 branch from 15762a3 to a3a35d1 Compare September 22, 2016 19:52

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 29, 2016

enj force-pushed the enj/i/node_config_noise/7193 branch from a3a35d1 to 9f3f501 Compare October 5, 2016 23:02

enj removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. lgtm Indicates that a PR is ready to be merged. labels Oct 5, 2016

stevekuznetsov suggested changes Oct 6, 2016

View reviewed changes

enj force-pushed the enj/i/node_config_noise/7193 branch from 9f3f501 to 69ae428 Compare October 6, 2016 22:01

stevekuznetsov suggested changes Oct 7, 2016

View reviewed changes

Suppress node configuration details during tests

b86eca7

Signed-off-by: Monis Khan <mkhan@redhat.com>

enj force-pushed the enj/i/node_config_noise/7193 branch from 69ae428 to b86eca7 Compare October 7, 2016 13:15

openshift-bot merged commit 56719d6 into openshift:master Oct 9, 2016

Conversation

enj commented Aug 27, 2016

Uh oh!

enj commented Aug 27, 2016

Uh oh!

stevekuznetsov commented Aug 27, 2016

Uh oh!

stevekuznetsov commented Aug 29, 2016

Uh oh!

stevekuznetsov commented Aug 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enj commented Aug 30, 2016

Uh oh!

stevekuznetsov commented Aug 30, 2016

Uh oh!

liggitt commented Aug 30, 2016

Uh oh!

enj commented Sep 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enj commented Sep 23, 2016

Uh oh!

enj commented Sep 27, 2016

Uh oh!

stevekuznetsov commented Oct 6, 2016

Uh oh!

liggitt commented Oct 6, 2016

Uh oh!

stevekuznetsov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enj commented Oct 6, 2016

Uh oh!

stevekuznetsov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Oct 7, 2016

Uh oh!

stevekuznetsov commented Oct 7, 2016

Uh oh!

openshift-bot commented Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Oct 7, 2016

Uh oh!

openshift-bot commented Oct 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

stevekuznetsov commented Aug 30, 2016 •

edited

Loading

openshift-bot commented Oct 7, 2016 •

edited

Loading