Skip to content

downgrade urllib3 - #338

Merged
thraxil merged 1 commit into
appsembler/hawthorn/masterfrom
backups-pin-urllib
Jan 8, 2021
Merged

downgrade urllib3#338
thraxil merged 1 commit into
appsembler/hawthorn/masterfrom
backups-pin-urllib

Conversation

@thraxil

@thraxil thraxil commented Jan 8, 2021

Copy link
Copy Markdown

The backups script for GCP installs a bunch of python libraries system-wide. One of those as a side-effect now results in a version of urllib3 getting installed system-wide that then breaks ansible on subsequent deploys:

https://appsembler.atlassian.net/wiki/spaces/ED/pages/390004931/Failed+to+install+repo+key

A proper fix would be to move the whole backups script setup into a virtualenv so we don't get conflicts. But since this is only a problem on Hawthorn (Juniper uses a newer version of Ansible that is OK with upgrading urllib3), the simple workaround for now is to just downgrade urllib3 back to a known safe version at the end of the task.

The backups script for GCP installs a bunch of python libraries
system-wide. One of those as a side-effect now results in a version of
`urllib3` getting installed system-wide that then breaks ansible on
subsequent deploys:

https://appsembler.atlassian.net/wiki/spaces/ED/pages/390004931/Failed+to+install+repo+key

A proper fix would be to move the whole backups script setup into a
virtualenv so we don't get conflicts. But since this is only a problem
on Hawthorn (Juniper uses a newer version of Ansible that is OK with
upgrading urllib3), the simple workaround for now is to just downgrade
urllib3 back to a known safe version at the end of the task.
@thraxil
thraxil marked this pull request as ready for review January 8, 2021 10:59

@OmarIthawi OmarIthawi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM.

If we're going to merge appsembler/hawthorn/master into appsembler/juniper/master later, this task will sweep in. So I recommend adding a variable to it that defaults to False.

- name: pin urllib3 back down to a version that doesn't break ansible
pip: name={{ item.name }} version={{ item.version }}
with_items:
- {name: urllib3, version: 1.13.1}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a nicer look.

Suggested change
- {name: urllib3, version: 1.13.1}
- name: urllib3
version: 1.13.1

pip: name={{ item.name }} version={{ item.version }}
with_items:
- {name: urllib3, version: 1.13.1}
when: BACKUPS_PROVIDER == 'gs'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In case we forgot to remove this in Juniper.

Suggested change
when: BACKUPS_PROVIDER == 'gs'
when:
- BACKUPS_PROVIDER == 'gs'
- BACKUPS_URLLIB3_DOWNGRADE | default(False)

@thraxil

thraxil commented Jan 8, 2021

Copy link
Copy Markdown
Author

@OmarIthawi I don't see any reason that we'd merge appsembler/hawthorn/master into appsembler/juniper/master.

@thraxil
thraxil merged commit b15eee7 into appsembler/hawthorn/master Jan 8, 2021
@thraxil
thraxil deleted the backups-pin-urllib branch January 8, 2021 14:20
@OmarIthawi

OmarIthawi commented Jan 8, 2021

Copy link
Copy Markdown

It's likely that we'll need to sync changes from hawthorn to juniper similar to what we do in the edx-platform repo: appsembler/edx-platform#795

Cherry-picking is also a viable solution, but it's easy to forget those changes. We've piled few changes so far:

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.

2 participants