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 rpm-ostree rebase registry:quay.io/coreos/fedora-coreos:stable #2909

Closed
cgwalters opened this issue Jun 16, 2021 · 10 comments · Fixed by #2940
Closed

Add rpm-ostree rebase registry:quay.io/coreos/fedora-coreos:stable #2909

cgwalters opened this issue Jun 16, 2021 · 10 comments · Fixed by #2940
Assignees
Labels
jira for syncing to jira

Comments

@cgwalters
Copy link
Member

cgwalters commented Jun 16, 2021

This is a key part of coreos/fedora-coreos-tracker#812

Basically:

$ rpm-ostree rebase registry:quay.io/coreos/fedora-coreos:stable
...
$ rpm-ostree status 
State: idle
BootedDeployment:
* registry:quay.io/coreos/fedora-coreos:stable
                Version: 34.20210614.dev.0 (2021-06-14T15:34:20Z)
                 Digest: registry:quay.io/coreos/fedora-coreos@sha256:41af286dc0b172ed2f1ca934fd2278de4a1192302ffa07087cea2682e7d372e3
...
$ dnf upgrade
Pulling registry:quay.io/coreos/fedora-coreos:stable
Found update: 
  Digest: registry:quay.io/coreos/fedora-coreos@sha256:41af286dc0b172ed2f1ca934fd2278de4a1192302ffa07087cea2682e7d372e3
  Version: 34.20210710.dev.0 (2021-07-10T11:23:15Z)
...
Run `reboot` to initiate a reboot.
$

Implementing this will follow in the path of the removed rojig:// code - we need to serialize this to the origin, plumb it through into status, etc. Call out to the same code behind rpm-ostree ex-container info to determine whether a new container image is available, etc.

@cgwalters
Copy link
Member Author

For symmetry, we should probably also add something like rpm-ostree stateroot-deploy registry:quay.io/coreos/fedora-coreos:stable which would replace this code.

@cgwalters cgwalters added the jira for syncing to jira label Jun 17, 2021
@cgwalters
Copy link
Member Author

I see this one as dependent-ish on #2743 - basically if we can avoid adding more things to the origin file and instead only add to the treefile, it aids #2326 and will reduce our long term maintenance burden. Let's try to get that in?

@cgwalters
Copy link
Member Author

(Eh, on second thought no need to block on #2743 - all we should do short term is emit warnings, and it won't be too hard to extend things later)

@kelvinfan001 kelvinfan001 self-assigned this Jun 23, 2021
@cgwalters
Copy link
Member Author

cgwalters commented Jun 24, 2021

One thing we need to debate and decide on is whether we can actually use registry:quay.io/coreos/fedora-coreos:stable and have that mean "a container image" and not "fetch from an ostree remote named registry the branch name quay.io/coreos/fedora-coreos:stable".

Luckily, : is not valid in refspecs, so we can disambiguate that way. Unfortunately, it means we can't use the common and convenient shortcut of omitting a tag meaning :latest. Users would get a very surprising error then when trying to rpm-ostree rebase registry:quay.io/someorg/exampleos.

Now, in skopeo current releases the main spelling of registry: is really docker:// but they wanted to get away from that and I'm pretty sure there's a PR to have the container stack support registry: but I'm not finding it right now.

In the short term, we could just require docker:// which would fix the ambiguity because ostree refs cannot contain //.

Alternatively...we could hack things and require registry:// for example, but that would be inventing a new thing that skopeo doesn't handle.

@kelvinfan001
Copy link
Member

kelvinfan001 commented Jun 25, 2021

In the short term, we could just require docker:// which would fix the ambiguity because ostree refs cannot contain //.

My impression is that registry: is "the future" so it feels a bit weird to introduce as a hard requirement something that "looks dated" (docker://).

Edit: ok, just found out that skopeo doesn't actually even support registry: yet, so I think this is actually a decent option; but what about rebasing to container images e.g. in the local containers-storage?

Alternatively...we could hack things and require registry:// for example, but that would be inventing a new thing that skopeo doesn't handle.

In my opinion, I also feel like inventing a new thing would make it a bit more confusing.

For now I've made it a requirement to have a container:// prefix, but that could be easily changed later. What were the downsides of having this requirement again (other than it being a bit less ergonomic to type)?

@cgwalters
Copy link
Member Author

For now I've made it a requirement to have a container:// prefix, but that could be easily changed later. What were the downsides of having this requirement again (other than it being a bit less ergonomic to type)?

I think the downside is it's a prefix specific to us, and not something understood by skopeo. I mean it's fine to keep short term as part of a WIP, but to me the value of this whole effort is trying to align more with the container ecosystem in large and small ways.

As a counter though, I suspect very few users in practice actually use skopeo, everyone uses podman run quay.io/foo/bar and not podman run docker://quay.io/foo/bar. But specifically for rpm-ostree ex-container we need to support e.g. serializing to oci-archive: and other things, so alignment with skopeo at that level is good.

@kelvinfan001
Copy link
Member

kelvinfan001 commented Jun 25, 2021

I see. The code currently looks like it was set up for taking in a "refspec type", but then I realized that it is the way it is because of rojig, and IIUC rojig never really became a thing, so I'm guessing rpm-ostree users never got used to typing something like rpm-ostree rebase ostree://fedora:fedora/x86_64/coreos/stable, and so having to add e.g. a container:// prefix would seem less natural of a transition when using container image references rather than OSTree refspecs.

In addition to not being able to take advantage of the omitted-tag-meaning-:latest shorcut, I noticed that skopeo and friends take in the syntax transport:details, where the details side seems quite unpredictable, so doing something like disambiguating by looking at the number of colons seems not so reliable in my opinion (as you mentioned, we don't want to be the next Norway :) ). So far it looks like all the skopeo transports that ostree-rs-ext supports can take in a tag, but will we possibly be supporting more transports whose details might not always allow for containing a colon?

@kelvinfan001
Copy link
Member

I think a natural next step after this would be supporting upgrade and deploy while based on a container image.
I feel like for upgrade we'd just try to import whatever container image the tag specified in the origin file points to. On the deploy side, would it be feasible to support deploy taking in a version number or commit hash while based on an image ref? Or will users who want to deploy to a specific commit just need to know the corresponding image digest and do a rebase?

@cgwalters
Copy link
Member Author

I think a natural next step after this would be supporting upgrade and deploy while based on a container image.
I feel like for upgrade we'd just try to import whatever container image the tag specified in the origin file points to.

Yep!

On the deploy side, would it be feasible to support deploy taking in a version number or commit hash while based on an image ref? Or will users who want to deploy to a specific commit just need to know the corresponding image digest and do a rebase?

I think let's only support container image digests to start. Anything else would be too hard to justify as it'd require us inventing some new out of band metadata.

@jlebon
Copy link
Member

jlebon commented Jun 29, 2021

I like the idea of matching skopeo, though to make it really explicit and avoid confusing errors, maybe we can just add a switch, e.g. rpm-ostree rebase registry:...quay.io/... --oci? So then more generally down the line we could support more of skopeo's transports without worrying about collisions/misinterpretations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira for syncing to jira
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants