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

Verify reloads & cache secrets #380

Merged
merged 2 commits into from
Oct 1, 2018
Merged

Verify reloads & cache secrets #380

merged 2 commits into from
Oct 1, 2018

Conversation

isaachawley
Copy link
Contributor

@isaachawley isaachawley commented Sep 27, 2018

When calling nginx -s reload block until we verify that the new config has been loaded.

  • Cache secrets to improve performance.
  • Adds a new config file to hold config version.
  • Adds a verify client that fetches the version.
  • Protects the Plus API from making changes on a worker process with old config.
  • Skips nginx -t, since it's done by nginx -s reload.

Reload performance is worse after this change since we block after reloading. However with the secrets cache fix overall performance is improved.

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

isaac added 2 commits September 19, 2018 12:44
Querying the api for secrets here significantly slows down the creation
of ingress objects. In most cases we will already have the secret in a
store.
- Write config version directly in nginx config
- Check config version by querying over socket
- Add config version check to API
- Atomic write (write and rename) for version config
@isaachawley isaachawley added bug An issue reporting a potential bug enhancement Pull requests for new features/feature enhancements performance labels Sep 27, 2018
@isaachawley isaachawley self-assigned this Sep 27, 2018
if err != nil {
glog.Fatalf("Failed to write to %v: %v", filename, err)
}
w.Close()

Choose a reason for hiding this comment

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

@isaachawley should you not defer right after os.Create 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.

Hmm. We don't normally do that. See writing secrets here. I'm open to changing it but I'd like to change them all at once.

@isaachawley isaachawley merged commit aa0f464 into master Oct 1, 2018
@isaachawley isaachawley deleted the verify-reloads branch October 1, 2018 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue reporting a potential bug enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants