Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

docs/dev: introduce "Distribution Point" concept #2953

Merged
merged 1 commit into from
Aug 29, 2016

Conversation

sgotti
Copy link
Contributor

@sgotti sgotti commented Jul 20, 2016

A Distribution Point represents the method to fetch an image starting from an
input string.

Before this concept was introduced, rkt used the ImageType concept, the
ImageType was mapped to the input image string format and internally
covered multiple concepts like distribution, transport and image type
(hidden since all are now appc ACIs).

The distribution point concept will be used as the primary information in the
different layers of rkt (fetching but also for references in a CAS/ref store).

This patch introduces the distribution point concept. See
Documentation/devel/distribution_point.md for a detailed description.

@lucab
Copy link
Member

lucab commented Jul 20, 2016

/cc @dgonyeo (this is part of the store rework I mentioned)

@lucab
Copy link
Member

lucab commented Jul 20, 2016

Can we please put the PR details inside a markdown doc? Even if not final, it will make discussion easier.

Should the archive URI be split in one distribution type per format? Something like aciarchive ociimagelayoutarchive

Bikeshed level: I think so. I also think schemes should be scoped (as they are rkt specific) and be made future-proof. Something like:

  • rkt:appc:v=0:name...
  • rkt:docker:v=0:name...
  • rkt:aci-archive:v=0:ArchiveUrl...
  • rkt:oci-archive:v=0:ArchiveUrl...

Where we basically reserve for ourself a single scheme (rkt:) and we internally manage sub-schemes (in the form of format:k=v:).

@sgotti
Copy link
Contributor Author

sgotti commented Jul 21, 2016

Can we please put the PR details inside a markdown doc? Even if not final, it will make discussion easier.

Done, I put the details in a not final doc in Documentation/devel/distribution.md

Bikeshed level: I think so. I also think schemes should be scoped (as they are rkt specific) and be made future-proof. Something like:

rkt:appc:v=0:name...
rkt:docker:v=0:name...
rkt:aci-archive:v=0:ArchiveUrl...
rkt:oci-archive:v=0:ArchiveUrl...
Where we basically reserve for ourself a single scheme (rkt:) and we internally manage sub-schemes (in the form of format:k=v:).

This seems good. Version will be the sub-scheme version right? (to be able to update its internal format without putting the version directly in the subscheme and changing the subscheme name).
Should v=... be mandatory for every subscheme and with a fixed position (after the subscheme) or just a possible subscheme dependent k=v entry? If mandatory can we just use semantic versioning without a key=value for it like rkt:appc:v0.1:....?

@s-urbaniak? thoughts on the URI (as names) direction and the suggested formats?

@lucab
Copy link
Member

lucab commented Jul 21, 2016

Version will be the sub-scheme version right? (to be able to update its internal format without putting the version directly in the subscheme and changing the subscheme name).

Right (and yes).

Should v=... be mandatory for every subscheme and with a fixed position (after the subscheme) or just a possible subscheme dependent k=v entry?

The former (which in case can be expanded into the latter given a k=v array, but I prefer not going that route if there's no need to)

If mandatory can we just use semantic versioning without a key=value for it like rkt:appc:v0.1:....?

I would avoid complexity and just keep a uint. This is mostly an internal format and we stick a version there "just in case".

@sgotti
Copy link
Contributor Author

sgotti commented Jul 22, 2016

Bikeshed level: I think so. I also think schemes should be scoped (as they are rkt specific)

Since my hope is that all this effort and others to decouple different layers will be shared between multiple tools (acbuild, new tools etc...) I won't make them too much rkt specific. If we want to go with an unique scheme I'll try to find a more generic name ("distribution" ?).

@lucab
Copy link
Member

lucab commented Jul 22, 2016

Since my hope is that all this effort and others to decouple different layers will be shared between multiple tools (acbuild, new tools etc...) I won't make them too much rkt specific. If we want to go with an unique scheme I'll try to find a more generic name ("distribution" ?).

So the owner of this would be some kind of pkg/container-distribution and not rkt itself, right? If so, just reflect it in some mnemonic way in the scheme. Maybe codi: or similar, your choice. My concerns are just:

  • we should pick a single scheme-root that we can reserve for ourself
  • we should stop squatting somebody else generic scheme (like appc: or docker:)

(I hope we are not starting to over-design here)

@cgonyeo
Copy link
Member

cgonyeo commented Jul 22, 2016

Since my hope is that all this effort and others to decouple different layers will be shared between multiple tools (acbuild, new tools etc...) I won't make them too much rkt specific.

It would be ideal to share this with acbuild at the very least. Is there a difference between distributions here and distribution handlers in #2964? If not, it sounded like distribution handlers are responsible for interacting with rkt's store, which means that if an external project wants to use these distribution handlers it would also have to use rkt's store logic.

I'm not sure if distribution is the best name for this concept

I think distribution is fine.

@sgotti
Copy link
Contributor Author

sgotti commented Jul 26, 2016

? If not, it sounded like distribution handlers are responsible for interacting with rkt's store, which means that if an external project wants to use these distribution handlers it would also have to use rkt's store logic.

That's a good point.
I'm not sure how it could be decoupled from the store:

  • for example in oci the distribution should check in the store if a blob already exists or it needs to be fetched. Doing a fetching phase without the store will mean fetching all blobs also if already available.
  • In proposal: Fetchers refactor #2964 also the transport cache should interact with the store to see if the blob for the specified transport url exists.

One idea will be to define a (read-write) store interface with the required CAS and ref methods (WriteBlob, ReadBlob, ListBlobs, HasBlob, SetRef ...). I have to experiment a little bit more with the store refactor to see if this will work or if some image type differences (ACI, OCI image) will force us to use dedicated store functions per image type (some example can be handling ACI manifest cache to avoid extracting the manifest every time its needed from an image),

@sgotti sgotti force-pushed the distribution_uri branch 2 times, most recently from 1face78 to 3b689e2 Compare July 28, 2016 19:26
@sgotti
Copy link
Contributor Author

sgotti commented Jul 28, 2016

Updated with the proposed changes:

  • A unique global scheme (now cimd... Please find a better name that recalls container image and distribution 😅 . cid already exists)
  • Moved archive to aci-archive (no more FORMAT parameter). In future there'll be an oci-image-layout-archive.
  • Removed SimpleString() from the distribution interface since it's better leaving this to the distribution user (there may been different ways for a simple string) like is done for the conversion from a simple string to the distribution uri. The docker distribution provides a SimpleDockerString that generates what an user will expect (removing default index, default repo when on default index and tag if latest, see the tests).
  • More tests.

@sgotti sgotti force-pushed the distribution_uri branch 2 times, most recently from f7fbb8b to ed95894 Compare July 29, 2016 15:54
@sgotti sgotti mentioned this pull request Aug 1, 2016
3 tasks
@sgotti sgotti changed the title [WIP] rkt: Introduce distribution concept rkt: Introduce distribution concept Aug 1, 2016
@sgotti
Copy link
Contributor Author

sgotti commented Aug 22, 2016

As suggested, and to speed up things, I split the proposal (this PR) from the implementation (#3101).

@s-urbaniak thanks for your comments on the implementation! Due to the split they got lost in #3101, can you please copy them there?

@s-urbaniak
Copy link
Contributor

@sgotti copy-pasted my comments in #3101 :-)

@@ -0,0 +1,147 @@

**NOTE:** I'm not sure if distribution is the best name for this concept or it should be called with other names like "image reference". So I'd like to hear your thoughts.
Copy link
Member

Choose a reason for hiding this comment

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

I guess we can settle on calling them "Distribution Points". Current name looks fine enough, we just need to disambiguate it from typical "Linux distro" usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Distribution Points looks good. I was also thinking to just Distribution URIs.

A Distribution Point represents the method to fetch an image starting from an
input string.

Before this concept was introduced, rkt used the ImageType concept, the
ImageType was mapped to the input image string format and internally
covered multiple concepts like distribution, transport and image type
(hidden since all are now appc ACIs).

The distribution point concept will be used as the primary information in the
different layers of rkt (fetching but also for references in a CAS/ref store).

This patch introduces the distribution point concept. See
Documentation/devel/distribution_point.md for a detailed description.
@sgotti sgotti changed the title rkt: Introduce distribution concept rkt: Introduce Distribution Point concept Aug 23, 2016
@sgotti
Copy link
Contributor Author

sgotti commented Aug 23, 2016

@lucab Thanks for the review! I used the Distribution Point name and I hope to have addressed all your comments.

@s-urbaniak
Copy link
Contributor

@lucab ping for final pass

@lucab
Copy link
Member

lucab commented Aug 29, 2016

I think this doc needs quite a bit of re-wording and polishing, but we are more interested in its content and the concepts it contains. Let's land it now and implement this. We will need to re-touch this anyway to show how to use them in practice. LGTM.

@lucab lucab changed the title rkt: Introduce Distribution Point concept docs/dev: introduce "Distribution Point" concept Aug 29, 2016
@lucab lucab merged commit 63241d1 into rkt:master Aug 29, 2016
@lucab lucab mentioned this pull request Nov 24, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants