Skip to content
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

Containerized deployment environment #2566

Closed
wants to merge 18 commits into from

Conversation

Alex-Lewandowski
Copy link

This PR is intended to simplify the HyP3 deployment process by organizing build parameters into a .env and providing a Docker image configured to deploy Lambdas with Python3.13 runtimes via CloudFormation.

Closes #2551

There is nothing included in this PR that prevents deploying HyP3 in the way maintainers always have, but it adds opt-in convenience. An example use-case is deploying HyP3 from a Mac, where iOS-specific Python package binaries may be incompatible with the AWS Lambda runtime.

  • Instructions updated in README.md
  • hyp3.example is a template used to create a deployment-specific hyp3.env file containing all build parameters as environment variables. Some vars are required and some are optional. CloudFormation will use default parameters for optional env vars that are commented or have empty values in hyp3.env. hyp3.env has been added to .gitignore to help prevent pushing semi-sensitive info to a public repo.
  • Dockerfile defines an image built off of amazon/aws-lambda-python:3.13, installs make and awscli, and copies in the hyp3 dir
  • Makefile updates:
    • All targets marked PHONY (since none produce output named for the target)
    • New make targets:
      • image: Creates an image from Dockerfile
      • shell: Runs a /bin/bash shell on the image, mounts the hyp3 dir, and pulls in user's AWS creds for the AWS_DEFAULT_PROFILE, as defined in hyp3.env, or defaults to the default profile. Assumes the user has AWS configured with a profile matching their provided AWS_DEFAULT_PROFILE.
      • package: Packages the CloudFormation template
      • deploy: Deploys HyP3 with CloudFormation
      • help: Lists all targets and their descriptions. Provides instructions for deploying with make
      • clean: Added removal of the deployment image if it is present, Docker is installed, and Docker Engine is running

To deploy with a deployment image:

  1. Configure a hyp3.env
  2. make image
  3. make shell
  4. make install
  5. make build
  6. make package
  7. make deploy

To do it the old way:

  1. Set up your local deployment environment
  2. make install
  3. make build
  4. Manually run aws cloudformation package ...
  5. Manually run aws cloudformation deploy ...

@Alex-Lewandowski Alex-Lewandowski requested a review from a team January 22, 2025 17:46
Copy link
Contributor

@jhkennedy jhkennedy left a comment

Choose a reason for hiding this comment

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

Overall, I like this! I haven't done an extensive review, but did jot down some thoughts for discussion.

hyp3.example Outdated Show resolved Hide resolved
@@ -61,52 +63,55 @@ Review the parameters in [cloudformation.yml](apps/main-cf.yml) for deploy time

### Deploy with CloudFormation

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth a note/admonishion here to say that manual deployments should only be done to temporary deploymetns, otherwise, they should be hooked into our CI/CD infrastructure...

I'll think on that a little.

Copy link
Contributor

Choose a reason for hiding this comment

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

more thoughts around this in this comment:
#2566 (comment)

Comment on lines +162 to +171
.PHONY: package
package: ## Run 'make install && make build', and package CloudFormation artifacts
aws cloudformation package \
--template-file ./apps/main-cf.yml \
--s3-bucket "$$ARTIFACT_BUCKET_NAME" \
--output-template-file packaged.yml; \


.PHONY: deploy
deploy: ## Deploy HyP3 with AWS CloudFormation
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these might deprecate .github/actions/deploy-hyp3. Though that may require writing a .env file as part of deployment...

Dockerfile Outdated Show resolved Hide resolved
-e AWS_DEFAULT_PROFILE \
-e AWS_DEFAULT_REGION \
-e AWS_DEFAULT_ACCOUNT \
${DEPLOY_ENV_IMAGE_NAME}:latest
Copy link
Contributor

@jhkennedy jhkennedy Jan 22, 2025

Choose a reason for hiding this comment

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

Suggested change
${DEPLOY_ENV_IMAGE_NAME}:latest
${DEPLOY_ENV_IMAGE_NAME}:latest

I'm not sure it we'll want to use the latest tag here or a specific version number...

Copy link
Author

Choose a reason for hiding this comment

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

I went with "latest" for the deployment environment image tag because I imagine them being short-lived (they are deleted with make clean). I'm happy to switch to a version number if there is a good reason. I could see wanting to record the version for CICD deployment, so maybe that is the reason to be more explicit.

Comment on lines +173 to +198
set -e && \
PARAM_OVERRIDES="" && \
declare -A PARAMS=( \
[AMI_ID]="AmiId" \
[AUTH_ALGORITHM]="AuthAlgorithm" \
[AUTH_PUBLIC_KEY]="AuthPublicKey" \
[CERT_ARN]="CertificateArn" \
[DEFAULT_APP_STATUS]="DefaultApplicationStatus" \
[DEFAULT_CREDITS_PER_USER]="DefaultCreditsPerUser" \
[DEFAULT_MAX_VCPUS]="DefaultMaxvCpus" \
[DOMAIN_NAME]="DomainName" \
[EXPANDED_MAX_VCPUS]="ExpandedMaxvCpus" \
[IMAGE_TAG]="ImageTag" \
[INSTANCE_TYPES]="InstanceTypes" \
[MONTHLY_BUDGET]="MonthlyBudget" \
[PRODUCT_LIFETIME_IN_DAYS]="ProductLifetimeInDays" \
[REQUIRED_SURPLUS]="RequiredSurplus" \
[SECRET_ARN]="SecretArn" \
[SUBNET_IDS]="SubnetIds" \
[SYSTEM_AVAILABLE]="SystemAvailable" \
[VPC_ID]="VpcId" \
); \
for var in "$${!PARAMS[@]}"; do \
value="$${!var}"; \
[ -n "$$value" ] && PARAM_OVERRIDES="$$PARAM_OVERRIDES $${PARAMS[$$var]}=$$value"; \
done; \
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

As this is currently organized, for the manual deployment workflow, build parameters are provided in hyp3.env, and not all are required. They are loaded as env vars when running the image. Any unused optional parameters end up as env vars with empty string values. This bit of code in the deploy target only adds parameter overrides for env vars with valid string values. The unused optional parameters are not included in the overrides and so defaults from the CF template will be used instead.

In the CICD deployment workflow, it looks like all the parameters are defined in deploy-*.yml, some of which are retrieved from GitHub secrets.

So, for CICD, by the time the deployment action is run, all the needed parameter overrides are known so you can hardcode which to include in the CF deploy command. When deploying from a local environment, I have to check which parameters were actually set in hyp3.env and only include those in the overrides. Otherwise, I would provide some of the parameter overrides values as empty strings, breaking the deployment.

@jtherrmann
Copy link
Contributor

My only concern with this PR is that it adds another method of deploying HyP3, when I'd prefer to see us standardize around one approach, preferably sticking to an infrastructure-as-code approach.

I'm not a fan of the deployment section in our README because it recommends deploying from the command-line, while most of our actual deployments consist of a GitHub environment + GitHub Actions deployment workflow. For example, our https://hyp3-opera-disp-sandbox.asf.alaska.edu deployment consists of a hyp3-opera-disp-sandbox GitHub environment and a deploy-opera-disp-sandbox.yml deployment workflow.

I typically adapt the steps from our New HyP3 Enterprise deployment wiki article whenever I set up a new deployment (regardless of whether it's an enterprise deployment), but I'd propose moving some version of those steps into the HyP3 README.

I know that @asjohnston-asf and maybe others have favored deploying from the command-line as shown in our README, which is understandable since it's a quick way of getting a deployment up and running, but I'd prefer that not be our standard recommended approach, since I'd rather all long-lived HyP3 deployments be managed via git / GitHub.

But that's just my opinion, I don't think we ever reached consensus on this within the HyP3 team.

@jhkennedy
Copy link
Contributor

My only concern with this PR is that it adds another method of deploying HyP3, when I'd prefer to see us standardize around one approach, preferably sticking to an infrastructure-as-code approach.

I agree with this, and we shouldn't do manual deployments. Maybe even more so now that multiple teams will be working on it.

In particular, HyP3 currently has an unwritten contract that all deployments are always up to date, so Vertex and the SDK only have to work with the latest version of HyP3. Keeping everything in IaC facilitates that and following the same processes ensures that every HyP3 deployment is listed here:
https://github.com/ASFHyP3/hyp3/settings/environments

If we start changing that contract, we're going to need to be a whole lot better about reporting version numbers and checking for compatibility.

I'm not a fan of the deployment section in our README ...
I typically adapt the steps from our New HyP3 Enterprise deployment wiki

Same on the README! We should document how we want HyP3 to be deployed there, so the wiki article should be moved largely to the README. Unfortunately, that wiki is private, so @Alex-Lewandowski couldn't have followed it even if he wanted to since he's not in this org yet.

I expect there are a ton of places where Tools shared knowledge and SoPs will need to be written down and exposed for multi-team development of HyP3.

@jhkennedy
Copy link
Contributor

jhkennedy commented Jan 24, 2025

All that said, I do like using docker to build out deployment packages. This potentially avoids some hard-to-resolve edge cases, especially around linking.

I do also like potentially dropping the custom github action so all of the "what you do with this repo" stuff is described in the makefile, but I don't really want to support .env files for the environment variables to discourage non IaC deployments.

@Alex-Lewandowski
Copy link
Author

Alex-Lewandowski commented Jan 24, 2025

I'd prefer to see us standardize around one approach, preferably sticking to an infrastructure-as-code approach.

I agree with this 100%. I much prefer standardizing around an IaC approach. I took a naive pass at deploying HyP3 based on the README and tried to add a little automation to that process, but there is no need to reinvent a squarer, less performant wheel.

My goal is to walk through the process of deploying HyP3 following the standardized process and pass on what I learn to the rest of the Services team. Can I have access to the HyP3 Enterprise deployment wiki?

It sounds like you already have a documented IaC approach with GitHub actions in place so most of this PR can go away. We could also convert it to a draft and use it for a README update based on the wiki. I'd love to step through it and help identify any areas that might be confusing to someone new to the HyP3 backend.

@Alex-Lewandowski Alex-Lewandowski marked this pull request as draft January 24, 2025 23:42
@jhkennedy
Copy link
Contributor

Can I have access to the HyP3 Enterprise deployment wiki?

@Alex-Lewandowski , yep, working on it!

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.

Verify Python==3.13 when running make build
3 participants