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

cincinnati: define "payload" semantics #18

Closed
lucab opened this issue May 27, 2019 · 6 comments · Fixed by #32
Closed

cincinnati: define "payload" semantics #18

lucab opened this issue May 27, 2019 · 6 comments · Fixed by #32

Comments

@lucab
Copy link
Contributor

lucab commented May 27, 2019

Following up from @jlebon comment at #17 (comment).

Right now we plan to use the sha-checksum as "payload" field in cincinnati graph. However, rpm-ostree can understand multiple kinds of identifiers (current: revision and version), so it would be nice to have some kind of explicit scheme. This would help in removing update-graph ambiguities and in future-proofing it.

Alternative approaches are:

  • require a mandatory type-prefix followed by a =, e.g. revision=...
  • add a key-value pair in metadata table to specify the scheme/prefix
@lucab
Copy link
Contributor Author

lucab commented May 31, 2019

@bgilbert @arithx this is partially related to coreos/fedora-coreos-tracker#98 (comment), so your feedback would be welcome.

The goal would be to have some room for future-proofing, in case we need to change what the client consider a "payload identifier". The feedback gathering is about 1) whether it's worth doing it 2) how that should be encoded in the Cincinnati graph.

@jlebon
Copy link
Member

jlebon commented May 31, 2019

However, rpm-ostree can understand multiple kinds of identifiers

One note, this is just an rpm-ostree deploy and rpm-ostree rebase UX syntax thing. It's not really a deeper concept than that (e.g. we don't write down things in that syntax anywhere). In general the checksum is really the fundamental thing we care about.

That said, the idea of a scheme in the cincinnati data makes sense to me regardless. My strawman is just keeping the payload as the checksum, but in the metadata having:

  • a scheme key set to e.g. "checksum"
  • an ostree-checksum key
  • an ostree-version key

?

The ostree- ones are intended to be stable keys from which you can always get that info (and having the version there is important for visualizing the graph using versions if possible).

@bgilbert
Copy link
Contributor

bgilbert commented Jun 4, 2019

I don't think the type prefix would actually change much. If we need to add a prefix later, we can do it without ambiguity; and doing so will break old clients whether we have a prefix or not. But if folks would prefer a prefix, there's little harm to it.

I think the ostree-* metadata items make sense. scheme is subject to the same argument as the above, though, so I'd favor leaving it out.

@jlebon
Copy link
Member

jlebon commented Jun 4, 2019

It mostly comes down to failure modes in old clients. E.g. with a scheme key, an old Zincati can explicitly say something like "unknown scheme foo, expected checksum" rather than going blind and trying to pass the payload string as-is to rpm-ostree (which would then try to resolve it as a version, and so traverse the full OSTree branch history looking for it before erroring out).

@bgilbert
Copy link
Contributor

bgilbert commented Jun 5, 2019

That's fair. I don't have strong opinions here.

@lucab
Copy link
Contributor Author

lucab commented Jun 11, 2019

Thanks all for the feedback. I'm going to wrap up, and follow @jlebon suggestions regarding metadata keys. Minor variation I'll have is to prefix our own metadata keys with org.fedoraproject.coreos, so that custom middle-layers can have their own safe space to operate (that's what we do for Openshift backend too).

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 a pull request may close this issue.

3 participants