Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Scaffold a requirements.yml file for collection dependencies #2652

Merged
merged 4 commits into from
Mar 17, 2020

Conversation

fabianvf
Copy link
Member

Users will now install the operator_sdk.util and community.kubernetes
collections at operator build time rather than them being baked into
the base image. This allows us to put version pins to ensure
compatibility of user Ansible content even when updating the base image.
This should make it safer and easier for users to update to newer
versions of the base image, as well as make it more transparent where
the Ansible content they execute is coming from. It also allows us to
push new features and bugfixes out to users via the collections without
having to continually re-release the base image with new content.

This will be a breaking change for existing users of the collection
however. In order to continue working, users will have to add a
requirements.yml to their projects and add an install step to their
build/Dockerfile.

Description of the change:

Motivation for the change:

Users will now install the operator_sdk.util and community.kubernetes
collections at operator build time rather than them being baked into
the base image. This allows us to put version pins to ensure
compatibility of user Ansible content even when updating the base image.
This should make it safer and easier for users to update to newer
versions of the base image, as well as make it more transparent where
the Ansible content they execute is coming from. It also allows us to
push new features and bugfixes out to users via the collections without
having to continually re-release the base image with new content.

This will be a breaking change for existing users of the collection
however. In order to continue working, users will have to add a
requirements.yml to their projects and add an install step to their
build/Dockerfile.
@@ -95,6 +97,9 @@ RUN TINIARCH=$(case $(arch) in x86_64) echo -n amd64 ;; ppc64le) echo -n ppc64el
&& curl -L -o /tini https://github.com/krallin/tini/releases/latest/download/tini-$TINIARCH \
&& chmod +x /tini

[[- if .Requirements ]]
COPY requirements.yml ${HOME}/requirements.yml
RUN ansible-galaxy collection install -r ${HOME}/requirements.yml[[ end ]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 done

@@ -73,13 +76,12 @@ RUN yum clean all && rm -rf /var/cache/yum/* \
&& pip3 install --no-cache-dir --ignore-installed ipaddress \
ansible-runner==1.3.4 \
ansible-runner-http==1.0.0 \
openshift==0.9.2 \
openshift~=0.10.0 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add entry in the changelog over the upgrade of openshift version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 done

@@ -95,6 +97,9 @@ RUN TINIARCH=$(case $(arch) in x86_64) echo -n amd64 ;; ppc64le) echo -n ppc64el
&& curl -L -o /tini https://github.com/krallin/tini/releases/latest/download/tini-$TINIARCH \
&& chmod +x /tini

[[- if .Requirements ]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add entry in the changelog over requirements. Is not it a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is not a breaking change, but existing users will indeed need to add a requirements file to their existing projects to pull in the operator_sdk.util collection and community.kubernetes collection. Added to changelog and migration guide

@@ -1,5 +1,8 @@
FROM quay.io/operator-framework/ansible-operator:dev
FROM quay.io/operator-framework/ansible-operator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not here it still being dev tag?
To use the image built in the CI?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah whoops, good catch that was a change for local testing

Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants