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

Add quiet flag to production deployment script #4263

Merged
merged 2 commits into from
Feb 13, 2025
Merged

Conversation

DanielRyanSmith
Copy link
Contributor

@DanielRyanSmith DanielRyanSmith commented Feb 13, 2025

This change adds a -q flag to the production deployment script, which adds the --quiet flag to certain gcloud commands.

The deployment script takes a long time to deploy (20+ minutes) and includes a number of confirmation prompts throughout the process. This change should make it easier to handle the deployment process with minimal supervision.

The new flag removes the confirmation steps for:

  • Deleting the oldest version of the 3 services (note that this command will always fail if the oldest versions have any traffic directed to them).
  • Deploying each service.

The new flag does NOT remove the confirmation steps for:

  • Redirecting traffic to the new service versions.
  • Removing an existing Docker instance.

Comment on lines 20 to 21
if confirm "Delete $1 service version $OLDEST_REV?"; then
gcloud app versions delete --service=$SERVICE $OLDEST_REV
else
echo "Skipping $1 service version $OLDEST_REV"
fi
gcloud app versions delete --service=$SERVICE $OLDEST_REV ${QUIET:+--quiet}
Copy link
Contributor Author

@DanielRyanSmith DanielRyanSmith Feb 13, 2025

Choose a reason for hiding this comment

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

A confirmation here is also removed because it is redundant with the confirmation that exists within the gcloud command, which made it required to confirm with 'y' twice

Copy link
Member

Choose a reason for hiding this comment

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

Nit: put the flags together before the version parameter?
I agree that 2 confirmations were too many.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@DanielRyanSmith DanielRyanSmith requested a review from past February 13, 2025 17:39
Copy link
Member

@past past left a comment

Choose a reason for hiding this comment

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

Just some nits and one question. Thanks!

Makefile Outdated
@@ -300,7 +300,7 @@ cleanup_staging_versions: gcloud_login

deploy_production: deployment_state
gcloud config set project wptdashboard
util/deploy.sh -r $(APP_PATH)
util/deploy.sh -r $(APP_PATH) ${QUIET:+-q}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I'd reorder and put the flags together, before the app parameter.

-b : skip GitHub issue creation
-f : Always deploy (even if checks have failed)"
-f : Always deploy (even if checks have failed)
-q : Disable all interactive prompts when running gcloud deploy commands"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: the quiet flag removes more things in deploy.sh, so I would rephrase it perhaps as "Disable all interactive prompts and debugging output".

While you are here, do you mind making capitalization more uniform by changing "skip" to "Skip" in the line above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Comment on lines 20 to 21
if confirm "Delete $1 service version $OLDEST_REV?"; then
gcloud app versions delete --service=$SERVICE $OLDEST_REV
else
echo "Skipping $1 service version $OLDEST_REV"
fi
gcloud app versions delete --service=$SERVICE $OLDEST_REV ${QUIET:+--quiet}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: put the flags together before the version parameter?
I agree that 2 confirmations were too many.

wptd_exec_it make deploy_production PROJECT=wptdashboard APP_PATH=api/query/cache/service
wptd_exec_it make deploy_production PROJECT=wptdashboard APP_PATH=webapp/web ${QUIET:+QUIET=true}
wptd_exec_it make deploy_production PROJECT=wptdashboard APP_PATH=results-processor ${QUIET:+QUIET=true}
wptd_exec_it make deploy_production PROJECT=wptdashboard APP_PATH=api/query/cache/service ${QUIET:+QUIET=true}
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need to similarly update the Makefile for this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deploy.sh actually already has a quiet flag implemented as well 🙂

@DanielRyanSmith DanielRyanSmith merged commit 38af5bc into main Feb 13, 2025
12 checks passed
@DanielRyanSmith DanielRyanSmith deleted the quiet-deploy branch February 13, 2025 22:58
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.

2 participants