Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Add execution environment metadata #211

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

AlanCoding
Copy link
Contributor

SUMMARY

We are building tooling for building container images with necessary python & system requirements to use content inside of collections.

https://github.com/ansible/ansible-builder

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

N/A

ADDITIONAL INFORMATION

The specific case here was a bit tricky, because in RHEL8-like platforms, the repos that provide kubernetes-client were not available. We plan to use this as a part of AWX, we make use of the connection plugin, and thus need the kubectl CLI.

The solution was that @shanemcd changed the base image from centos8 to fedora ansible/ansible-runner#502. This will install in fedora out-of-the-box. The way that bindep works, it just won't try to install it unless the platform is fedora. This is the only approach I came up with that respects the philosophy of bindep, in that there's a universal truth - "are you on fedora? great, then dnf install this". People on other platforms can get it with more steps, but we don't want to be opinionated about how at this early stage.

Other requirements came from the individual module/plugin requirements: entry, which I scraped here:

https://github.com/AlanCoding/collection-dependencies-demo/blob/dd568760a0ccc8fe5c38ca149e78e62ca6af50bb/sniff_req/discovered.json#L599-L606

Similar to the issue with kubectl, things which are more complex installs by the features in ansible-builder right now, so we can't add helm.

Add python requirements
Add kubectl system requirement strictly for Fedora
@geerlingguy
Copy link
Collaborator

geerlingguy commented Aug 28, 2020

Just speaking in a general sense—for Execution Environments to be a universally useful tool, wouldn't it be better to figure out a way to make them able to either autodiscover (I guess that's partly what's going on here) or users manually associate dependencies for collections when they build them?

I may also be missing something here, but it seems like the way this is architected, anyone maintaining a collection would need to do something similar if they wanted to make it so their collection works in EEs, and currently there's no documentation to that effect.

Alternatively, could this metadata be defined in the meta/ dir or in galaxy.yml so we don't have to have extra files that are (seemingly?) useless outside the EE use case?

@AlanCoding
Copy link
Contributor Author

We haven't published doc pages, but the section "Collection Execution Environment Dependencies" is intended to outline the options here:

https://github.com/ansible/ansible-builder/blob/devel/README.md

@AlanCoding
Copy link
Contributor Author

Alternatively, could this metadata be defined in the meta/ dir or in galaxy.yml so we don't have to have extra files that are (seemingly?) useless outside the EE use case?

I'm happy to add meta/execution-environments.yml which would point to these 2 files, but the tool is allowing that to be skipped for expedience right now, if they exist with these names.

Right now ansible/galaxy-importer#92 is up, which adds these to the galaxy-importer data. The galaxy.yml idea seems like not as good of a place compared to meta/ to me. On the other hand, MANIFEST.json seems appropriate, but I don't have a sketch of what that would look like.

@geerlingguy
Copy link
Collaborator

geerlingguy commented Aug 28, 2020

@AlanCoding - I think my primary concern (stepping out of my k8s collection maintainer shoes right now) is we've had an ongoing discussion for years in the Ansible community about the best way to define and install libraries required by plugins/other content.

I just want to make sure that whatever format we choose, it is able to be used by all, and not something that specifically caters to EEs and then the community is still left to figure out some other option in other use cases.

For example, see: ansible/proposals#57 and ansible/proposals#158. cc @gundalow @bcoca

@jillr
Copy link
Contributor

jillr commented Sep 15, 2020

The proposals mentioned are as you noted several years old. While I don't disagree that this is clearly conversation the community still wants to have, in the immediate term (ie; this cycle) we are implementing and shipping EEs. All of our supported and/or Ansible maintained collections are shipping with EE support and this requirements file format. We've known that EEs are coming and had ample opportunities to weight in on their architecture.

Given that this is the design that will be used for EEs in the near term, are there any aspects of this PR (the specific requirements detailed, etc) that need to be changed for this collection to work successfully in EEs, or a preferred file location or format that is already currently supported by EEs?

@AlanCoding
Copy link
Contributor Author

I'm happy to add an integration test to ansible-builder, but this is tricky without throwing in additional setup in terms of credentials or the need to stand up remote resources. For an example, see this playbook for another collection. Maybe we could load a config file like this?

config: "{{ lookup('file', 'service.yml') | from_yaml }}"

If that only requires writing the config file locally, then I can do that in the container, and this would be perfect verification that it's working.

On the other side of things, it would be a whole lot to ask to switch out the entire image building pipeline. Something I would like to look into is if we could have tests inherit python requirements from the top-level requirements.txt file. Like:

In terms of just testing the install, it has been getting indirectly tested as a part of this requirements file:

https://github.com/ansible/ansible-builder/blob/d6003f4bade48d17a25e82244ab5f205cc2a8987/test/data/awx/requirements.yml#L17-L19

@geerlingguy
Copy link
Collaborator

I'm okay with merging this for EE, but I still wish we had gone through a process of making this 'the solution' for community as well. As it is, it seems the only consumer of these files currently (and indeed, the only team that knows of their existence) is the Tower team internally at Red Hat.

In addition, I would really like to see this documented somewhere on docs.ansible.com (more discoverable).

@AlanCoding
Copy link
Contributor Author

Might be a bad time to mention this, but we started on https://ansible-builder.readthedocs.io/en/latest/

@codecov
Copy link

codecov bot commented Sep 16, 2020

Codecov Report

Merging #211 into main will increase coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #211      +/-   ##
==========================================
+ Coverage   42.41%   42.62%   +0.20%     
==========================================
  Files           3        4       +1     
  Lines         547      610      +63     
  Branches      111      122      +11     
==========================================
+ Hits          232      260      +28     
- Misses        271      303      +32     
- Partials       44       47       +3     
Impacted Files Coverage Δ
...s/community/kubernetes/plugins/module_utils/raw.py 42.49% <0.00%> (-1.21%) ⬇️
...ns/community/kubernetes/plugins/action/k8s_info.py 53.06% <0.00%> (ø)
...ommunity/kubernetes/plugins/module_utils/common.py 38.65% <0.00%> (+0.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9d8bf7...5447a71. Read the comment docs.

@AlanCoding
Copy link
Contributor Author

This will be a collection that we are referencing in the basic instructions on execution environments, and I would like to bump this to see if we could get it merged. I would love to hear new ideas about how to put this out better to community, and hook things up in testing. Right now, however, it's a bit of a standstill because I don't have any overwhelming consensus direction to go in, other than building out the image that we know is needed for AWX / Tower jobs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants