Skip to content

Conversation

@heyealex
Copy link
Contributor

Overhaul of the install_ansible startup script runner. The goal of this update is to provide a more generalizable process for installing python, pip and ansible on a variety of VM OS images and environments.

This has been tested against the following VM images using the startups script test_configs:

  • debian-10
  • centos8
  • hpc-centos7
  • ubuntu 20.04
  • rocky8

Other OS images have been tested against the install_ansible.sh script alone (i.e not testing against follow-on runners like install-nfs.sh)

Submission Checklist

  • Have you installed and run this change against pre-commit? (pre-commit install)
  • Are all tests passing? (make tests)
  • Have you written unit tests to cover this change?
  • Is unit test coverage still above 80%?
  • Have you updated all applicable documentation?
  • Have you followed the guidelines in our Contributing document?

@heyealex heyealex requested a review from nick-stroud August 18, 2022 17:19
@heyealex heyealex force-pushed the update-ansible-install-script branch from a7629f5 to 2a1a51b Compare August 18, 2022 20:55
@heyealex heyealex force-pushed the update-ansible-install-script branch 3 times, most recently from d0556ff to 7afbb11 Compare August 24, 2022 23:30
@heyealex heyealex assigned nick-stroud, tpdownes and cboneti and unassigned heyealex Aug 25, 2022
@heyealex
Copy link
Contributor Author

I believe everything has been addressed. Please take a look again when you get a chance.

Rework of the install ansible script to generalize better and force
consistent versions.
Activates and deactivates the ghpc venv before running ansible-local
runners.
Fixes to the install_ansible script after running against the
test_configs/*-ss.yaml tests.
* Correct -f to -d check for ghpc-venv directory
* Use ansible-playbook without a full path - it should be loaded with
  the venv or be available already.
* Use apt-get rather than apt for a more consistent interface.

Updates to the test_configs/*-ss.yaml blueprints
* Remove test build cache to allow for actual tests to run successfully
* Remove image from nfs-server where failures occured (it assumes yum
  exists)
* Add spack dependency install playbook

Fixes to other scripts
* return rather than exit
Addressed feedback, including:
* Installing pip with itself
* Adding a variable for required versions of ansible and pip
* Reorganizing conditionals
* Other various fixes that came up during testing
@heyealex heyealex force-pushed the update-ansible-install-script branch from 78026ba to 72ea643 Compare August 25, 2022 00:03
@heyealex heyealex force-pushed the update-ansible-install-script branch 2 times, most recently from c5be948 to f5e81de Compare August 25, 2022 16:51
@heyealex heyealex force-pushed the update-ansible-install-script branch from f5e81de to 5aea917 Compare August 25, 2022 17:49
install_python_deps() {
if [ -f /etc/debian_version ] || grep -qi ubuntu /etc/lsb-release 2>/dev/null ||
grep -qi ubuntu /etc/os-release 2>/dev/null; then
apt-get install -y python3-distutils
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the functionality you are looking for from distutils?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran into a dependency when upgrading pip in ubuntu and debian that required distutils to be installed first.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd put a comment somewhere in the script that captures this. It's not obvious from the package description:

https://packages.debian.org/bullseye/python3-distutils

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

fi
if [ -z "${ansible_version}" ] || [ "${ansible_major_vers}" -ne "${ansible_req_major_vers}" ] ||
[ "${ansible_minor_vers}" -lt "${ansible_req_minor_vers}" ]; then
${python_path} -m pip install ansible==${REQ_ANSIBLE_PIP_VERSION}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is making the choice to install 4.10.0 on all platforms (aligning all platforms to the last version compatible with CentOS 7 and Rocky 8). That is a reasonable choice. It might also be reasonable to install whatever pip thinks is the best version. Anyhow, I think this is a choice we should all understand.

In particular, I cannot find documentation for 4.10.0 at https://docs.ansible.com/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pip package 4.10.0 will install version 2.11 of ansible-core, which I am able to find listed in the documentation: https://docs.ansible.com/ansible-core/2.11/index.html

It is however the oldest version listed currently.

I'm inclined to keep the hard version in place for now and investigate installing as python -m pip install ansible>=4.10.0 when we can run further tests against it (I'll a task for it in the backlog). How do you all feel about that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that idea as the best way to make progress.

- Install ansible into this virtual environment if the current version of
ansible is not version 2.11 or higher.
To use the virtualenv created by this script, you can activate it by running the
Copy link
Contributor

Choose a reason for hiding this comment

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

please use "virtual environment"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Document the installation of python3-distutils and provide instructions
on pointing to the correct python interpreter when using
ansible-playbook.
nick-stroud
nick-stroud previously approved these changes Aug 25, 2022
@nick-stroud nick-stroud assigned heyealex and unassigned nick-stroud Aug 25, 2022

# Activate ghpc-venv virtual environment if it exists
if [ -d /usr/local/ghpc-venv ]; then
source /usr/local/ghpc-venv/bin/activate
Copy link
Member

Choose a reason for hiding this comment

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

This is a sad side repercussion of having virtual env. Should we rather fix all scripts that expect ansible, or just install in the system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I don't like this.
Right now, I'm tempted to pull the plug on the virtual environment for now and put it back in later when I/we have a better understanding of the effects.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You could do what @tpdownes had done before which is to still install into a virtual env but then symbolic link to the that virtual env, exposing it to the system: example.

I am also fine to do that as a follow on as we have people waiting for a newer version of ansible as of this moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is also possible to invoke any pip-installed binary with a full path. It will do the "right thing" in terms of Python libraries e.g. swap /usr/bin/ansible-playbook for /usr/local/ghpc-venv/bin/ansible-playbook.

@heyealex heyealex merged commit 5e36eb1 into GoogleCloudPlatform:develop Aug 26, 2022
@heyealex heyealex deleted the update-ansible-install-script branch November 29, 2022 23:22
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.

4 participants