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

Don't exclude galaxy.yml from collection when doing build #68383

Closed
geerlingguy opened this issue Mar 20, 2020 · 10 comments
Closed

Don't exclude galaxy.yml from collection when doing build #68383

geerlingguy opened this issue Mar 20, 2020 · 10 comments
Assignees
Labels
affects_2.11 feature This issue/PR relates to a feature request. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@geerlingguy
Copy link
Contributor

geerlingguy commented Mar 20, 2020

Instead of ignoring galaxy.yml when building collections, let's embrace it!

SUMMARY

Currently there are a few new features being baked into collections like:

PR New metadata file
#67291 meta/action_groups.yml
#67684 meta/main.yml
#67684 meta/runtime.yml
#67684 meta/routing.yml

The general problem I have is we're starting to see an explosion of metadata files, and not only will this be hard to document and find a good pattern for in ansible/ansible, it will be hard for collection maintainers to remember all the various places different types of collection metadata should go.

In discussing this with @jborean93 on IRC, he mentioned one possible solution is to ship the galaxy.yml with the collection, and use that file as the home for all collection metadata. It already has like 99% of it, so why not store everything in one place?

There's precedent: Node.js/NPM package.json, PHP/Composer composer.json, Ruby/Bundler Gemfile...

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

Collections

ADDITIONAL INFORMATION

Related: https://github.com/ansible-collections/kubernetes/issues/58

@ansibot
Copy link
Contributor

ansibot commented Mar 20, 2020

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Mar 20, 2020
@jborean93
Copy link
Contributor

I think this is a good idea we can continue to create the MANIFEST.json and use that when parsing the collection metadata during install but we should use the existing galaxy.yml for runtime information.

The benefit of using just galaxy.yml is

  • We continue to just have 1 file for collection metadata instead of multiple files
  • The existing galaxy.yml has a doc that is auto generated so any additional features should have docs by default instead of manually having to maintain it
  • Has the ability to specify complex data like lists and dicts in yaml so we could theoretically put anything there
  • We can potentially use it as an alternative way to detect collection info on install paving way for installation from source control repos or other locations

@geerlingguy
Copy link
Contributor Author

We can potentially use it as an alternative way to detect collection info on install paving way for installation from source control repos or other locations

That could enable #61680

@nitzmahone
Copy link
Member

I'm not totallly opposed to this... My primary concern was dealing with the dual files of galaxy.yml/files.json (which one take precedence when both exist?), and having to load and ignore all the file metadata at runtime from files.json... But if we're willing to rally around galaxy.yml for all runtime needs (which means we have to start preserving it in thepublished artifacts), it's less of an issue, and solves some other issues as well. So let's talk.

@jborean93
Copy link
Contributor

My primary concern was dealing with the dual files of galaxy.yml/files.json (which one take precedence when both exist?)

My thought around this was to ignore MANIFEST.json and FILES.json except when publishing the collection, installing from an actual Galaxy endpoint, or using verify to verify the file hashes. These scenarios are separate from runtime behaviour around routing and default groups which are the 2 PRs that have proposed adding a meta/<file>.yml file for each of their configs. This was we keep galaxy.yml as the source of truth for ansible side config and only use the json equivalent files when we deal with Galaxy.

The only sticky bit here would be verify and whether we eventually add something like verification at runtime.

Using galaxy.yml moves us away from Ansible requires a built artifact to run way of thinking. IMO there is no real reason why we need to enforce this as running from checkout works perfectly fine just the way of doing this is not ideal and this proposed change will help us make that better.

@bcoca
Copy link
Member

bcoca commented Mar 24, 2020

So i would:

  • Put 'collection level' meta in manifest.json/galaxy.yml (create function that reads them keeping manifest as higher precedence for installed vs 'working copy')
  • use meta/ files for plugin specific data, routing.yml and action_groups.yml at least

Putting everything in one file does not scale well in the end and creates a very complex format. While having simple specific files for certain features seems easier for content authors in the end.
A single file is convenient when there is not a lot of data, but the galaxy.yml can turn into a huge file with very complex formats if we try to stuff everything into it, see a merged community.general version .. then think on how that will grow over time.

Another note, right now we don't really read the contents of galaxy.yml except to build manifest and we don't really read manifest except at install time for dependencies. At runtime we use 'both files' to figure out if something is a collection but only by existence, not by reading contents.

@geerlingguy
Copy link
Contributor Author

Putting everything in one file does not scale well in the end and creates a very complex format.

@bcoca - Yes and no... I'm already getting confused about where certain metadata about my collection should go. Part of that is the fact that things are not all standardized yet, but part of it is that it seems like there are currently already a couple locations for metadata, and (as mentioned in the original comment on this issue) it looks like there eventually may be 3 or 4 (or more) files to stash metadata inside.

@nitzmahone
Copy link
Member

nitzmahone commented Mar 27, 2020

Some of that was also sort of by design, eg: 99% of people won't know or care what meta/action_groups.yml is for, and even meta/routing.yml and meta/exec_env.yml will likely only be of interest to a small subset of people that build collections. The routing metadata for some collections could end up being large someday if lots of things move out/deprecate, ditto for things like file metadata, potential future docs, etc. If we just assume "one file for all", it could get pretty gnarly for complex collections like community.general, and the various runtimes that consume them will have to parse past a lot of junk they don't care about (to say nothing of their human maintainers ;) ).

So it's a bit of "damned if you do..." IMO. I can live with it either way if there's a lot of support for galaxy.yml for everything, but it wouldn't be my first choice.

Bigger picture: we need to get this resolved pretty soon... Maybe something to bring up at this weekend's EU contrib summit @gundalow ?

@bcoca bcoca changed the title Don't ignore galaxy.yml when building collections Don't exclude galaxy.yml from collection when doing build Apr 14, 2020
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 14, 2020
@sivel
Copy link
Member

sivel commented Apr 14, 2020

We really need to sit down and create a full PRD for how collections should be handled. We're not looking the whole picture, and we need to think through all of this properly. I've assigned this to @thaumos as he is working on similar documentation.

cc @s-hertel

@nitzmahone
Copy link
Member

Hasn't been any movement here from the PM side of things, so going to just close for now.

@ansible ansible locked and limited conversation to collaborators Mar 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.11 feature This issue/PR relates to a feature request. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

No branches or pull requests

8 participants