ci-operator/templates/e2e-metal: fix csr approval and bootstrap log gathering #3587
Conversation
openshift-ci-robot
added
approved
|
/test pj-rehearse |
| while true; do | ||
| if [[ ! -f /tmp/install-complete ]]; then | ||
| oc get csr -ojson | jq -r '.items[] | select(.status == {} ) | .metadata.name' | xargs --no-run-if-empty oc adm certificate approve | ||
| sleep 15 & wait |
There was a problem hiding this comment.
Backgrounding slow processes gets them into the jobs output so we can TERM them for graceful shutdown. I don't think we care about gracefully stopping sleep, it won't cause any problems getting hard-killed by container removal. Can we just make this line sleep 15, or am I missing something?
| oc get csr -ojson | jq -r '.items[] | select(.status == {} ) | .metadata.name' | xargs oc adm certificate approve | ||
| function approve_csrs() { | ||
| while true; do | ||
| if [[ ! -f /tmp/install-complete ]]; then |
There was a problem hiding this comment.
Can drop the break and rephrase this loop to:
while [[ ! -f /tmp/install-complete ]]; do
oc get csr -ojson | jq -r '.items[] | select(.status == {} ) | .metadata.name' | xargs --no-run-if-empty oc adm certificate approve
sleep 15
fi
|
|
||
| oc patch configs.imageregistry.operator.openshift.io cluster --type merge --patch '{"spec":{"storage":{"emptyDir":{}}}}' | ||
| update_image_registry & |
There was a problem hiding this comment.
If the pattern is to echo some progress message before taking action, should we have echo 'Updating image registry storage...' or something similar before this line? I'm also fine just having the script silently do its job, but it feels odd to have this update_image_registry appear to happen under an Approving pending CSRs header.
|
|
||
| echo "Completing UPI setup" | ||
| openshift-install --dir=/tmp/artifacts/installer wait-for install-complete & | ||
| wait "$!" | ||
| touch /tmp/install-complete |
There was a problem hiding this comment.
Do we need this? We already have this trap to set either /tmp/setup-success or /tmp/exit.
There was a problem hiding this comment.
its from #3573 .
need the approve_csr to keep looping until install-complete. Any other suggestions?
There was a problem hiding this comment.
need the approve_csr to keep looping until install-complete.
Guard for both (like this)?
There was a problem hiding this comment.
WDYT 0217808...dd9f801
abhinavdahiya
force-pushed
the
metal-ci-template
branch
from
2e5e65d to
0217808
Compare
| @@ -366,14 +374,9 @@ objects: | |||
| mkdir -p /tmp/artifacts/pods /tmp/artifacts/nodes /tmp/artifacts/metrics /tmp/artifacts/bootstrap /tmp/artifacts/network | |||
|
|
|||
|
|
|||
| if [ -f /tmp/artifacts/installer/terraform.tfstate ] | |||
| if [ -f /tmp/artifacts/terraform/terraform.tfstate ] | |||
There was a problem hiding this comment.
Everyone else puts this under the installer directory:
$ git grep -h terraform.tfstate origin/master -- ci-operator/templates/openshift/installer | grep -o '/tmp.*tfstate' | sort | uniq -c
12 /tmp/artifacts/installer/terraform.tfstatebut I guess that's because they're running Terraform via the installer. So +1 to this, although I was initially confused ;).
abhinavdahiya
force-pushed
the
metal-ci-template
branch
from
0217808 to
dd9f801
Compare
| echo "Approving any pending CSR requests" | ||
| oc get csr -ojson | jq -r '.items[] | select(.status == {} ) | .metadata.name' | xargs oc adm certificate approve | ||
| function approve_csrs() { | ||
| while [[ ! -f /tmp/exit ]] || [[ ! -f /tmp/setup-success ]]; do |
There was a problem hiding this comment.
I think you want && to bail out if either exists.
…athering Carries changes from [1] and [2] [1]: openshift#3573 [2]: openshift#3475
abhinavdahiya
force-pushed
the
metal-ci-template
branch
from
dd9f801 to
a26eb31
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, wking The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@abhinavdahiya: Updated the following 2 configmaps:
|
Carries changes from 1 and 2
/cc @wking @staebler