Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

feat: Add devstack ami playbook ISRE-1389 #6827

Merged
merged 50 commits into from
Oct 16, 2023
Merged

Conversation

jdmulloy
Copy link
Contributor

@jdmulloy jdmulloy commented Oct 26, 2022

This pull request adds the ansible framework to update a pre-existing hosted devstack AMI. It also adds support for Jammy (ubuntu 22.04) into the common aws role.

Make sure that the following steps are done before merging:

  • A SRE team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  • Are you adding any new default values that need to be overridden when this change goes live? If so:
    • Update the appropriate internal repo (be sure to update for all our environments)
    • If you are updating a secure value rather than an internal one, file a SRE ticket with details.
    • Add an entry to the CHANGELOG.
  • If you are making a complicated change, have you performed the proper testing specified on the Ops Ansible Testing Checklist? Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  • Think about how this change will affect Open edX operators. Have you updated the wiki page for the next Open edX release?

@adzuci adzuci requested a review from thomty November 2, 2022 19:15
@adzuci
Copy link
Contributor

adzuci commented Nov 2, 2022

@jdmulloy Could we add a description to this PR and do you feel it's ok to merge early and add on or should this be a draft PR?

@jdmulloy jdmulloy force-pushed the jdmulloy/cloud_dev_stack_ami branch 2 times, most recently from 620e96a to 8b407f6 Compare November 3, 2022 19:50
@jdmulloy
Copy link
Contributor Author

jdmulloy commented Nov 3, 2022

@jdmulloy Could we add a description to this PR and do you feel it's ok to merge early and add on or should this be a draft PR?

It's not ready. I had to put stuff up on github to pull it down into minikube with ArgoCD/GoCD. I find it easier to work with Pull Requests and to have the link for tickets. I could have just pushed the branch I suppose.

List from https://docs.docker.com/engine/install/ubuntu/ includes
pacakges docker-ce docker-ce-cli containerd.io docker-compose-plugin
@adzuci
Copy link
Contributor

adzuci commented Jun 9, 2023

@jdmulloy I chatted about this PR with @johnnagro and @thomty this week and think it's a key piece to easing some of the pain that new engineers face. Is it ok if I book some time with you and @thomty to better understand what you think needs to happen to it before it can be merged?

@thomty thomty changed the title feat: Add stub devstack ami playbook ISRE-1389 feat: Add devstack ami playbook ISRE-1389 Oct 12, 2023
@thomty thomty requested review from adzuci and removed request for thomty October 12, 2023 15:48
insertafter: EOF
dest: /home/ubuntu/.ssh/known_hosts
content: |
github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl
Copy link
Contributor

Choose a reason for hiding this comment

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

Will these ever need to be updated and should we add a comment about where we got them from?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's likely a better way to pull these in an automated fashion.

They're pulled from https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/githubs-ssh-key-fingerprints

Definitely an optimization available.

@@ -74,13 +74,13 @@
state: "present"
when: >
ansible_distribution in common_debian_variants and
(ansible_distribution_release != 'bionic' and ansible_distribution_release != 'focal')
(ansible_distribution_release == 'precise' or ansible_distribution_release == 'trusty' or ansible_distribution_release == 'xenial')
Copy link
Contributor

Choose a reason for hiding this comment

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

For my own understanding this is because we are using Ubuntu 22.04 (jammy?) and this code only needs to run on three older versions of Ubuntu.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. Changing logic from not running on bionic or focal, to running only on precise, trusty, or xenial. Anything older than those 3 is well out of LTS release range.

Copy link
Contributor

@adzuci adzuci left a comment

Choose a reason for hiding this comment

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

LGTM

@thomty thomty merged commit 3aa70b9 into master Oct 16, 2023
3 of 4 checks passed
@thomty thomty deleted the jdmulloy/cloud_dev_stack_ami branch October 16, 2023 14:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants