-
Notifications
You must be signed in to change notification settings - Fork 59
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
hashi_vault collection - add support for python requirements discovery in ansible-builder
#105
Conversation
Allows ansible-builder to automatically discover python library requirements for execution environments.
Hello again @erinn ! I'm going to take a closer look into builder and other collections that publish a Additionally, I need to update that as well re: I'd also like to see a changelog fragment for this, so that other builder/AWX users such as yourself will see it in release notes. Thank you! |
Codecov Report
@@ Coverage Diff @@
## main #105 +/- ##
=======================================
Coverage 83.52% 83.52%
=======================================
Files 16 16
Lines 892 892
Branches 87 87
=======================================
Hits 745 745
Misses 129 129
Partials 18 18 Continue to review full report at Codecov.
|
azure.azcollections is one example that uses the meta/execution-environments.yml file, vmware_rest is another and they use requirements.txt just for reference. I'll add a fragment. I can't of course say for sure what is correct, but it seems to be working for some other folks. |
0001-prepare-builder-requirements-CI-changelog.patch.txt Hey @erinn thanks for your patience, I've looked into this a little bit, and I've made some changes. I was not able to push them into your branch (you may not have ticked the box to allow maintainers to make changes when you created the PR), so I'm including a Summary
attachment contents: From 0db74699dd0c2704ad177b7ff9c9a84608519556 Mon Sep 17 00:00:00 2001
From: Brian Scholer <1260690+briantist@users.noreply.github.com>
Date: Sun, 11 Jul 2021 13:39:30 -0400
Subject: [PATCH] prepare builder requirements, CI, changelog
---
.github/workflows/ansible-builder.yml | 40 +++++++++++++++++++
.../fragments/105-support-ansible-builder.yml | 3 ++
meta/ee-requirements.txt | 4 ++
meta/execution-environment.yml | 4 ++
requirements.txt | 1 -
5 files changed, 51 insertions(+), 1 deletion(-)
create mode 100644 .github/workflows/ansible-builder.yml
create mode 100644 changelogs/fragments/105-support-ansible-builder.yml
create mode 100644 meta/ee-requirements.txt
create mode 100644 meta/execution-environment.yml
delete mode 100644 requirements.txt
diff --git a/.github/workflows/ansible-builder.yml b/.github/workflows/ansible-builder.yml
new file mode 100644
index 0000000..5213ecf
--- /dev/null
+++ b/.github/workflows/ansible-builder.yml
@@ -0,0 +1,40 @@
+---
+name: ansible-builder
+on:
+ push:
+ paths:
+ - 'meta/execution-environment.yml'
+ - 'meta/ee-requirements.txt'
+ pull_request:
+ paths:
+ - 'meta/execution-environment.yml'
+ - 'meta/ee-requirements.txt'
+ schedule:
+ - cron: '0 13 * * *'
+
+env:
+ NAMESPACE: community
+ COLLECTION_NAME: hashi_vault
+
+jobs:
+ builder:
+ name: ansible-builder requirements
+ runs-on: ubuntu-latest
+ steps:
+ - name: Check out code
+ uses: actions/checkout@v2
+ with:
+ path: ansible_collections/${{ env.NAMESPACE }}/${{ env.COLLECTION_NAME }}
+
+ - name: Set up Python
+ uses: actions/setup-python@v2
+ with:
+ python-version: 3.9
+
+ - name: Install ansible-builder
+ run: pip install ansible-builder
+
+ # this is kind of a naive check, since we aren't comparing the output to anything to verify
+ # so the only we'll catch with this is an egregious error that causes builder to exit nonzero
+ - name: Verify Requirements
+ run: ansible-builder introspect --sanitize .
diff --git a/changelogs/fragments/105-support-ansible-builder.yml b/changelogs/fragments/105-support-ansible-builder.yml
new file mode 100644
index 0000000..431e4c8
--- /dev/null
+++ b/changelogs/fragments/105-support-ansible-builder.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+ - hashi_vault collection - add ``execution-environment.yml`` and a python requirements file to better support ``ansible-builder`` (https://github.com/ansible-collections/community.hashi_vault/pull/105).
diff --git a/meta/ee-requirements.txt b/meta/ee-requirements.txt
new file mode 100644
index 0000000..a9d32a7
--- /dev/null
+++ b/meta/ee-requirements.txt
@@ -0,0 +1,4 @@
+# ansible-builder doesn't seem to properly handle "; python_version" type of constraints
+# requirements here are assuming python 3.6 or higher
+hvac >=0.10.6
+urllib3 >= 1.15
diff --git a/meta/execution-environment.yml b/meta/execution-environment.yml
new file mode 100644
index 0000000..c899493
--- /dev/null
+++ b/meta/execution-environment.yml
@@ -0,0 +1,4 @@
+---
+version: 1
+dependencies:
+ python: meta/ee-requirements.txt
diff --git a/requirements.txt b/requirements.txt
deleted file mode 100644
index 3e6a595..0000000
--- a/requirements.txt
+++ /dev/null
@@ -1 +0,0 @@
-hvac
--
2.31.1
|
Hmm interesting, the option is checked to allow edits, so I am not sure what is going on there. What you've got looks good, I can pull it in or we can try to figure out this 'Allow edits by maintainers' thing. |
I most likely did something wrong then 😅 but yeah go ahead and pull it in whichever way is easiest. Thanks! |
ansible-builder
heh confirmed it was me who did not push correctly 🤦 just added a small change to add in the AWS requirements after some feedback in IRC (thanks @tadeboro ) |
# ansible-builder doesn't seem to properly handle "; python_version" type of constraints | ||
# requirements here are assuming python 3.6 or higher | ||
hvac >=0.10.6 | ||
urllib3 >= 1.15 | ||
|
||
boto3 # these are only needed if inferring AWS credentials or | ||
botocore # using a boto profile; including for completeness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why these requirements are not in the requirements.txt
file at the collection's root? This way, we do not need the meta/execution-environment.yml
file at all since the requirements.txt
file is automatically used by ansible-builder as a source of dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tadeboro that's what @erinn had originally proposed; there currently isn't one in the root. I wanted to point at the one used by the integration/unit tests since it already existed, but I found that builder doesn't handle constraints correctly, like this:
hvac >= 0.10.6, != 0.10.12, != 0.10.13, < 1.0.0 ; python_version == '2.7' # bugs in 0.10.12 and 0.10.13 prevent it from working in Python 2
hvac >= 0.10.6 ; python_version >= '3.5'
Resulted in a single line like this:
hvac >=0.10.6, >= 0.10.6, != 0.10.12, != 0.10.13, < 1.0.0
So I opted to make a separate one for this purpose for the time being that assumes py >= 3.6
edit to add: (I may revisit that in v2 of the collection when py2 & 3.5 will be dropped)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, EEs use Python 3.8, so those markers are usually not needed. But maybe builder should learn how to deal with them? Anyhow, this PR looks good and I think it is ready to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tadeboro so interestingly enough, I suspected it might be the case that some standard version or range of versions was used for EE's, and I tried to verify it but could not find it in the docs. If you happen to know where that is, I would really appreciate a link!
In any case, thanks again for reviewing.
@erinn thanks for bringing this to my attention, I'll be looking to ensure the collection is EE-ready and supported, please don't hesitate to raise any issues you see with that. I have some other changes I'm planning to wrap up soon that will be released with this change in 1.3.2, so I hope to maybe release that by this weekend or so. |
SUMMARY
Allows ansible-builder to automatically discover python library requirements for execution environments.
ISSUE TYPE
COMPONENT NAME
None
ADDITIONAL INFORMATION
ansible-builder documentation for collections.. This work should not have any effect on anything else, but simply makes it easier to build execution environments for ansible tower/awx.