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 --schedule flag to usage run command #10678

Merged
merged 1 commit into from
Jun 16, 2022
Merged

Conversation

andrew-farries
Copy link
Contributor

Description

⚠️ This is based on #10674. Review and merge that one first ⚠️

Add a --schedule flag to the usage run command to allow setting the schedule on which the reconciler runs.

The main motivation here is to make local development easier (don't have to wait 1 min after invoking the command to get a reconciliation run), but we'll probably want to set this in-cluster via a configmap later on.

Related Issue(s)

Part of #9036

How to test

DB_HOST=localhost DB_USERNAME=gitpod DB_PASSWORD=foo DB_PORT=3306 go run . run --api-key-file ./apikeys --schedule 5s

Reconciliation now runs after 5 seconds. Without the flag, reconciliation runs after 1m.

Release Notes

NONE

@andrew-farries andrew-farries requested a review from a team June 15, 2022 08:48
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 15, 2022
@andrew-farries andrew-farries force-pushed the af/add-schedule-flag-to-usage branch from 0f56f84 to 6cd3157 Compare June 16, 2022 14:52
@roboquat roboquat added size/XS and removed size/L labels Jun 16, 2022
@jankeromnes jankeromnes self-assigned this Jun 16, 2022
@@ -72,6 +73,7 @@ func run() *cobra.Command {
}

cmd.Flags().BoolVar(&verbose, "verbose", false, "Toggle verbose logging (debug level)")
cmd.Flags().DurationVar(&schedule, "schedule", 1*time.Minute, "The schedule on which the reconciler should run")
Copy link
Contributor

Choose a reason for hiding this comment

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

Side-note: Maybe also increase the default reconciliation loop period to 30min or 1h? (I'm a little worried about high-frequency logging noise)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rebased to change the default frequency to 1 hour.

@jankeromnes
Copy link
Contributor

jankeromnes commented Jun 16, 2022

@andrew-farries The usage component now crash-loops in preview environments -- that's not expected right?

{"@type":"type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent","error":"open /stripe-secret/apikeys: no such file or directory","level":"fatal","message":"Failed to initialize stripe client.","serviceContext":{"service":"usage","version":"commit-0f56f84b454fb6e5a3daecf0fe4e0a0a1c3551db"},"severity":"CRITICAL","time":"2022-06-16T15:02:38Z"}

I wonder if we should do an extra step so that the Stripe secret also gets mounted as an apikeys file in the component. 🤔

To allow overriding the default scheduler when testing the reconciler
locally.

Change default to 1 hour.
@andrew-farries andrew-farries force-pushed the af/add-schedule-flag-to-usage branch from 6cd3157 to 4a15532 Compare June 16, 2022 15:13
@andrew-farries
Copy link
Contributor Author

The stripe secret is already mounted into the usage component (#10631)

The problem is that this PR doesn't set with-payment=true, without which the stripe secret isn't mounted.

Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Looks good to me and works like a charm! ✨

Many thanks. 🙏

@roboquat roboquat merged commit 27de293 into main Jun 16, 2022
@roboquat roboquat deleted the af/add-schedule-flag-to-usage branch June 16, 2022 15:20
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/XS team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants