-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Quit agent endpoint with config #14223
Conversation
cc @taoism4504 for documentation updates |
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.
Looking good! Had a few questions/suggestions.
Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com>
Thanks for the reviews. I think that's all comments addressed. Docs site deployment link is here if you'd like to browse: https://vault-aq18lm9e5-hashicorp.vercel.app/docs/agent#api |
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.
Working great for me in my testing!
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.
Looks great!
Reminder that if there's a release/1.10.x
by the time this gets merged, we'd need to also backport it to that branch.
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.
Text looks good.
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.
LGTM!
} | ||
|
||
// Get a randomly assigned port and then free it again before returning it. | ||
// There is still a race when trying to use it, but should work better |
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.
The risk of a port collision should be relatively low since most kernels incrementally assign ports from their dynamic range of many thousands of ports.
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.
TIL, thanks
Builds on #12789 and closes #11089
Support a quit API endpoint so that when Vault Agent can be programatically shut down. This is useful for instance when Agent is a sidecar in a Kubernetes job to stop the job from hanging forever. Currently the only workarounds are to use
shareProcessNamespace: true
for the container so that process kill signals can be sent, or to avoid the sidecar container entirely and just rely on an init container.