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

Add compatibility code to the entrypoint so new EEs work with old runner version #988

Merged

Conversation

AlanCoding
Copy link
Member

@AlanCoding AlanCoding commented Feb 4, 2022

In #957 we combined 2 callback names (awx_display, minimal) into just 1, awx_display. Fundamentally, this makes it hard for the containers to work with ansible-runner versions before/after, and vice versa. Ping review by @shanemcd

In that PR, I paid attention to the ability for current ansible-runner to work with old images.

I was failing to fully appreciate the implications of old ansible-runner not working with current images.

This PR adds a shim to the entrypoint so that the image with work even if used by old versions of ansible-runner. In the PR 957 I removed the use of the LAUNCHED_BY_RUNNER environment variable. This allows using that as a poor proxy for the version of ansible-runner that is starting the container.

@rebeccahhh pointed out to me that our tests for AAP controller 4.1.x were failing due to these problems. I've re-tested those with a custom image built from this branch, quay.io/alancoding/ansible-runner:marty_youve_got_to_come_back_with_me, and this gets them to pass.

There's no reasonable way to ever get test coverage, because the whole idea is compatibility with old versions. So I am working on running make test with that custom image tagged as quay.io/ansible/ansible-runner:devel while I have the commit f1b6c3a checked out, which was the last commit before the PR landed.

@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Feb 4, 2022
@AlanCoding
Copy link
Member Author

I think I've confirmed passing tests locally

2095 passed, 2 skipped, 1 xpassed in 143.30s (0:02:23)

With source tree

$ git status
HEAD detached at f1b6c3a

and images

(env) [alancoding@alan-red-hat ansible-runner]$ podman images quay.io/ansible/ansible-runner:devel
REPOSITORY                         TAG                                   IMAGE ID      CREATED       SIZE
quay.io/alancoding/ansible-runner  devel                                 526a9df2d92a  14 hours ago  887 MB
quay.io/alancoding/ansible-runner  marty_youve_got_to_come_back_with_me  526a9df2d92a  14 hours ago  887 MB
quay.io/ansible/ansible-runner     devel                                 526a9df2d92a  14 hours ago  887 MB
(env) [alancoding@alan-red-hat ansible-runner]$ docker images quay.io/ansible/ansible-runner:devel
REPOSITORY                       TAG       IMAGE ID       CREATED        SIZE
quay.io/ansible/ansible-runner   devel     bf1c2e76aa12   12 hours ago   859MB

the 14/12 hours ago would indicate the one built locally.

@AlanCoding AlanCoding marked this pull request as ready for review February 4, 2022 14:55
@AlanCoding AlanCoding requested a review from a team as a code owner February 4, 2022 14:55
Copy link
Member

@shanemcd shanemcd left a comment

Choose a reason for hiding this comment

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

Doh!

@shanemcd shanemcd added the gate label Feb 4, 2022
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

@ansible-zuul ansible-zuul bot merged commit 46a69a2 into ansible:devel Feb 4, 2022
@sivel sivel removed the needs_triage New item that needs to be triaged label May 16, 2023
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 this pull request may close these issues.

None yet

4 participants