Fix k8s_wait_for_job_completed #873
Conversation
| @@ -58,12 +58,11 @@ k8s_wait_for_job_completed() { | |||
| while [ "$retryTimeSeconds" -gt 0 ]; do | |||
| # Avoid to exit the function if the job is not completed yet | |||
| set +e | |||
| kubectl get jobs -n $namespace -l $labelSelector \ | |||
| -o jsonpath='{.items[*].status.conditions[?(@.type=="Complete")].status}' | grep "True" | |||
| kubectl get jobs -n $namespace -l $labelSelector -o jsonpath='{.items[*].status.conditions[?(@.type=="Complete")].status}' | grep "True" | |||
There was a problem hiding this comment.
Removed the line split since I found that adding a trailing space \ make it fail. It's not the cause of the error here but it's safer to have it in just one line.
There was a problem hiding this comment.
Is it? Why? I mean we use line split everywhere. What error were you seeing?
There was a problem hiding this comment.
I mean that while I was copying this code I accidentally added a trailing space after the \. The code in the repo was fine but while I was debugging I found that somehow I introduced the trailing space. To avoid some issue like that in the future I removed the splitting of the command. It's not that important though, we can keep using \ if the command is too long.
| res=$? | ||
| set -e | ||
| # There is a job that finished | ||
| if [[ $res ]]; then | ||
| if [[ "$res" -eq "0" ]]; then |
There was a problem hiding this comment.
This was the error:
▶ a=1; if [[ $a ]]; then echo foo; fi
foo
▶ a=0; if [[ $a ]]; then echo foo; fi
foo
There was a problem hiding this comment.
Thanks @andresmgot, that makes sense.
I believe that the underlying issue is that we overrode the e option around kubectl.... | grep true so the script did not fail right? In any case we needed the set +e for some reason I do not remember.
There was a problem hiding this comment.
In any case we needed the set +e for some reason I do not remember.
The command kubectl get jobs may fail because the job has not been created yet. That's why we need to "catch" errors there so we can retry.
| @@ -58,12 +58,11 @@ k8s_wait_for_job_completed() { | |||
| while [ "$retryTimeSeconds" -gt 0 ]; do | |||
| # Avoid to exit the function if the job is not completed yet | |||
| set +e | |||
| kubectl get jobs -n $namespace -l $labelSelector \ | |||
| -o jsonpath='{.items[*].status.conditions[?(@.type=="Complete")].status}' | grep "True" | |||
| kubectl get jobs -n $namespace -l $labelSelector -o jsonpath='{.items[*].status.conditions[?(@.type=="Complete")].status}' | grep "True" | |||
There was a problem hiding this comment.
Is it? Why? I mean we use line split everywhere. What error were you seeing?
| res=$? | ||
| set -e | ||
| # There is a job that finished | ||
| if [[ $res ]]; then | ||
| if [[ "$res" -eq "0" ]]; then |
There was a problem hiding this comment.
Thanks @andresmgot, that makes sense.
I believe that the underlying issue is that we overrode the e option around kubectl.... | grep true so the script did not fail right? In any case we needed the set +e for some reason I do not remember.
Fix #869
k8s_wait_for_job_completedwas exiting prematurely. Check the comment below.Tested on https://circleci.com/gh/kubeapps/kubeapps/4881