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

ansible-galaxy run in a container for stability #157

Merged
merged 2 commits into from Apr 10, 2018

Conversation

literalice
Copy link
Contributor

@literalice literalice commented Apr 10, 2018

In the README document, the run.sh must need only docker runtime, but actually we need to install ansible-galaxy in a machine directly for executing run.sh .

I think ansible-galaxy should be run in a container same as ansible-playbook command.

run.sh Outdated
@@ -45,10 +43,12 @@ else

fi

DOCKER_RUN_COMMAND="yum -y install git && ansible-galaxy install -r /tmp/src/requirements.yml --roles-path=/tmp/src/galaxy && $DOCKER_RUN_COMMAND"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sherl0cks
Copy link
Contributor

galaxy is in the container, but there is a regression in the applier container - it no longer ships git. We'll get this fixed upstream. redhat-cop/openshift-applier#18

@oybed ☝️

@sherl0cks
Copy link
Contributor

sherl0cks commented Apr 10, 2018

@literalice can you update the script to consume v3.9.0 tag from here instead: https://hub.docker.com/r/redhatcop/openshift-applier/tags/?

That tag should have the bug fixed.

@oybed
Copy link
Contributor

oybed commented Apr 10, 2018

@literalice @sherl0cks as the script doesn't specify a specific tag of the container image, this should just work as-is now with the latest image.

However, I'd recommend updating the galaxy requirements file to use the v3.9.0 tag for other purposes.
https://github.com/rht-labs/labs-ci-cd/blob/master/requirements.yml

@sherl0cks
Copy link
Contributor

just like we are pinning tags in other places, I'd like to pin it here too

@literalice
Copy link
Contributor Author

@sherl0cks @oybed Thank you for promptly handling the matter with care!
I agree to specifying the tag for preventing unexpected regression.
I've updated the PR, so would you check it?

@sherl0cks
Copy link
Contributor

@literalice I'm running a test now. run.sh, due to its nature, doesn't run through our CI. But I am actually testing by recreating the CI infra with your change. Should have feedback shortly.

@sherl0cks
Copy link
Contributor

@literalice looks good. Thanks for the change! Do let us know if you have any issues or feature requests.

@sherl0cks sherl0cks merged commit e6b3604 into rht-labs:master Apr 10, 2018
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

3 participants