patch(update_bundle.yaml): Add resource pinning of OCI images for bundles #235

lucasgameiroborges · 2024-09-18T15:05:43Z

UPDATE: Refactored this PR to take into account John Meinel's suggested formatting, see result here: https://github.com/canonical/postgresql-k8s-bundle/pull/212/files

Related ticket: https://warthogs.atlassian.net/browse/DPE-5381

This PR will make update_bundle.yaml include OCI image versions + resource revisions that correspond to each charm, as this is will be required information for airgapped deployments using the proxy store.

Note: As of now, the proxy store doesn't take into consideration the charm revisions nor the resource revisions like juju does, so this may be subjected to change in the future! We are including this in the bundle as preparation for those features.

See more information about proxy store and airgapped deployments here: https://documentation.ubuntu.com/snap-store-proxy/en/airgap-charmhub

taurus-forever

LGTM in general, question: can we fetch store information once?
It is to be sure the received charm revision and the received OCI resource/tag matches each other (to minimize the risk of race during the charmcrat release).

lucasgameiroborges · 2024-09-20T15:00:58Z

LGTM in general, question: can we fetch store information once?
It is to be sure the received charm revision and the received OCI resource/tag matches each other (to minimize the risk of race during the charmcrat release).

You mean like, in case a new charm revision/oci-image pair released at the same time this workflow runs, and then fetch_latest_revision fetches the rev n-1 but fetch_oci_image_from_channel gets the oci-image related to rev n?

I think we can fetch all the info on a single request to snapcraft.io, yes. Will try it after lunch.

lucasgameiroborges · 2024-09-20T19:41:34Z

@taurus-forever I did a refactor to fetch both data objects from store at once (for each charm separately). See new commit in canonical/postgresql-k8s-bundle#196

lucasgameiroborges · 2024-09-20T19:43:06Z

python/cli/data_platform_workflows_cli/update_bundle.py

    revisions = []
    for channel in channel_map:
        if (
-            channel["channel"]["risk"] == risk
-            and channel["channel"]["track"] == track


no need to check for channel here, as we're fetching charm's data by channel directly from store.

carlcsaposs-canonical

looks like a good first prototype!

could you add context to the PR description about why this is being added (airgapped with store proxy?)?

python/cli/data_platform_workflows_cli/update_bundle.py

carlcsaposs-canonical · 2024-09-23T08:45:12Z

python/cli/data_platform_workflows_cli/update_bundle.py

+    metadata = yaml.safe_load(response.json()["default-release"]["revision"]["metadata-yaml"])
+    return channel_map, metadata


are we using update_bundle.yaml for any charms not maintained by our team? if so, would recommend using ["resources"]["download"]["url"] & extracting image from json downloaded at that url instead—this should work regardless if charm is using metadata.yaml or charmcraft.yaml or they specify their oci images in a different way (since upstream-source is not standardized by juju)

also could you test what happens with pinning here? e.g. if in metadata.yaml I specify a tag (instead of hash), when I charmcraft upload-resource, does juju pin the oci or will the resource revision be constant but the hash it deploys variable?

yep, we have to update charms not maintained by our team. I actually thought that way of specifying oci-image was something standard by juju, sad that's not the case. I agree that using the using the json from the url is a better approach, I'll try this approach next.

About pinning in metadata.yaml, I am not sure what juju does in this case (will check that), but let's suppose we're fetching the oci image from the download URL: would it matter which way juju does it, as longs as we're fetching the proper image that came with the charm itself?

would it matter which way juju does it

I think we should check

we've seen some issues with resources before (e.g. juju grabbing latest in a channel when deploying on an older rev), and while it might not be our problem to solve, we should still be aware if it exists—so that our users on airgapped envs get the right resources

Just did the refactoring to use the OCI image definition from "resources"]["download"]["url"] - It is not the same as the one we define in metadata.yaml, but it is the one we should export using skopeo on airgapped envs, so it should be a fair change. Revisions also match, I see the same mapping as the output of charmcraft status <charm>, but its worth mentioning that default-release only give us the tip of the channel (that we use for updating the bundle), it doesn't fetch this info for older revisions on the channel.

carlcsaposs-canonical · 2024-09-23T08:46:00Z

python/cli/data_platform_workflows_cli/update_bundle.py

+                f"Revision not found for {app['charm']} on {app['channel']} for Ubuntu {app.get('series', default_series)}"
+            )
+        if oci_image := fetch_oci_image_from_metadata(metadata):
+            app["resources"] = oci_image


is this format standardized/recognized by the proxy? is there any documentation about the format? (if so, please include a link in a comment)

Here's the example I based from: https://documentation.ubuntu.com/snap-store-proxy/en/airgap-charmhub/#export-charms

it looks like that syntax is for an override: https://documentation.ubuntu.com/snap-store-proxy/en/airgap-charmhub/#export-oci-images

do we need to override or does the default work? I'm wondering since it looks like the proxy is not downloading the image, but just getting the reference to it

After some re-reading of docs + local testing: We actually don't need to specify specific OCI image versions for each charm inside the bundle file, store-admin itself takes care of that. Here is the process:

In order to export the bundle's charms to a proxy store, users have to first run store-admin export charms bundle.yaml to download each charm as a .tar.gz package. Inside it, there is a resources folder with a collection of files, called "OCI image metadata blob" (example here), which contain the full remote path location + hash for the exact OCI image that the downloaded charm needs - out of the box, no need to specify anything on bundle (tested and confirmed locally)

Now, besides the .tar.gz package itself, the users also need to manually download each of the OCI images listed in the resources folder and store them on a local OCI registry (that's where the airgapped juju model is configured to look). This can be done using skopeo by running the commands described here. The full path where the user will store the images inside the local registry is important, more info on next point.

Finally, user will run sudo snap-store-proxy push-charms <path-to-tar-gz> to push the locally downloaded charm to the proxy store, and at this point juju deploy will work. But how does the proxy know which one of the local OCI images to use? By default, it is assumed that the user kept the path+name+hash format from the remote (normally its /charm/<random_sequence>/my-oci-image@sha256:<hash>) when copying to the local registry, but the user may choose not to do it. If the user wants to store the images in a different way, then they should edit the bundle.yaml file themselves and specify which local image to use for each charm, following this template.

So, considering all this, we actually should not specify anything inside the resources key on the bundle! this will define a specific override which may not correspond to what the user wants.

Now, if we want to facilitate things for the user a little bit (not much really, considering the overall amount of setup required) we could fetch the "OCI image metadata blobs" for each charm from the bundle (its possible to do so using the snapcraft io API) and put them in a separate file inside the bundle repo (keeping them updated according to charm revisions)

This way, users wouldn't need to unpack each .tar.gz file to see which remote OCI images they must fetch and export using skopeo. WDYT?

does it contain the correct hash even if the charm is not using the latest resource revision in a channel? (e.g. we're using a charm revision that is not latest in channel?

Still not sure - in my local tests, the revision specified for each charm is being ignored and store-admin is exporting the latest rev in the channel by default. I'm in contact with the Store team to verify this + my understanding in the comment from before - keep you updated.

IMO downloading the oci images should happen in the proxy or related tooling. I don't think we should upload oci images to the bundle git repos.

The download will still be done in the user side, my suggestion was to include the oci image's metadata in a separate file - ie a list of objects like this:

{ "ImageName": "registry.jujucharms.com/charm/kotcfrohea62xreenq1q75n1lyspke0qkurhk/postgresql-image@sha256:8a72e1152d4a01cd9f469...", "Password": "MDAxOGxvY2F0aW9uIGNoYXJtc3Rvcm...", "Username": "docker-registry" }

So that users know what images to download.

UPDATE:

As of right now, charmhub proxy doesn't have revision pinning of charms, see https://warthogs.atlassian.net/browse/PF-5185 - that's why my local testing wasn't working 🥲

It also doesn't have a way to pin/specify resources. But they are working on a proposal, they may adhere to the syntax juju uses to pin revisions of resources: https://juju.is/docs/sdk/bundle.yaml#heading--applications-application-resources-resource

charmhub proxy doesn't have revision pinning of charms

so is there currently no use for the update_bundle workflow? cc @taurus-forever

we originally used the update_bundle workflow when we were expected to provide bundles, but I believe currently the only reason we're maintaining bundles is for the airgapped proxy

so is there currently no use for the update_bundle workflow?

I mean, juju itself does consider charm revisions and resource pinnings when deploying bundles - It's the proxy store tooling that doesn't. But they do have plans to support it in the future, both charm revisions and resource revisions

Long story here! Put a little TLDR in the PR description - basically the store doesn't support any sort of revision pinning as of now, but they will in the future, so we will do the pinnings as preparation on our part.

lucasgameiroborges · 2024-09-26T03:55:45Z

python/cli/data_platform_workflows_cli/update_bundle.py

+                app["resources"] = {
+                    resource["name"]: resource["revision"],
+                    "oci-image": image_path,
+                    "src-creds": src_creds,


included the credentials because they are necessary for the skopeo set when setting up the airgapped env, and they are pretty much public anyway (public snapcraft api returns it without auth) so my idea was to add it in order for the bundle to contain all information one could need when deploying on airgapped. LMK if you don't want it and I remove it!

taurus-forever

This is very close to what we need. Let's present it to John and ask the first feedback.

taurus-forever · 2024-09-26T11:56:21Z

python/cli/data_platform_workflows_cli/update_bundle.py

+                    "oci-image": image_path,
+                    "src-creds": src_creds,


I would keep them close => by using the close label names. What about oci-image + oci-creds (or even split creds it to oci-username and oci-password ) ?

In this case the final structure will be:

postgresql-k8s: channel: 14/edge charm: postgresql-k8s constraints: arch=amd64 resources: oci-image: docker://registry.jujucharms.com/charm/kotcfrohea62xreenq1q75n1lyspke0qkurhk/postgresql-image@sha256:3abdbc00413b065fbfea8c6a3aaad8e137790ebb3e7bf5e1f42e19cbc1861926 oci-password: MDAxOGxvY2F0aW9uIGNoYXJtc3RvcmUKMDAzMGlkZW50aWZpZXIgOTBlODUwYWU2MzllNTZmMGU2OTc5MmYyOTQzZWY4ZDYKMDA1NGNpZCBpcy1kb2NrZXItcmVwbyBjaGFybS9rb3RjZnJvaGVhNjJ4cmVlbnExcTc1bjFseXNwa2UwcWt1cmhrL3Bvc3RncmVzcWwtaW1hZ2UKMDAxM2NpZCBhbGxvdyBwdWxsCjAwMmZzaWduYXR1cmUghzwoR8THj8usK6ytvyxvGT7UbLKl/fS9K16+68B6wUEK oci-username: docker-registry postgresql-image: 164 revision: 398

I would keep them close => by using the close label names

What do you mean by this? :-)

What about oci-image + oci-creds (or even split creds it to oci-username and oci-password ) ?

We can split if you prefer - my initial motivation was to make it as easy as possible for users to copy/paste it into the skopeo command:

skopeo copy docker://<oci-image> --src-creds <src-creds>

Oh, you named them to match skopeo. Sold.

I would keep them close => by using the close label names

What do you mean by this? :-)

Forget. I meant to keep YAML elements close to each other for better YAML readability by humans:

oci-image: ... oci-password: ... oci-username: ... postgresql-image: 164

VS

oci-image: ... postgresql-image: 164 src-creds: ...

But matching to skopeo kind of make sense in light of https://documentation.ubuntu.com/snap-store-proxy/en/airgap-charmhub/#export-oci-images

This PR aims to condense both previous patches #237 and #235 in a single, experimental `_update_bundle.yaml` workflow that will pin both snap and rock resource versions, in accordance to what was previously discussed: - For rocks: Roughly follows John Meinel's proposed format, with `oci-*` lines commented in order to preserve compatibility with `juju deploy`. See the following PR commit: canonical/postgresql-k8s-bundle@2683a2d - For snaps: follows existing [store proxy standard](https://documentation.ubuntu.com/snap-store-proxy/en/airgap-charmhub/#export-snap-resources). See the following PR commit: canonical/postgresql-bundle@b223dc3 See also test runs on: - mysql bundle VM: canonical/mysql-bundle@612d531 - mysql bundle K8s: canonical/mysql-k8s-bundle@018fae7 Important info: - `storage-admin` currently has 2 blocking issues: - Pinning of charm revisions is being ignored: https://bugs.launchpad.net/snapstore-client/+bug/2083876 - Charm resource (e.g. rocks) version pinning not supported: https://bugs.launchpad.net/snapstore-client/+bug/2083878 - This is `amd64` only. An eventual patch for `arm64` should be straightforward. - This is specific to SQL bundles and their associated charms. This is the case because snap pinning requires specific logic per-charm, see point below. - This fetches snap revisions by individual parsing of each charm's source code (not by deploying the bundle, like on previous patch) --------- Co-authored-by: Carl Csaposs <[email protected]>

lucasgameiroborges added 2 commits September 18, 2024 12:03

add oci image check

a8a1e4a

fix typo

2c70291

lucasgameiroborges changed the title ~~[WIP] Add oci image check to update_bundle.yaml~~ patch(update_bundle.yaml): Add resource pinning of OCI images for bundles Sep 20, 2024

lucasgameiroborges marked this pull request as ready for review September 20, 2024 14:33

lucasgameiroborges requested review from carlcsaposs-canonical and taurus-forever September 20, 2024 14:33

taurus-forever reviewed Sep 20, 2024

View reviewed changes

make only 1 call to store api

a220fd0

lucasgameiroborges requested a review from taurus-forever September 20, 2024 19:41

lucasgameiroborges commented Sep 20, 2024

View reviewed changes

carlcsaposs-canonical reviewed Sep 23, 2024

View reviewed changes

refactor

11b5e73

lucasgameiroborges requested a review from carlcsaposs-canonical September 26, 2024 03:29

lucasgameiroborges commented Sep 26, 2024

View reviewed changes

taurus-forever reviewed Sep 26, 2024

View reviewed changes

refactor

a24c7c8

lucasgameiroborges requested a review from taurus-forever October 2, 2024 03:17

carlcsaposs-canonical mentioned this pull request Oct 2, 2024

breaking: Remove update_bundle.yaml #238

Merged

lucasgameiroborges mentioned this pull request Oct 3, 2024

patch: Add _update_bundle.yaml #239

Merged

lucasgameiroborges closed this Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patch(update_bundle.yaml): Add resource pinning of OCI images for bundles #235

patch(update_bundle.yaml): Add resource pinning of OCI images for bundles #235

lucasgameiroborges commented Sep 18, 2024 •

edited

Loading

taurus-forever left a comment

lucasgameiroborges commented Sep 20, 2024

lucasgameiroborges commented Sep 20, 2024

lucasgameiroborges Sep 20, 2024 •

edited

Loading

carlcsaposs-canonical left a comment

carlcsaposs-canonical Sep 23, 2024

lucasgameiroborges Sep 23, 2024

carlcsaposs-canonical Sep 23, 2024

lucasgameiroborges Sep 26, 2024

carlcsaposs-canonical Sep 23, 2024

lucasgameiroborges Sep 23, 2024

carlcsaposs-canonical Sep 23, 2024

lucasgameiroborges Sep 23, 2024 •

edited

Loading

lucasgameiroborges Sep 23, 2024 •

edited

Loading

lucasgameiroborges Sep 24, 2024

lucasgameiroborges Sep 24, 2024 •

edited

Loading

carlcsaposs-canonical Sep 25, 2024

lucasgameiroborges Sep 25, 2024

lucasgameiroborges Sep 26, 2024

lucasgameiroborges Sep 26, 2024

taurus-forever left a comment

taurus-forever Sep 26, 2024

lucasgameiroborges Sep 26, 2024

taurus-forever Sep 26, 2024

		metadata = yaml.safe_load(response.json()["default-release"]["revision"]["metadata-yaml"])
		return channel_map, metadata

patch(update_bundle.yaml): Add resource pinning of OCI images for bundles #235

patch(update_bundle.yaml): Add resource pinning of OCI images for bundles #235

Conversation

lucasgameiroborges commented Sep 18, 2024 • edited Loading

taurus-forever left a comment

Choose a reason for hiding this comment

lucasgameiroborges commented Sep 20, 2024

lucasgameiroborges commented Sep 20, 2024

lucasgameiroborges Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

carlcsaposs-canonical left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasgameiroborges Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

lucasgameiroborges Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasgameiroborges Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taurus-forever left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasgameiroborges commented Sep 18, 2024 •

edited

Loading

lucasgameiroborges Sep 20, 2024 •

edited

Loading

lucasgameiroborges Sep 23, 2024 •

edited

Loading

lucasgameiroborges Sep 23, 2024 •

edited

Loading

lucasgameiroborges Sep 24, 2024 •

edited

Loading