Skip to content

Conversation

@cabutlermit
Copy link
Contributor

@cabutlermit cabutlermit commented Oct 13, 2022

What this PR does

  • Update Makefile with outputs from mitlib-tf-workloads-ecr for dev
  • Modify (slightly) the Makefile to adjust for S3 bucketname in Dev1
  • Update Pipfile to add the "name" argument for the pypi.org source
  • Add dev-build.yml workflow (from the mitlib-tf-workloads-ecr repo)
  • Add the stage-build.yml caller workflow
  • Add the prod-promote.yml caller workflow
  • Modify (slightly) the dev-build workflow to address the extra steps needed in the build process that aren't currently covered by our shared workflow
  • Remove the "test container build" step from the ci.yml workflow
  • Update README from ReStructuredText to Markdown

Side effects of this change

MAJOR: Once this change is merged into the main branch, we will not be able to redeploy this app into our legacy AWS account without rolling back to an earlier commit. It might make sense to tag a final "old" release to the last commit on main before this PR.

Helpful background context

The Carbon app and associated infrastructure needs to be migrated to our AWS Organization from our legacy AWS account. This will be done with little-to-no refactoring of the application itself. But, the repo does need some updates to match the automated deployment pipeline in our AWS Org.

The infrastructure is in place in the Dev1 AWS Account, and is ready for code review to push into Stage-Workloads and Prod-Workloads.

How can a reviewer manually see the effects of these changes?

  1. Checkout this branch locally, authenticate to the Dev1 AWS Account, and run make dist-dev. This will verify that a dev/test version of the container can be built with the new Makefile by a dev and pushed to Dev1 for testing.
  2. Via the AWS Console, create an ECR task manually for this container to see it run.
  3. Enable one of the EventBridge rules (which are disabled by default in Dev1) to verify that the "cron" runs of the task work successfully.
  4. The CloudConnector (a proxy between our AWS subnets and MIT IP-restricted servers) is not designed to fully work in Dev1. So, it is not possible to test the data reads from the MIT Data Warehouse from Dev1. Once this is merged to main we can do thorough testing from Stage-Workloads.

Related Jira Tickets

Includes new or updated dependencies?

NO

Developer

  • README is updated to reflect all changes as needed
  • All related Jira tickets are linked in commit message(s)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated to reflect all changes or is not needed
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Why these changes are being introduced:
The Carbon app and associated infrastructure needs to be migrated to
our AWS Organization from our legacy AWS account. This will be done
with little-to-no refactoring of the application itself. But, the
repo does need some updates to match the automated deployment pipeline
in our AWS Org.

How this addresses that need:
* Update Makefile with outputs from mitlib-tf-workloads-ecr for dev
* Modify (slightly) the Makefile to adjust for S3 bucketname in Dev1
* Update Pipfile to add the "name" argument for the pypi.org source
* Add dev-build.yml workflow (from the mitlib-tf-workloads-ecr repo)
* Modify (slightly) the dev-build workflow to address the extra steps
needed in the build process that aren't currently covered by our
shared workflow

Side effects of this change:
None.

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/IN-568
@cabutlermit cabutlermit force-pushed the awsorg branch 4 times, most recently from 76f438f to 3843eab Compare October 17, 2022 15:41
Why these changes are being introduced:
The new workflows for ECR deployment automation are now available in
the main branch of the shared .github repo. And, the updated outputs
are available in the mitlib-tf-workloads-ecr repo.

How this addresses that need:
* Update Makefile with latest output from mitlib-tf-workloads-ecr
* Update dev-build.yml with latest output from mitlib-tf-workloads-ecr
* Create stage-build.yml workflow
* Create prod-promote.yml workflow
* Remove the container build step from the ci.yml workflow (now that
we automate the build elsewhere)
* Remove the old stage workflow (it was linked to our legacy AWS
account)
* Updates to README

Related GitHub Tickets:

* https://mitlibraries.atlassian.net/browse/IN-568
* https://mitlibraries.atlassian.net/browse/IN-615
Why these changes are being introduced:
The `make install` command requires that pipenv is already installed.

* Add one more command to the PREBUILD to force the installation
of pipenv to both dev-build.yml and stage-build.yml
@cabutlermit
Copy link
Contributor Author

@hakbailey & @zotoMIT I'm tagging both of you on this to start with. Together with PR#1 on the infrastructure repo, this is first step of the migration of Carbon from legacy AWS to the AWS Org. Almost all of the changes here related to the deployment automation of the container, so to any actual changes to the code or functionality of the app itself.

Copy link

@zotoMIT zotoMIT left a comment

Choose a reason for hiding this comment

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

LGTM

@cabutlermit cabutlermit marked this pull request as ready for review October 26, 2022 14:41
Copy link
Contributor

@hakbailey hakbailey left a comment

Choose a reason for hiding this comment

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

This looks fine to me and it seems like all the workflows with the updated commands ran as expected. I was able to do a manual ECS task run in dev1 so the container is working, but it didn't complete due to a secrets manager permission...I assume that has to do with the infra changes not yet being applied and am ignoring it :)

Also, I agree that tagging a release before merging this PR seems like a good idea just in case.

@cabutlermit
Copy link
Contributor Author

it didn't complete due to a secrets manager permission...I assume that has to do with the infra changes not yet being applied and am ignoring it :)

That's actually a problem and I won't ignore it. 😁 There must be an IAM mistake somewhere in the infra code that I need to fix (but there shouldn't be anything to fix in this repo).

@cabutlermit
Copy link
Contributor Author

@hakbailey I found the mistakes in the Carbon infra and the most recent run of the Carbon task was successful enough (it could read the secret from Secrets Manager and publish messages to SNS). Once we get this into Stage-Workloads, we can do more testing of the Data Warehouse connection and the FTP upload.

@cabutlermit cabutlermit merged commit b43351c into main Oct 26, 2022
@hakbailey hakbailey deleted the awsorg branch January 23, 2023 16:27
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