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

HHMI hub #3136

Merged
merged 21 commits into from
Sep 18, 2023
Merged

HHMI hub #3136

merged 21 commits into from
Sep 18, 2023

Conversation

GeorgianaElena
Copy link
Member

For #3080

This currently only has a staging hub, deployed at https://staging.hhmi.2i2c.cloud/.
Need to add a prod hub for it

@github-actions
Copy link

github-actions bot commented Sep 14, 2023

Merging this PR will trigger the following deployment actions.

Support and Staging deployments

Cloud Provider Cluster Name Upgrade Support? Reason for Support Redeploy Upgrade Staging? Reason for Staging Redeploy
gcp hhmi Yes Following helm chart values files were modified: enc-support.secret.values.yaml, support.values.yaml Yes Following helm chart values files were modified: common.values.yaml, staging.values.yaml, enc-staging.secret.values.yaml

Production deployments

Cloud Provider Cluster Name Hub Name Reason for Redeploy
gcp hhmi prod Following helm chart values files were modified: common.values.yaml, prod.values.yaml, enc-prod.secret.values.yaml

Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

This LGTM and consider this approved if you provide oauth 15.1 functional config now that the oauth16 upgrade was reverted. Work towards oauth16 can be found in #3144

config/clusters/hhmi/common.values.yaml Outdated Show resolved Hide resolved
config/clusters/hhmi/common.values.yaml Outdated Show resolved Hide resolved
cpu:
guarantee: 0.5
# We're on n1-highmem-16 machines
limit: 14
Copy link
Member

Choose a reason for hiding this comment

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

This probably needs a G after

Copy link
Member Author

@GeorgianaElena GeorgianaElena Sep 18, 2023

Choose a reason for hiding this comment

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

@yuvipanda, I think it's ok because this is the cpu limit? I just double-checked and the memory guarantee and limit have the G.

@GeorgianaElena
Copy link
Member Author

Thanks @consideRatio and @yuvipanda for the review. I've now also added (and deployed) a prod hub too. Also updated the auth config for oauthenticator < 16

This LGTM and consider this approved if you provide oauth 15.1 functional config

Per Erik's comment above, I will merge this now.

@GeorgianaElena GeorgianaElena merged commit fecf167 into 2i2c-org:master Sep 18, 2023
32 checks passed
@GeorgianaElena GeorgianaElena deleted the hhmi-hub branch September 18, 2023 11:48
@github-actions
Copy link

🎉🎉🎉🎉

Monitor the deployment of the hubs here 👉 https://github.com/2i2c-org/infrastructure/actions/runs/6222063303

@@ -8,7 +8,7 @@
prefix = "{{ cluster_name }}"
project_id = "{{ project_id }}"

zone = "{{ cluster_region }}"
zone = "{{ cluster_region }}-b"
Copy link
Contributor

Choose a reason for hiding this comment

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

Quick question in a high-level post review (so I might be totally wrong 😉): do we always want to deploy to the -b zone? Should not get the zone from foo_cluster.tfvars file instead of "building it" from the region?
cc @GeorgianaElena

Copy link
Member Author

Choose a reason for hiding this comment

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

Should not get the zone from foo_cluster.tfvars file instead of "building it" from the region?

@damianavila, so this template is actually used by the deployer to generate a foo_cluster.tfvars file. This initial file generated by the deployer is more of a starting point with some "default" basics that are meant to be edited to fulfill all of the needs for the cluster.

do we always want to deploy to the -b zone?

Not necessarily, but it is my understanding that this is the go-to zone for us-west2 (which I believe is the default region if none is requested) if we want to be able to have GPU machines (which for a daskhub is probable) available and a vaster CPU types machines, including AMDs.

Copy link
Contributor

Choose a reason for hiding this comment

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

@damianavila, so this template is

I knew I was missing something (well-placed "so I might be totally wrong 😉" message).
Thanks for the clarification!!

Not necessarily,

Yep, it makes sense now that I actually realize what I am seeing 😉
Again, thanks for your patience and feedback.

@colliand
Copy link
Contributor

I believe this issue can be closed since the hub has been deployed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done 🎉
Development

Successfully merging this pull request may close these issues.

5 participants