-
Notifications
You must be signed in to change notification settings - Fork 23
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
Upgrades the dependencies to latest versions #92
Conversation
@TrilokGeer Are you running into issues with https://github.com/ansible/ansible-runner-http being on an older version
|
&& yum install -y libffi-devel openssl-devel python39-devel gcc python39-pip python39-setuptools \ | ||
&& pip3 install --upgrade pip~=23.3.2 \ | ||
&& pip3 install pipenv==2023.11.15 \ | ||
&& pipenv install --deploy \ |
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.
Pipenv usage seems to contradict by installing packages to global path and also, blocks usage of different version when used with requires
version. As the container provides isolation, using pip installer performs the same installation on the container layers without conflicts.
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.
The following issue and PRs provide a context as to why Pipfile and Pipfile.lock were added:
- Provide a way to replicate Ansible/Python environment of Docker image operator-sdk#4237
- Pipenv operator-sdk#4494
- Use pipenv for python dependency management operator-sdk#4543
The idea is to have fixed versions of the dependencies as well through the Pipfile and Pipfile.lock so that it can be locally reproduced.
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.
The point here is about the conflicts with respect to package installation path. The package management is still being managed by pipenv with usage of pipfile and pipfile.lock. Using PIPENV_SYSTEM=1 enables installing the packages outside the virtual env set by pipenv. This resorts to using underlying system's python version than the desired version with which virtual env is instantiated.
&& yum install -y libffi-devel openssl-devel gcc python3.11-devel python3.11-pip python3.11-setuptools \ | ||
&& pip3.11 install --upgrade pip~=24.2 \ | ||
&& pip3.11 install pipenv \ | ||
&& pipenv requirements > requirements.txt \ |
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.
Using pipenv for now to reuse piplock.
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.
I think creating the requirements.txt file while building the image may have some issues while reproducing it locally as was the ask in operator-framework/operator-sdk#4237.
We can have fixed dependencies list beforehand and then use it while building the image. This can be achieved in 2 ways:
- Either we continue with the same method and generate the Pipfile.lock as mentioned here:
ansible-operator-plugins/images/ansible-operator/README.md
Lines 1 to 7 in 42b5d80
We build the base image using the Dockerfile, which validates the python requirements scaffolding that it copies from this directory. To update the requirements (`Pipfile` and `Pipfile.lock`) build and execute the image generated by `pipfile.Dockerfile` like so: 1. docker build -f ./pipfile.Dockerfile -t pipfile-generator . 2. docker run --rm -it -v .:/tmp/pip-airlock:Z pipfile-generator 3. Commit the newly-generated `Pipfile.lock` file (NB: this directory is in root .gitignore file, so you must `git add -f`) - Or we generate the requirements.txt file along with Pipfile.lock and then use the requirements.txt file during building the image.
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.
- Or we generate the requirements.txt file along with Pipfile.lock and then use the requirements.txt file during building the image.
The requirements.txt is generated based on Pipfile.lock, refer https://pipenv.pypa.io/en/latest/cli.html#requirements.
Regarding the issue mentioned, the dependencies are managed via pipfile.
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.
Ack. But I think the Pipfile.lock file is not updated after your last change to the Pipfile. I used both the files to generate the requirements.txt.backup file. After that I updated the Pipfile.lock using the latest Pipfile and then generated the requirements.txt. Following is the diff between the files:
# diff -y requirements.txt requirements.txt.backup
-i https://pypi.org/simple -i https://pypi.org/simple
annotated-types==0.7.0; python_version >= '3.8' annotated-types==0.7.0; python_version >= '3.8'
> ansible==10.4.0; python_version >= '3.10'
ansible-core==2.17.4; python_version >= '3.10' ansible-core==2.17.4; python_version >= '3.10'
ansible-runner==2.4.0; python_version >= '3.9' ansible-runner==2.4.0; python_version >= '3.9'
ansible-runner-http==1.0.0 ansible-runner-http==1.0.0
authlib==1.3.2; python_version >= '3.8' authlib==1.3.2; python_version >= '3.8'
cachetools==5.5.0; python_version >= '3.7' cachetools==5.5.0; python_version >= '3.7'
certifi==2024.8.30; python_version >= '3.6' certifi==2024.8.30; python_version >= '3.6'
cffi==1.17.1; platform_python_implementation != 'PyPy' cffi==1.17.1; platform_python_implementation != 'PyPy'
charset-normalizer==3.3.2; python_full_version >= '3.7.0' charset-normalizer==3.3.2; python_full_version >= '3.7.0'
click==8.1.7; python_version >= '3.7' click==8.1.7; python_version >= '3.7'
cryptography==43.0.1; python_version >= '3.7' cryptography==43.0.1; python_version >= '3.7'
docutils==0.21.2; python_version >= '3.9' docutils==0.21.2; python_version >= '3.9'
dparse==0.6.4b0; python_version >= '3.7' dparse==0.6.4b0; python_version >= '3.7'
filelock==3.12.4; python_version >= '3.8' filelock==3.12.4; python_version >= '3.8'
google-auth==2.35.0; python_version >= '3.7' | google-auth==2.34.0; python_version >= '3.7'
idna==3.10; python_version >= '3.6' | idna==3.8; python_version >= '3.6'
jinja2==3.1.4; python_version >= '3.7' jinja2==3.1.4; python_version >= '3.7'
kubernetes==29.0.0; python_version >= '3.6' kubernetes==29.0.0; python_version >= '3.6'
lockfile==0.12.2 lockfile==0.12.2
markdown-it-py==3.0.0; python_version >= '3.8' markdown-it-py==3.0.0; python_version >= '3.8'
markupsafe==2.1.5; python_version >= '3.7' markupsafe==2.1.5; python_version >= '3.7'
marshmallow==3.22.0; python_version >= '3.8' marshmallow==3.22.0; python_version >= '3.8'
mdurl==0.1.2; python_version >= '3.7' mdurl==0.1.2; python_version >= '3.7'
oauthlib==3.2.2; python_version >= '3.6' oauthlib==3.2.2; python_version >= '3.6'
packaging==24.1; python_version >= '3.8' packaging==24.1; python_version >= '3.8'
pexpect==4.9.0 pexpect==4.9.0
psutil==6.0.0; python_version >= '2.7' and python_version not psutil==6.0.0; python_version >= '2.7' and python_version not
ptyprocess==0.7.0 ptyprocess==0.7.0
pyasn1==0.6.1; python_version >= '3.8' pyasn1==0.6.1; python_version >= '3.8'
pyasn1-modules==0.4.1; python_version >= '3.8' pyasn1-modules==0.4.1; python_version >= '3.8'
pycparser==2.22; python_version >= '3.8' pycparser==2.22; python_version >= '3.8'
pydantic==2.9.2; python_version >= '3.8' | pydantic==2.9.1; python_version >= '3.8'
pydantic-core==2.23.4; python_version >= '3.8' | pydantic-core==2.23.3; python_version >= '3.8'
pygments==2.18.0; python_version >= '3.8' pygments==2.18.0; python_version >= '3.8'
python-daemon==3.0.1; python_version >= '3' python-daemon==3.0.1; python_version >= '3'
python-dateutil==2.9.0.post0; python_version >= '2.7' and pyt python-dateutil==2.9.0.post0; python_version >= '2.7' and pyt
pyyaml==6.0.2; python_version >= '3.8' pyyaml==6.0.2; python_version >= '3.8'
requests==2.31.0; python_version >= '3.7' requests==2.31.0; python_version >= '3.7'
requests-oauthlib==2.0.0; python_version >= '3.4' requests-oauthlib==2.0.0; python_version >= '3.4'
requests-unixsocket==0.3.0 requests-unixsocket==0.3.0
resolvelib==1.0.1 resolvelib==1.0.1
rich==13.8.1; python_full_version >= '3.7.0' rich==13.8.1; python_full_version >= '3.7.0'
rsa==4.9; python_version >= '3.6' and python_version < '4' rsa==4.9; python_version >= '3.6' and python_version < '4'
ruamel.yaml==0.18.6; python_version >= '3.7' ruamel.yaml==0.18.6; python_version >= '3.7'
ruamel.yaml.clib==0.2.8; python_version < '3.13' and platform ruamel.yaml.clib==0.2.8; python_version < '3.13' and platform
safety==3.2.7; python_version >= '3.7' safety==3.2.7; python_version >= '3.7'
safety-schemas==0.0.5; python_version >= '3.7' safety-schemas==0.0.5; python_version >= '3.7'
setuptools==75.1.0; python_version >= '3.8' | setuptools==74.1.2; python_version >= '3.8'
shellingham==1.5.4; python_version >= '3.7' shellingham==1.5.4; python_version >= '3.7'
six==1.16.0; python_version >= '2.7' and python_version not i six==1.16.0; python_version >= '2.7' and python_version not i
typer==0.12.5; python_version >= '3.7' typer==0.12.5; python_version >= '3.7'
typing-extensions==4.12.2; python_version >= '3.8' typing-extensions==4.12.2; python_version >= '3.8'
urllib3==1.26.20; python_version >= '2.7' and python_version urllib3==1.26.20; python_version >= '2.7' and python_version
websocket-client==1.8.0; python_version >= '3.8' websocket-client==1.8.0; python_version >= '3.8'
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.
Ah, good catch
@acornett21 , the issue did not surface while running the tests (manually and e2e). Though, there were other issues related to 2.32.0 version of requests (docker/docker-py#3256) and incompatibility between urllib3 version causing |
@TrilokGeer I actually meant this issue... oops |
Ah, yes! The issue is present when using 2.32.* versions. The requests version is pinned to 2.31.0 . |
/label tide/merge-method-squash |
b96b5d8
to
a309787
Compare
@TrilokGeer I think we can also update the versions of the ansible-collections here: ansible-operator-plugins/pkg/plugins/ansible/v1/scaffolds/internal/templates/requirements.go Lines 38 to 45 in 42b5d80
|
Thanks for the suggestion, the changes are still in progress. It'd be easier for you to take a look when all the tests are successful. |
images/ansible-operator/Pipfile
Outdated
urllib3 = "~=1.26.17" | ||
urllib3 = "~=1.26.2" |
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.
Are we downgrading the urllib3
version?
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.
The Pipfile.lock is generated with urllib3
pinned to 1.26.20
: https://github.com/operator-framework/ansible-operator-plugins/pull/92/files#diff-82452578a3e817b3b4dc5c154471041589c4b535216c59dbd8982bc18e2f8d32R880
safety = "==3.2.7" | ||
PyYAML = "==6.0.2" |
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.
Any specific reason why safety
and PyYAML
are pinned to the specific versions? I think these will be automatically upgraded to the latest version when we re-generate the Pipfile.lock from the Pipfile when they are not pinned to a specific version.
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.
These packages are pinned to have dependent pinned version of these packages as pip freeze is not being used. Pip freeze will result in a list of packages that generate version conflicts. Hence, using this way of installation of these packages pinned with latest possible versions.
@@ -2,7 +2,7 @@ | |||
# It is built with dependencies that take a while to download, thus speeding | |||
# up ansible deploy jobs. | |||
|
|||
FROM registry.access.redhat.com/ubi8/ubi:8.9-1107 AS basebuilder | |||
FROM registry.access.redhat.com/ubi9/ubi:9.4-1214 AS basebuilder |
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.
Since we are upgrading the ubi version as well as the python version, I think the pipfile.Dockerfile should also be updated with the similar changes here:
FROM registry.access.redhat.com/ubi8/ubi:8.9-1107 AS basebuilder | |
# Install Rust so that we can ensure backwards compatibility with installing/building the cryptography wheel across all platforms | |
RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | |
ENV PATH="/root/.cargo/bin:${PATH}" | |
RUN rustc --version | |
# Copy python dependencies (including ansible) to be installed using Pipenv | |
COPY ./Pipfile ./ | |
# Instruct pip(env) not to keep a cache of installed packages, | |
# to install into the global site-packages and | |
# to clear the pipenv cache as well | |
ENV PIP_NO_CACHE_DIR=1 \ | |
PIPENV_SYSTEM=1 \ | |
PIPENV_CLEAR=1 | |
# Ensure fresh metadata rather than cached metadata, install system and pip python deps, | |
# and remove those not needed at runtime. | |
RUN set -e && yum clean all && rm -rf /var/cache/yum/* \ | |
&& yum update -y \ | |
&& yum install -y libffi-devel openssl-devel python39-devel gcc python39-pip python39-setuptools \ | |
&& pip3 install --upgrade pip~=23.3.2 \ | |
&& pip3 install pipenv==2023.11.15 \ |
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.
Ah!, missed to check-in this file.
images/ansible-operator/Dockerfile
Outdated
&& pip3 install pipenv==2023.11.15 \ | ||
&& pipenv install --deploy \ | ||
&& yum install -y python3.11 \ | ||
&& yum install -y libffi-devel openssl-devel gcc python3.11-devel python3.11-pip python3.11-setuptools \ |
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.
Any specific reason to change to python3.11 and not latest python3.12?
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.
Historically during the development, there were package conflicts, I believe it was because of cached packages when trying manually. Hence the 3.11 got stuck. Nevertheless, 3.12 seems to work fine with no issues, upgraded the version.
- name: community.docker | ||
version: "3.10.3" | ||
version: "3.12.1" |
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.
The latest version of community.docker is 3.12.2
which was released last week (https://github.com/ansible-collections/community.docker/releases). Should we upgrade to it?
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.
Well, at the time of this work, the latest is 3.12.1., let's check with this upgrade.
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.
Completed the upgrade, all e2e cases are successful.
@@ -1,4 +1,4 @@ | |||
FROM registry.access.redhat.com/ubi8/ubi:8.9-1107 AS basebuilder | |||
FROM registry.access.redhat.com/ubi9/ubi:9.4-1214 AS basebuilder |
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.
Note: We should update operator-sdk's
images to the same version as here to not run into issues.
I'm not 100% sure, but I believe that the ansible-operator-plugins/.github/workflows/test-ansible.yml Lines 30 to 38 in 42b5d80
|
@arkadeepsen @chiragkyal If your concerns have been addressed would you mind resolving them? Also do you see any other issues with this? We'd like to merge this, and then iterate on the gorelease issue where the |
@acornett21 I am good with the changes. |
Signed-off-by: Trilok Geer <tgeer@redhat.com>
Signed-off-by: Trilok Geer <tgeer@redhat.com>
…e.Dockerfile Signed-off-by: Trilok Geer <tgeer@redhat.com>
Signed-off-by: Trilok Geer <tgeer@redhat.com>
aebc842
to
62f4f77
Compare
Closing the PR in favour of #101 |
Description of the change:
Upgrades the dependencies to latest versions of python, ansible-core and collections.
Motivation for the change:
As the packages are either not supported or nearing EOL by this year end, upgrading the packages for better maintenance and support.