feat: leverage retry-action to increase reliability of builds.

[dylanmtaylor]

This is a proposed implementation of #502. It needs to run the GitHub action to be tested, but should improve reliability without manual build re-triggers.

[dylanmtaylor]

I need to figure out why this is failing before it's merged.

[dylanmtaylor]

The issue is related to this section

        labels: |
          ${{ steps.meta.outputs.labels }}

I think the retry action is sensitive to the indentation here.

[dylanmtaylor]

This is now fixed using a workaround:

I pad labels in another step and output it to the environment. This is necessary because we need it to remain a multiline string without using any character substitution. I then pull the string into the inputs for the retry-action. You can confirm the inputs work on the build step.

[dylanmtaylor]

It's not the cleanest in the world, but if it gets us working retries, that is a huge win.

      # Workaround to fix indentation for the labels so that retries work.
      - name: Left pad labels
        id: left_pad_labels
        shell: bash
        env:
          LABELS: ${{steps.meta.outputs.labels}}
        run: |
          echo "Labels:"
          echo "${LABELS}"
          export TRANSFORMED=$(echo "$LABELS" | sed 's,^,  ,g')
          echo "Transformed:"
          echo "${TRANSFORMED}"
          echo "LABELS<<EOF" >> $GITHUB_ENV
          echo "$TRANSFORMED" >> $GITHUB_ENV
          echo "EOF" >> $GITHUB_ENV

^ This could be a single set of echo commands to the GITHUB_ENV but I left these in for troubleshooting purposes so it is clear what is going on.

[bsherman]

I'm not confident we want to always retry the image build job. Usually when image build fails it is due to legit issues with package dependencies (rpmfusion etc). There are a few places where scripts can fail in ways that warrant retry, but I'd like to see those particular bits improved instead.

I DO think it would valuable to auto retry when the image push job fails since those failures are almost always due to some random ghcr/github hiccup.

[dylanmtaylor]

I'm not confident we want to always retry the image builds. Usually when image builds fail it is due to legit issues with package dependencies (rpmfusion etc). There are a few places where scripts can fail in ways that warrant retry, but I'd like to see those particular bits improved instead.

I DO think it would valuable to auto retry when the image push fails since those failures are almost always due to some random ghcr/github hiccup.

This can be modified to make the push retry.

[bsherman]

Thank you for your desire to help improve the ublue build reliability!

I do have some requests which I believe will make the solution more relevant to areas where I believe we most often see spurious failures.

List of requests follows:

1. when specifying uses: Wandalen/wretry.action@master please use an actual version selector not master (I believe @1 would work). This will cause dependabot to alert us with PRs when a new major version update is available.
2. please add retry action to Get current version labels step (this step is known to cause spurious failures)
3. please add retry action to Push to GHCR push step (this step is known to cause spurious failures)
4. please remove retry action build_image step (I believe the few spurious failures I've seen in this step should be fixed in the respective shell scripts, not at the top level)
5. since not using retry action for build_image step, please remove the left padding step

[dylanmtaylor]

I have force pushed a new version of this commit. @bsherman could you please review it?

[dylanmtaylor]

Updated with the requested changes.

[bsherman]

@dylanmtaylor why did you just force push?

[dylanmtaylor]

@dylanmtaylor why did you just force push?

I missed one line of whitespace change at the end of the file. I thought that it would be ideal to not have any changes to whitespace.

[dylanmtaylor]

Commit difference: https://github.com/ublue-os/main/compare/2b8ffe22406934db5af86d8d6acd17b072414a29..5f8f7196899cc6ba2853cd236758a42b130bbe43

[dylanmtaylor]

Interesting - the last attempt actually failed during the build action.

error creating build container: writing blob: storing blob to file "/var/tmp/storage1964624803/9": happened during read: unexpected EOF
time="2024-02-19T17:54:53Z" level=error msg="exit status 125"
Error: Error: buildah exited with code 125

[dylanmtaylor]

I think it may be beneficial to add the retry to the build image after all. I am testing a revised version of this with it in there too. If it builds successfully the first time it'll do nothing, and if not it gets rerun only a couple of times.

[dylanmtaylor]

I opened #506 as an alternative version of this which I feel is a bit better due to retrying the build action as well.

[dylanmtaylor]

Converting to draft as I feel the merits of this vs #506 should be discussed before one is merged.

[bsherman]

@dylanmtaylor why did you just force push?

I missed one line of whitespace change at the end of the file. I thought that it would be ideal to not have any changes to whitespace.

Understood.

In general, it's preferable to not force push in PRs as it removes context from code reviews, etc. We've been looking at using a tool to enforce policy across all ublue-os org repos, which would include preventing force-push, but for now, I'm just asking to please not do it anymore. :-)

Our auto-merge settings already squash commits, so it force push of pre-squashed commits doesn't really gain anything.

[dylanmtaylor]

This is ready to go in, assuming it fully passes CI. @bsherman

[dylanmtaylor]

I think I can see what happened here.

Error: The template is not valid. .github/workflows/build.yml (Line: 243, Col: 17): Error reading JToken from JsonReader. Path '', line 0, position 0.

^ The step output is blank because we only run the push action on the main branch and not on PRs. Signatures should also not run on PRs due to this.

[bsherman]

Error: The template is not valid. .github/workflows/build.yml (Line: 243, Col: 17): Error reading JToken from JsonReader. Path '', line 0, position 0.

Hmm... annoying that the fromJSON function must be running in the template even if the step is skipped.

Oh well, thank you for fixing.