ci-operator/templates/e2e-metal: fix csr approval and bootstrap log gathering #3587
Conversation
openshift-ci-robot
added
approved
|
/test pj-rehearse |
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?
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
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.
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)?
abhinavdahiya
force-pushed
the
metal-ci-template
branch
from
2e5e65d to
0217808
Compare
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
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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@abhinavdahiya: Updated the following 2 configmaps:
|
4 participants
Carries changes from 1 and 2
/cc @wking @staebler