Skip to content

Conversation

@SalimKayal
Copy link
Collaborator

@SalimKayal SalimKayal commented Oct 10, 2025

This pull request introduces several enhancements to the buildpacks, builder, and run image:

  • ARM64 Architecture Support: Enabled linux/arm64 targets for the buildpacks, allowing them to be built and run on ARM64 architectures. The publishing scripts (publish_builder.sh, publish_buildpack.sh) have also been updated to remove the explicit --target "linux/amd64" flag, which should allow pack to build for all supported targets by default.
  • Run Image Update: The base run image in run-image/Dockerfile has been updated from paketobuildpacks/run-jammy-full:latest to paketobuildpacks/run-jammy-full:0.1.99.
  • Dockerfile Improvements:
    • wget and tini have been added to the apt-get install commands in the run-image/Dockerfile for broader utility.
    • A userdel command ((userdel $(getent passwd ${user_id}|cut -d: -f1) || true)) was added before groupadd and useradd to ensure cleaner user management during image creation.
  • Tini Dependency Adjustment: The tini dependency has been removed from the [[requires]] section in buildpacks/ttyd/bin/detect, as tini is now universally installed in the run-image/Dockerfile.

These changes aim to improve platform compatibility, update core components, and streamline the overall build and packaging process.

@SalimKayal SalimKayal requested a review from olevski October 10, 2025 09:37
Copy link
Member

@olevski olevski left a comment

Choose a reason for hiding this comment

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

Thanks Salim this looks really nice. I have 2 questions just so I understand why we are making some of the changes.

Also we should probably also be running the tests on arm runners just to be safe. But this is for a followup in a spearate PR at some other time.

Comment on lines -21 to -26
[[requires]]
name = "tini"
[requires.metadata]
launch = true
Copy link
Member

Choose a reason for hiding this comment

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

question: why remove this? If it is already there I think then it won't be installed. I think with the changes you have made with this and by adding tini to the run image then you make the buildpacks only work with our run image. I was hoping we can keep things as general as possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tini is not multi-arch in paketo's buildpack. I could make my own tini buildpack instead if it's any useful

@SalimKayal
Copy link
Collaborator Author

SalimKayal commented Oct 30, 2025

quick note : the tests can't pass because the run image has changed and that the change is not yet reflected in the registry. See #105

@SalimKayal SalimKayal requested a review from olevski November 6, 2025 14:18
Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1,8 +1,10 @@
ARG base_image=index.docker.io/paketobuildpacks/run-jammy-full:latest
ARG base_image=index.docker.io/paketobuildpacks/ubuntu-noble-run:0.0.38
FROM ${base_image}
Copy link
Member

Choose a reason for hiding this comment

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

Are we missing AS base?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well caught

Copy link
Collaborator Author

@SalimKayal SalimKayal left a comment

Choose a reason for hiding this comment

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

I am fixing the tests and version change scripts. I will re-draft this pr

@@ -1,8 +1,10 @@
ARG base_image=index.docker.io/paketobuildpacks/run-jammy-full:latest
ARG base_image=index.docker.io/paketobuildpacks/ubuntu-noble-run:0.0.38
FROM ${base_image}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well caught

@SalimKayal SalimKayal marked this pull request as draft November 13, 2025 16:08
@rokroskar
Copy link
Member

Any chance we could separate out the multi-arch part from the regular buildpack dependency update here? So we can release that and push it out with the next renku release? Either that or we can merge #101 with the fix for vscodium misbehaving.

leafty
leafty previously approved these changes Nov 14, 2025
Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

Looks good for me

Co-authored-by: Flora Thiebaut <flora.thiebaut@sdsc.ethz.ch>
@SalimKayal
Copy link
Collaborator Author

Any chance we could separate out the multi-arch part from the regular buildpack dependency update here? So we can release that and push it out with the next renku release? Either that or we can merge #101 with the fix for vscodium misbehaving.

If I remember correctly there was a problem of library mismatch with the slurm executables that are mounted by the cscs within images.

imho we can merge #101 with the versions updated to the latest ones ?

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.

5 participants