-
Notifications
You must be signed in to change notification settings - Fork 0
Update Deployment Workflows #14
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
Conversation
Why these changes are being introduced: This is the first step in migrating this repository to optionally build for either AMD64 or ARM64 CPU Architecture for deployment in AWS. How this addresses that need: * Update the `Makefile` with the new output from the mitlib-tf-workloads-ecr repository (and then make some further modifications that will be reflected back in the mitlib-tf-workloads-ecr repository soon) * Update the dev-build.yml workflow with the new output from the mitlib-tf-workloads-ecr repository * Update the stage-build.yml workflow with the new output from the mitlib-tf-workloads-ecr repository * Update the prod-promote.yml workflow with the new output from the mitlib-tf-workloads-ecr repository * Update the README with notes about building and deploying in AWS Side effects of this change: None. Since we did not create an `.aws-architecture` file, the builds for AWS will still default to AMD64 as before. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-1448
|
@MITLibraries/dataeng -- this is really for @ghukill to review (since I've already been in touch with him about this). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's looking good to me, but I had a question.
I have created a .aws-architecture file locally with linux/arm64. I have confirmed that make check-arch exits cleanly.
Then I perform a build with make dist-dev.
Lastly, I'm trying to start a container locally and pass a specific --platform. When I use --platform linux/amd64 it works, but I get the following error for linux/arm64:
$ docker run \
-p "2718:2718" \
-v "./tests/fixtures:/tmp/fixtures" \
-e NOTEBOOK_MOUNT="/tmp/fixtures/inline_deps" \
--platform linux/arm64 \
marimo-launcher-dev:latest
Unable to find image 'marimo-launcher-dev:latest' locally
docker: Error response from daemon: pull access denied for marimo-launcher-dev, repository does not exist or may require 'docker login': denied: requested access to the resource is deniedIt's almost as if docker run --platform linux/arm64 ... is unable to find an image that matches that platform? but I had built it with a .aws-architecture file. Am I missing something here?
I don't doubt it would work as expected in AWS, thinking mostly about local development when we are trying out different architectures.
| -t $(ECR_URL_DEV):`git describe --always` \ | ||
| -t $(ECR_NAME_DEV):latest . | ||
| ### Terraform-generated Developer Deploy Commands for Dev environment ### | ||
| check-arch: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this check-arch command. After some cycles with all this, might be neat to weave this into either pre-commit, or make lint, or something.
|
|
||
| dist-dev: check-arch ## Build docker container (intended for developer-based manual build) | ||
| @ARCH_TAG=$$(cat .arch_tag); \ | ||
| docker buildx inspect $(ECR_NAME_DEV) >/dev/null 2>&1 || docker buildx create --name $(ECR_NAME_DEV) --use; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cabutlermit - so you did end up going with buildx? Am I correct in remembering you were kind of waffling between approaches for a bit? Sounds like maybe this approach won out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I still ended up withdocker buildx in the end for the Makefile commands (the shared workflows only use docker build). I kept buildx in the Makefile, mostly because I wanted to ensure that the make commands would build either AMD64 or ARM64 regardless of the CPU architecture of the developer's machine.
Ah, interesting, a followup. I did get this to work by using Here are my images: Maybe my question is: why does it appear that the |
Ahh... So, this is not fully documented yet, but will be when we get to the next phase is the procoess (steps 4 - ... in the Confluence doc). For now, let's avoid trying to build for a different architecture since the focus of this is to switch to the new But, to your question: as oon as you introduce the In the long run, if you ever see a container in ECR with |
Makes sense! Thanks. |
ghukill
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Looking forward to following and working through next steps.
Thanks for the question response, better understanding it's kind of a phased approach.
Yup. My approach to this is to have a plan (steps 1-3 in the Confluence doc) to move all our application repos to the new workflows without having to do any architecture testing or code changes. Just update the workflows and keep using Then, we move on to the optional phase of determining if it makes sense to move a repo from |
Purpose and background context
This is the first step in migrating this repository to optionally build for either AMD64 or ARM64 CPU Architecture for deployment in AWS. The following changes are made to this repository, to align with Steps 1-3 in How-To: Update a Container App to new CPU Architectire. Other than shifting this repository to the new shared workflows, this does not change anything for the container build nor the CPU architecture (which still defaults to
X86_64/AMD64.Makefilewith the new output from the mitlib-tf-workloads-ecr repository (and then make some further modifications that will be reflected back in the mitlib-tf-workloads-ecr repository soon)dev-build.ymlworkflow with the new output from the mitlib-tf-workloads-ecr repositorystage-build.ymlworkflow with the new output from the mitlib-tf-workloads-ecr repositoryprod-promote.ymlworkflow with the new output from the mitlib-tf-workloads-ecr repositoryREADMEwith notes about building and deploying in AWSNote: There were some minor modifications to the
Makefilethat are different from the currentMakefiletemplate in the mitlib-tf-workloads-ecr repository. The minor changes made here will appear in a new PR on that repo in the next day or two.How can a reviewer manually see the effects of these changes?
dev-build.ymlworkflow. Check themarimo-launcherECR Repository and verify that the build from the GHA workflow has been uploaded to ECR with the correct tags.make dist-dev,make publish-dev, andmake docker-clean. Check themarimo-launcherECR Repository and verify that the builds from theMakefilecommands were properly uploaded to ECR with the expected tags.Includes new or updated dependencies?
NO
Changes expectations for external applications?
NO
What are the relevant tickets?
Developer
Code Reviewer(s)