Lease Proxy Client: Add many improvements#76538
Conversation
8bd204d to
eb18803
Compare
| fi | ||
|
|
||
| local ec=0 | ||
| local response=$(curl --no-progress-meter -X POST -o "$response_body" \ |
There was a problem hiding this comment.
Should we have a retry mechanism? If Boskos has a brief outage, I wouldn't want to skip the lease or error out
There was a problem hiding this comment.
We have --retry 5 --retry-delay 10 --retry-all-errors now. Transient errors like CONN_REFUSED or HTTP 500 force curl to retry, whereas errors like HTTP 404 do not. Those are treated like non-transient by curl.
| fi | ||
| } | ||
|
|
||
| local response="$(curl --connect-timeout 300 --max-time 600 \ |
There was a problem hiding this comment.
Same comment here about a retry, although hopefully we're just using images with jq already
There was a problem hiding this comment.
Same as above.
| local jitter_value='0' | ||
| local jitter_unit='' | ||
| if [[ $jitter_defined -eq 1 ]]; then | ||
| if [[ "$jitter" =~ ^([1-9][[:digit:]]*)(m|s)$ ]]; then | ||
| jitter_value="${BASH_REMATCH[1]}" | ||
| jitter_unit="${BASH_REMATCH[2]}" | ||
| if [[ "$jitter_unit" == "m" ]]; then | ||
| jitter_value=$(( jitter_value * 60 )) | ||
| fi | ||
| else | ||
| printf "jitter parameter is invalid\n" | ||
| return 1 | ||
| fi | ||
| fi |
eb18803 to
f3137a0
Compare
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danilo-gemoli, droslean 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 |
|
/retest-required |
1 similar comment
|
/retest-required |
|
@danilo-gemoli: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@danilo-gemoli: Updated the following 13 configmaps:
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This PR has been tailored specifically to handle the use use we have in #76238, the script is now more reliable, robust and secure.
I believe that providing few examples of how to use the
lease__*functions defined would help the reviewers more than trying to describe the technicalities that have been introduced.Use Case 1 - Acquire and release
Script:
Output:
Description:
--scope=stepmeans that the leases are scoped to the current step and must be released before it completes. This is the default behavior.lease_handleis an opaque file handle that can be used to:Under the hood is just a temporary file, but here we provide some helper functions like
lease__catto deal with it without being aware of the implementation details.Use Case 2 - Acquire in a step and release in a different one
Script:
Output:
Description:
--scope=testmeans that the leases are scoped to the entire life-cycle of a test, therefore they can be acquired from a step (as an example:ipi-install-install) and released later on, possibly in post steps.Use Case 3 - Deferred acquire and release
Script:
Output:
Description:
--jitterand--delaypauses the execution for the specified amount of time before acquiring and/or releasing.Use Case 4 - Refresh install leases
Script:
Output:
Acquired leases: openshift-org-aws--install-quota-slice-00 # After 20m openshift-org-aws--install-quota-slice-00 releasedDescription:
lease__install_lease_eligiblechecks whether a test using aCLUSTER_PROFILE_SETis eligible for acquiring an install lease. So far only the cluster profile sets that matchopenshift-org-*are eligible: see #76068 and #76340.The following lines refresh a lease:
The
lease__releaseis idempotent and thread safe, therefore the following facts hold:lease__release --handle="$install_lease_handle"several times is safe: leases are released only once.lease__release --handle="$install_lease_handle"are safe./cc @jupierce @stbenjam