-
Notifications
You must be signed in to change notification settings - Fork 10
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
Conversation
…packaging when not in image. Still have parsing issues with public key
…nize hyp3.example, provide hints, and default values
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.
Overall, I like this! I haven't done an extensive review, but did jot down some thoughts for discussion.
@@ -61,52 +63,55 @@ Review the parameters in [cloudformation.yml](apps/main-cf.yml) for deploy time | |||
|
|||
### Deploy with CloudFormation | |||
|
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 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.
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.
more thoughts around this in this comment:
#2566 (comment)
.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 |
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 think these might deprecate .github/actions/deploy-hyp3. Though that may require writing a .env
file as part of deployment...
-e AWS_DEFAULT_PROFILE \ | ||
-e AWS_DEFAULT_REGION \ | ||
-e AWS_DEFAULT_ACCOUNT \ | ||
${DEPLOY_ENV_IMAGE_NAME}:latest |
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.
${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...
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 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.
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; \ |
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.
This seems a little overly complex for me compared to:
https://github.com/ASFHyP3/hyp3/blob/develop/.github/actions/deploy-hyp3/action.yml#L112-L135
And looks like it doesn't handle these special cases:
https://github.com/ASFHyP3/hyp3/blob/develop/.github/actions/deploy-hyp3/action.yml#L93-L106
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.
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.
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 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. |
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: 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.
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. |
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 |
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 , yep, working on it! |
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.
README.md
hyp3.example
is a template used to create a deployment-specifichyp3.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 inhyp3.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 ofamazon/aws-lambda-python:3.13
, installs make and awscli, and copies in the hyp3 dirMakefile
updates:image
: Creates an image fromDockerfile
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 inhyp3.env
, or defaults to thedefault
profile. Assumes the user has AWS configured with a profile matching their provided AWS_DEFAULT_PROFILE.package
: Packages the CloudFormation templatedeploy
: Deploys HyP3 with CloudFormationhelp
: Lists all targets and their descriptions. Provides instructions for deploying with makeclean
: Added removal of the deployment image if it is present, Docker is installed, and Docker Engine is runningTo deploy with a deployment image:
hyp3.env
make image
make shell
make install
make build
make package
make deploy
To do it the old way:
make install
make build
aws cloudformation package ...
aws cloudformation deploy ...