Skip to content

Added release server publishing retry#34605

Merged
camscale merged 2 commits intomasterfrom
fred/retry-release-server-1
Nov 16, 2023
Merged

Added release server publishing retry#34605
camscale merged 2 commits intomasterfrom
fred/retry-release-server-1

Conversation

@fheinecke
Copy link
Copy Markdown
Contributor

The release server publishing is failing due to a TLS issue. While it is being resolved I've added a retry to our pipelines to publish three times instead of once.

This should be removed once the issue is fixed.

@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

Comment thread .drone.yml Outdated
Comment on lines +16357 to +16362
- docker run -i -v /tmpfs/creds:/tmpfs/creds -e DRONE_REPO -e DRONE_TAG -e RELCLI_BASE_URL
-e RELCLI_CERT -e RELCLI_KEY $RELCLI_IMAGE auto_publish -f -v 6 || true
- docker run -i -v /tmpfs/creds:/tmpfs/creds -e DRONE_REPO -e DRONE_TAG -e RELCLI_BASE_URL
-e RELCLI_CERT -e RELCLI_KEY $RELCLI_IMAGE auto_publish -f -v 6 || true
- docker run -i -v /tmpfs/creds:/tmpfs/creds -e DRONE_REPO -e DRONE_TAG -e RELCLI_BASE_URL
-e RELCLI_CERT -e RELCLI_KEY $RELCLI_IMAGE auto_publish -f -v 6
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this just always call the command 3 times? Can we instead wrap this in a "for" loop with a few iterations and abort it on success? Also, why 3? I think @logand22 said for him it worked on like 25th attempt, can we just make it retry 50 times or something?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It worked on second attempt for me. I mentioned 25 because I called the command many times after the first success to make sure it never occurred after the first success.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If it works for Logan on the second attempt and for me (consistently) on the third, then three or more times should be sufficient.

I can put a shell for loop in here, but adding shell logic to dronegen to be outputted into a yaml shell script is... messy. I took this approach because I am reasonably confident that it will work, and is simple enough to debug issues that may arise with it.

Copy link
Copy Markdown
Contributor

@camscale camscale Nov 15, 2023

Choose a reason for hiding this comment

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

I think all it needs is || \ on the end instead of || true. Then it is one long list that stops when one of the commands succeeds and fails when all fail.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But if we do want to do 50 times or whatever, then a loop is a lot better. It shouldn't be that messy - let me knock something up.

@fheinecke fheinecke added the no-changelog Indicates that a PR does not require a changelog entry label Nov 14, 2023
Change the drone generation to use a loop to run the `auto_publish`
relcli command instead of listing them one-by-one and loop 10 times
instead of 3. The loop will terminate the first time `relcli` succeeds.

The loop has an `|| false` at the end to ensure the loop command fails
if all invocations of `relcli` fail. With `set -e`, even though the exit
status of the loop is non-zero, the shell seems to continue. With the
`|| false` at the end, it makes it exit on failure. I'm not sure exactly
how drone runs the commands so this may not be necessary but it seems
safer.

e.g.

    set -e
    for i in $(seq 10); do false && break; done
    echo hello

This will echo "hello" even though all invocations inside the loop
failed.

    set -e
    for i in $(seq 10); do false && break; done || false
    echo hello

This will not echo "hello" - `set -e` causes an exit before that command
due to the `|| false`.
@camscale camscale enabled auto-merge November 16, 2023 01:40
@camscale camscale added this pull request to the merge queue Nov 16, 2023
Merged via the queue into master with commit 2dd1abb Nov 16, 2023
@camscale camscale deleted the fred/retry-release-server-1 branch November 16, 2023 02:00
@public-teleport-github-review-bot
Copy link
Copy Markdown

@fheinecke See the table below for backport results.

Branch Result
branch/v12 Failed
branch/v13 Failed
branch/v14 Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-required no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants