Add Noble base stack - Githubissues

pbusko commented 3 months ago

Summary

This PR creates a Noble base stack

Use Cases

Ubuntu 24.04 has reached feature freeze and will be released soon

Checklist

[X] I have viewed, signed, and submitted the Contributor License Agreement.
[ ] I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
[X] I have added an integration test, if necessary.
[X] I have reviewed the styleguide for guidance on my code quality.
[X] I'm happy with the commit history on this PR (I have rebased/squashed as needed).

mhdawson commented 3 months ago

@pbusko what would be really useful if you could identify the files which are simply copies of what is used in existing stacks versus new/modified files. Maybe even a tree diff if that is possible.

I think there is likely very little "new" to review here as the stacks typically share a lot of common files. If you provide that info, it will make it faster/easier for people to review.

pbusko commented 3 months ago

what would be really useful if you could identify the files which are simply copies of what is used in existing stacks versus new/modified files. Maybe even a tree diff if that is possible.

I think there is likely very little "new" to review here as the stacks typically share a lot of common files. If you provide that info, it will make it faster/easier for people to review.

The only new things are addition of the arm64 platform and modified integration tests because of that (using local registry instead of skopeo copy to the docker daemon)

mhdawson commented 3 months ago

In case it helps reviewers of the 28 file only 12 in the PR show as changed between this PR and the existing files for the jammy-base-stack

diff -u -r jammy-base-stack noble-base-stack/ >temp1 midawson@midawson-virtualbox checkit]$ cat temp1 |grep "diff"

I've put strikethrough on files reported by the diff which are not actually files in the PR

diff -u -r jammy-base-stack/buildpack_integration_test.go noble-base-stack/buildpack_integration_test.go ~~diff -u -r jammy-base-stack/.git/config noble-base-stack/.git/config~~ ~~diff -u -r jammy-base-stack/.git/HEAD noble-base-stack/.git/HEAD~~ ~~Binary files jammy-base-stack/.git/index and noble-base-stack/.git/index differ~~ ~~diff -u -r jammy-base-stack/.git/logs/HEAD noble-base-stack/.git/logs/HEAD~~ (using local registry instead of skopeo copy to the docker daemon) ~~diff -u -r jammy-base-stack/.git/logs/refs/heads/main noble-base-stack/.git/logs/refs/heads/main~~ ~~diff -u -r jammy-base-stack/.git/logs/refs/remotes/origin/HEAD noble-basestack/.git/logs/refs/remotes/origin/HEAD~~ ~~diff -u -r jammy-base-stack/.git/packed-refs noble-base-stack/.git/packed-refs~~ ~~diff -u -r jammy-base-stack/.git/refs/heads/main noble-base-stack/.git/refs/heads/main~~ diff -u -r jammy-base-stack/.github/workflows/create-release.yml noble-base-stack/.github/workflows/create-release.yml diff -u -r jammy-base-stack/go.mod noble-base-stack/go.mod diff -u -r jammy-base-stack/go.sum noble-base-stack/go.sum diff -u -r jammy-base-stack/init_test.go noble-base-stack/init_test.go diff -u -r jammy-base-stack/metadata_test.go noble-base-stack/metadata_test.go diff -u -r jammy-base-stack/README.md noble-base-stack/README.md diff -u -r jammy-base-stack/scripts/.util/tools.json noble-base-stack/scripts/.util/tools.json diff -u -r jammy-base-stack/stack/build.Dockerfile noble-base-stack/stack/build.Dockerfile diff -u -r jammy-base-stack/stack/run.Dockerfile noble-base-stack/stack/run.Dockerfile diff -u -r jammy-base-stack/stack/stack.toml noble-base-stack/stack/stack.toml

The only files reported as "Only in" in the diff are in the .git directory and we'd expect them to be different between two repos so I think that confirms there are also no "missing" files.

Grep for files that have jammy in them only shows files that are in the list above, so all instances of jammy references should be updated to noble.

From my look through as @pbusko mentions the only really new different stuff is in

buildpack_integration_test.go
metadata_test.go
stack/stack.toml

and is needed to be able to do 2 arches instead of one.

@pbusko my last question would be if you could explain a bit more why using a local registry instead of skopeo copy to the docker daemon is needed because of supporting the 2 arches? Other than that its obvious to me why all of the deltas from the Jammy stack are there.

c0d1ngm0nk3y commented 3 months ago

@mhdawson

my last question would be if you could explain a bit more why using a local registry instead of skopeo copy to the docker daemon is needed

In a nutshell: skopeo copy cannot handle 2 architectures in one oci file. But we did not come up with this solution, but copied it from the jammy-tiny-builder. I think the arm work was done by @jericop and @dmikusa

mhdawson commented 3 months ago

@mhdawson

my last question would be if you could explain a bit more why using a local registry instead of skopeo copy to the docker daemon is needed

In a nutshell: skopeo copy cannot handle 2 architectures in one oci file. But we did not come up with this solution, but copied it from the jammy-tiny-builder. I think the arm work was done by @jericop and @dmikusa

Thanks for the explanation and info, understanding that I think this PR looks good to me. As a newer stack maintainer I think this PR should have a review from another maintainer as well before landing, but will add my LGTM.

loewenstein commented 3 months ago

Thanks @mhdawson, added @paketo-buildpacks/stacks-maintainers back to the reviewers to get attention.

sophiewigmore commented 3 months ago

The changes here look decent, although there might be one issue. Since you are enabling multi-arch support, the workflows need some changes. The only other stack we have ARM64 / multi-arch support for so far is Jammy Tiny stack, and the scripts and github workflows have some subtle differences from what is in https://github.com/paketo-buildpacks/github-config/tree/main/stack in order to support multi-arch. (Eventually, those changes will make it into github-config when we are ready to roll out ARM64 support to everywhere, but not yet.)

As a result, I think you'll need to modify the workflows/scripts here to more closely match what we have in the Tiny stack. As a result, those changed files will need to be added to the .syncignore files in the repo to prevent your changes from being overwritten by github-config.

Here's the diff files to check out:

 ‣ diff -qr scripts/ ~/workspace/paketo-buildpacks/jammy-tiny-stack/scripts/
Only in /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/scripts: .syncignore
Files scripts/.util/tools.json and /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/scripts/.util/tools.json differ
Files scripts/.util/tools.sh and /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/scripts/.util/tools.sh differ
Only in /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/scripts: publish.sh
Files scripts/receipts.sh and /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/scripts/receipts.sh differ

 ‣ diff -qr .github/ ~/workspace/paketo-buildpacks/jammy-tiny-stack/.github/
Files .github/.syncignore and /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/.github/.syncignore differ
Files .github/workflows/create-release.yml and /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/.github/workflows/create-release.yml differ
Files .github/workflows/push-image.yml and /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/.github/workflows/push-image.yml differ
Files .github/workflows/test-pull-request.yml and /Users/swigmore/workspace/paketo-buildpacks/jammy-tiny-stack/.github/workflows/test-pull-request.yml differ

Option 2 is to not include multi-arch support, and just use the workflows as-is, then we can add ARM64 later. Let me know if you have questions

loewenstein commented 3 months ago

I tend to prefer Option 2 then I guess. Adding @dmikusa @jericop as they asked to include ARM64 right away.

pbusko commented 3 months ago

@sophiewigmore we've updated the automation scripts to support arm64 platform

sophiewigmore commented 3 months ago

ready to merge whenever you are @pbusko

pbusko commented 3 months ago

@sophiewigmore thank you. There's nothing left from our side, I believe the PR can be merged.

loewenstein commented 3 months ago

Given that there are two approvals from maintainers and the clear indication to merge, I'll use my permissions as steering member to hit the merge button and avoid another timezone roundtrip. I hope that's fine for everyone.

paketo-buildpacks / noble-base-stack

Add Noble base stack #1

Summary

Use Cases

Checklist