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

Add support for the Pebble entrypoint #205

Merged

Conversation

cjdcordeiro
Copy link
Collaborator

@cjdcordeiro cjdcordeiro commented Mar 2, 2023

Needs canonical/pebble#188.

TODO:
Once the above PRs are merged:
- re-run the tests
- update the Tutorials and How-to guides

In this PR:

  • the services field is added to the Rockcraft project, with a lightweight validation according to the spec from https://github.com/canonical/pebble#layer-specification

  • the OCI entrypoint is fixed to always be pebble enter

  • the OCI cmd is null

  • the implicit pebble part now also creates the $PEBBLE path, where the Pebble layer is later be written

  • entrypoint and cmd are kept, but marked as deprecated, printing an error message and exiting the build whenever they are used

  • the Reference docs are updated

  • updated how-to guides and tutorial

  • adjusted all spread tests

  • Have you signed the CLA?


'pebble enter' is the OCI entrypoint for all ROCKs.
This commit is therefore deprecating the use of the
'entrypoint' and 'cmd' fields in rockcraft.yaml, in
favour of a new 'services' field, representing the
Pebble services to be written into a new Pebble layer
inside the ROCK's filesystem.
@cjdcordeiro cjdcordeiro self-assigned this Mar 2, 2023
@cjdcordeiro cjdcordeiro marked this pull request as draft March 2, 2023 12:45
@cjdcordeiro cjdcordeiro force-pushed the ROCKS-434_add-pebble-services-support branch 5 times, most recently from 321b160 to 448551c Compare March 23, 2023 07:21
@cjdcordeiro
Copy link
Collaborator Author

@tigarmo @sergiusens

canonical/pebble#188 is already approved and should be getting merged any time now...I've already created a Pebble snap with that version of Pebble, so this PR can now be reviewed

@cjdcordeiro
Copy link
Collaborator Author

fyi: multi-arch snaps in https://snapcraft.io/pebble

Copy link
Collaborator

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

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

thanks for the work! I only reviewed the code in rockcraft/ so far; once we agree on the points and the tests and docs are updated I'll review them

rockcraft/project.py Show resolved Hide resolved
rockcraft/project.py Outdated Show resolved Hide resolved
rockcraft/project.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/oci.py Show resolved Hide resolved
rockcraft/lifecycle.py Show resolved Hide resolved
rockcraft/oci.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

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

I did another pass at the code in rockcraft/, and also reviewed the tests and docs. Many thanks for all this work meu amigo ;)

rockcraft/oci.py Outdated Show resolved Hide resolved
rockcraft/pebble.py Show resolved Hide resolved
rockcraft/pebble.py Outdated Show resolved Hide resolved
rockcraft/project.py Show resolved Hide resolved
tests/unit/test_pebble.py Outdated Show resolved Hide resolved
tests/unit/test_project.py Outdated Show resolved Hide resolved
tests/unit/test_project.py Outdated Show resolved Hide resolved
tests/unit/test_pebble.py Show resolved Hide resolved
docs/tutorials/code/chisel/task.yaml Show resolved Hide resolved
docs/tutorials/code/hello-world/task.yaml Show resolved Hide resolved
@tigarmo
Copy link
Collaborator

tigarmo commented Mar 31, 2023

Here's a question for @sergiusens and @cjdcordeiro ; when we merge this PR, everyone's rockcraft.yaml files will be broken. Do we want to pre-warn the users, via I don't know Mattermost, a few days in advance? It's not like they can do anything until the update drops, but at least they won't be caught completely by surprise.

@cjdcordeiro
Copy link
Collaborator Author

Here's a question for @sergiusens and @cjdcordeiro ; when we merge this PR, everyone's rockcraft.yaml files will be broken. Do we want to pre-warn the users, via I don't know Mattermost, a few days in advance? It's not like they can do anything until the update drops, but at least they won't be caught completely by surprise.

I'd say so. More than that, it would probably be worth creating a legacy track for those who absolutely need to keep using the old version for a bit longer. @sergiusens wdyt?

tigarmo and others added 11 commits March 31, 2023 14:15
Docker Hub is fast but the Ubuntu images are updated infrequently and
the registry itself has pull limits for anonymous users. This limit is
annoying because currently rockcraft does not caching of base images.

The caching is coming, but in the meantime switch to Amazon ECR. The
registry has newer images and is only a little bit slower (from testing)
than Docker Hub).
Split the walking of the layer directory into a new function, which
returns the mapping from paths to arcnames. This is a step to simplify
usrmerge handling, as the original function was already too crowded.
No functional changes.
The situation is this: the primed payload has two distinct directories
(e.g. bin/dir1 and usr/bin/dir1) that, because of the usrmerge handling,
"point" to the same location (in this case, /usr/bin/dir1). We check
that the two directories have the same attributes (ownership and mode)
and then add a single entry to the layer (either one, they are all
equivalent).

All other cases (dirs with different attributes, conflicts between
files) raise errors. The reason for this is that we haven't seen this
come up "in the wild" yet, and it's hard to choose a behavior when we
don't know the circunstances.

Fixes canonical#203
…ical#207)

When rockcraft is installed or refreshed, the snap configure hook will
remove outdated images and instances from LXD.

Signed-off-by: Callahan Kovacs <[email protected]>
Craft-parts 1.18.4 makes the /dev rbind mount private, fixing chroot
and overlayfs mounts when using lxd 5.11.

Fixes canonical#195.

Signed-off-by: Claudio Matsuoka <[email protected]>
Lxd was pinned down to version 5.9 to work around the overlayfs issue
reported in canonical#195. With
this fix, this should no longer be necessary.

Signed-off-by: Claudio Matsuoka <[email protected]>
When Rockcraft creates the tar archive from the priming area, subdirs
symlinks are skipped from archive.

Solution is to add subdirs when they are symlinks.

Fix canonical#208

---------
Co-authored-by: Tiago Nobrega <[email protected]>
cjdcordeiro and others added 8 commits March 31, 2023 14:15
Craft-parts 1.19.0 requires overlays to be enabled.

Signed-off-by: Claudio Matsuoka <[email protected]>
Signed-off-by: Claudio Matsuoka <[email protected]>
Co-authored-by: Tiago Nobrega <[email protected]>

fix: spread tests

fix: linting

fix: re-add set_entrypoint to lifecycle and fix tasks

fix: small refactoring and updates

Update rockcraft/pebble.py

Co-authored-by: Tiago Nobrega <[email protected]>

Update tests/unit/test_pebble.py

Co-authored-by: Tiago Nobrega <[email protected]>

fix: add missing imports
@cjdcordeiro cjdcordeiro marked this pull request as ready for review March 31, 2023 12:22
@cjdcordeiro cjdcordeiro force-pushed the ROCKS-434_add-pebble-services-support branch 2 times, most recently from 30f2351 to 1c88fdc Compare March 31, 2023 12:35
@cjdcordeiro cjdcordeiro changed the title WIP: Add support for the Pebble entrypoint Add support for the Pebble entrypoint Apr 12, 2023
Copy link
Contributor

@cmatsuoka cmatsuoka left a comment

Choose a reason for hiding this comment

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

Lgtm, added a comment about the Pydantic alias generator.

rockcraft/project.py Show resolved Hide resolved
@tigarmo tigarmo enabled auto-merge (squash) April 13, 2023 13:40
@sergiusens sergiusens merged commit ca48ac3 into canonical:main Apr 13, 2023
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.

7 participants