Skip to content
This repository has been archived by the owner on Aug 2, 2019. It is now read-only.

Update installation-functions.sh #38

Merged
merged 1 commit into from
May 15, 2019

Conversation

vtereso
Copy link
Contributor

@vtereso vtereso commented Apr 30, 2019

Ignore headers. Simplify what grep is checking

Ignore headers. Simplify what grep is checking
Copy link
Contributor

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Having STATUS in there always felt goofy. :)

@@ -30,7 +30,7 @@ function timeout() {

# Waits for all pods in the given namespace to complete successfully.
function wait_for_all_pods {
timeout 300 "$CMD get pods -n $1 && [[ \$($CMD get pods -n $1 2>&1 | grep -c -v -E '(Running|Completed|Terminating|STATUS)') -eq 0 ]]"
timeout 300 "$CMD get pods -n $1 && [[ \$($CMD get pods -n $1 2>&1 --no-headers | grep -c -v -E '(Running|Completed|Terminating)') -eq 0 ]]"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe to assume that option is available for all expected versions of kubectl and oc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least as far back as v3.6, oc is replaced with kubectl. References: v3.6.0, v3.11.0, and master

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking to see what version of kubectl it was introduced in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was introduced in v1.4. Very confusingly, the documentation specifies --no-header, even when discussed within the PR itself, but the code specifies "no-headers". Search no header on v1.4 Release docs. PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good enough for me! We'll see if anyone complains. Thanks again!

@jcrossley3 jcrossley3 merged commit 90c3713 into openshift-cloud-functions:master May 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants