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

Invalid requirements.txt leads to container built incorrectly #364

Open
john-westcott-iv opened this issue Apr 7, 2022 · 3 comments
Open

Comments

@john-westcott-iv
Copy link
Member

If you reference a requirements.txt for python modules but it has invalid syntax ansible-builder build will make the image but the python modules will not be installed. Even when running -v 3 you wont see any errors with the build process. After some time of digging I found the introspect command in the Containerfile and running this like ansible-builder introspect --sanitize --user-pip=requirements.txt --user-bindep=bindep.txt --write-bindep=/tmp/src/bindep.txt --write-pip=/tmp/src/requirements.txt finally revealed my syntax error:

# Sanitized dependencies for /usr/share/ansible/collections
Warning: failed to parse requirments from user, error: Invalid requirement, parse error at "'=4.0.30'"
---
python: []
system:
- 'unixODBC  # from collection user'
- 'unixODBC-devel  # from collection user'
- 'gcc  # from collection user'
- 'gcc-c++  # from collection user'
- 'python38-devel  # from collection user'

Creating parent directory for /tmp/src/bindep.txt

In my case I did module=version instead of module==version but this lead to a great deal of frustration trying to figure out why my modules did not exist in the container. I realize that this particular issue was user induced is on my end, but it led me to wish for a mechanism that would allow me to see when there are warnings or errors like these instead of an incomplete container build.

@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Apr 7, 2022
@eqrx eqrx removed the needs_triage New item that needs to be triaged label Apr 19, 2022
@eqrx
Copy link
Contributor

eqrx commented Apr 19, 2022

We will look into it!

@Akasurde Akasurde changed the title Invalid requrements.txt leads to container built incorrectly Invalid requirements.txt leads to container built incorrectly Apr 26, 2022
@ansiblejunky
Copy link

I just hit a similar problem, where the ansible-builder introspect command that ansible-builder runs shows an error in the resulting file but this never produced a non-zero result in the overall build process so I never thought there was an issue... cuz the build succeeded. But there definitely was an issue! So I've since added this to my build to manually call that ansible-builder introspect command to manually check for the output and produce non-zero result myself.

The ask would be to have ansible-builder ensure this is properly handled.

@sivel
Copy link
Member

sivel commented May 9, 2023

This is still the case, I wonder if we shouldn't make the warning an error instead:

diff --git a/src/ansible_builder/_target_scripts/introspect.py b/src/ansible_builder/_target_scripts/introspect.py
index 0d84e84..8f2ab43 100644
--- a/src/ansible_builder/_target_scripts/introspect.py
+++ b/src/ansible_builder/_target_scripts/introspect.py
@@ -345,7 +345,8 @@ def sanitize_requirements(collection_py_reqs):
                 consolidated.append(req)
                 seen_pkgs.add(req.name)
         except Exception as e:
-            logger.warning('Warning: failed to parse requirements from %s, error: %s', collection, e)
+            logger.error('failed to parse requirements from %s, error: %s', collection, e)
+            sys.exit(1)
 
     # removal of unwanted packages
     sanitized = []

Akasurde added a commit to Akasurde/ansible-builder that referenced this issue Jun 6, 2023
Fixes: ansible#364

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
Akasurde added a commit to Akasurde/ansible-builder that referenced this issue Aug 15, 2023
Fixes: ansible#364

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
Akasurde added a commit to Akasurde/ansible-builder that referenced this issue Aug 17, 2023
Fixes: ansible#364

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@Shrews Shrews linked a pull request Apr 12, 2024 that will close this issue
5 tasks
@Shrews Shrews removed a link to a pull request Apr 17, 2024
5 tasks
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.

4 participants