Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc image tool script improvements #264

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Mar 15, 2024

  • Removes the redundant 64 suffix from the image filenames/mount directories on disk. This doesn't affect the filename on S3.
  • Adds comments to the mkfs / tune2fs usages, since their purpose and arguments are IMO not immediately obvious.

Docs for the related tools:
https://manpages.ubuntu.com/manpages/jammy/en/man8/mkfs.8.html
https://manpages.ubuntu.com/manpages/jammy/en/man8/tune2fs.8.html

Split out of #263.

GUS-W-15245261.

* Removes the stray `64` suffix from the image filenames/mount
  directories on disk. This doesn't affect the filename on S3. 
* Adds comments to the `mkfs` / `tune2fs` usages, since there
  purpose and arguments are IMO not immediately obvious.

Split out of #263.

GUS-W-15245261.
@edmorley edmorley self-assigned this Mar 15, 2024
@edmorley edmorley marked this pull request as ready for review March 15, 2024 12:51
@edmorley edmorley requested a review from a team as a code owner March 15, 2024 12:51
@edmorley edmorley enabled auto-merge (squash) March 15, 2024 12:52
@edmorley edmorley disabled auto-merge March 15, 2024 13:55
@edmorley edmorley enabled auto-merge (squash) March 15, 2024 13:55
@edmorley edmorley merged commit 6af6d3a into main Mar 18, 2024
4 checks passed
@edmorley edmorley deleted the edmorley/image-tools-cleanups branch March 18, 2024 09:05
edmorley added a commit that referenced this pull request Mar 21, 2024
At the very end of the image publishing process, Cheverny is notified
that the images have been uploaded and the staging manifest should
be updated, via the `publish-manifests` script.

For the v123 release, this script silently didn't publish the manifests:
https://github.com/heroku/base-images/actions/runs/8366257519/job/22906072260#step:7:123

Compare to the previous release:
https://github.com/heroku/base-images/actions/runs/8229492350/job/22500811676#step:7:101

It turns out this is due to:
- The script relying on the glob `/tmp/*64-*.manifest` to
  determine which stacks were built.
- That glob no longer matching after #264 (where the `64`
  bit reference was removed).

To make things worse, the script not only doesn't fail if no manifests
are found, but it also piped any errors to `/dev/null` in:
`ls /tmp/*64-*.manifest >& /dev/null`

As such, I've:
- updated the glob to match the new manifest filename format
- removed the conditional (since there is no scenario in which
  the manifests being missing is expected)
- for completeness, added `--exit-status` to the `jq` usage,
  to make it exit non-zero in more cases (jq would have exited
  non-zero for this no-files-matched case anyway, so this is just
  for completeness)

See:
https://jqlang.github.io/jq/manual/#invoking-jq

GUS-W-15304768.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants