ansible-builder icon indicating copy to clipboard operation
ansible-builder copied to clipboard

Invalid requirements.txt leads to container built incorrectly

Open john-westcott-iv opened this issue 2 years ago • 3 comments

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.

john-westcott-iv avatar Apr 07 '22 17:04 john-westcott-iv

We will look into it!

eqrx avatar Apr 19 '22 13:04 eqrx

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.

ansiblejunky avatar Sep 26 '22 23:09 ansiblejunky

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 = []

sivel avatar May 09 '23 18:05 sivel