[VC-35738] Stop the API server when the context is cancelled#604
Merged
wallrj merged 2 commits intoVC-35738/featurefrom Nov 5, 2024
Merged
[VC-35738] Stop the API server when the context is cancelled#604wallrj merged 2 commits intoVC-35738/featurefrom
wallrj merged 2 commits intoVC-35738/featurefrom
Conversation
wallrj
force-pushed
the
VC-35738/stop-the-server
branch
from
296359b to
fddc921
Compare
Member
|
How did you catch this bug? Was it by running the If not, we should add some form of test case somewhere. I'd suggest adding a comment to https://venafi.atlassian.net/browse/VC-35565 to mention that |
maelvls
reviewed
Nov 4, 2024
| server.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| }) | ||
| log.Info("Readyz endpoints enabled", "path", "/readyz") |
Member
There was a problem hiding this comment.
Nit, unimportant: weird that we start a readiness and liveness server when running --one-shot, but that's all right IMO
Member
Author
There was a problem hiding this comment.
I agree it's weird, perhaps in a future PR we can make it so that the API server is not started in one-shot mode.
maelvls
approved these changes
Nov 4, 2024
Member
maelvls
left a comment
maelvls
left a comment
There was a problem hiding this comment.
LGTM
It'd be great to have some fast test that uses a fake Venafi Cloud API to test that --one-shot immediately returns. It could look like the test cases in
jetstack-secure/pkg/agent/config_test.go
Line 615 in 1f00f09
Run func
wallrj
force-pushed
the
VC-35738/stop-the-server
branch
3 times, most recently
from
29a5520 to
c214e58
Compare
Member
Author
|
@maelvls I added a test. Before: $ go test ./pkg/agent/... --run TestRunOneShot -v
=== RUN TestRunOneShot
run_test.go:50: Running child process
run_test.go:68: STDOUT
I1105 11:47:58.084463 81129 logs.go:158] "Preflight agent version: development ()" source="agent"
I1105 11:47:58.084574 81129 logs.go:158] "Using the Jetstack Secure API Token auth mode since --api-token was specified." source="agent"
I1105 11:47:58.085278 81129 logs.go:158] "Reading data from local file: testdata/success/input.json" source="agent"
I1105 11:47:58.085357 81129 logs.go:158] "Data saved to local file: /dev/null" source="agent"
run_test.go:69: STDERR
run_test.go:70:
Error Trace: /home/richard/projects/venafi/venafi-kubernetes-agent/pkg/agent/run_test.go:70
Error: Received unexpected error:
signal: killed
Test: TestRunOneShot
Messages: context deadline exceeded
--- FAIL: TestRunOneShot (3.01s)
FAIL
FAIL github.com/jetstack/preflight/pkg/agent 3.033s
FAILAfter: $ go test ./pkg/agent/... --run TestRunOneShot -v -count 1
=== RUN TestRunOneShot
run_test.go:50: Running child process
run_test.go:68: STDOUT
I1105 11:48:56.732058 82228 logs.go:158] "Preflight agent version: development ()" source="agent"
I1105 11:48:56.732211 82228 logs.go:158] "Using the Jetstack Secure API Token auth mode since --api-token was specified." source="agent"
I1105 11:48:56.732225 82228 run.go:117] "Healthz endpoints enabled" logger="APIServer" addr=":8081" path="/healthz"
I1105 11:48:56.732235 82228 run.go:121] "Readyz endpoints enabled" logger="APIServer" addr=":8081" path="/readyz"
I1105 11:48:56.732305 82228 run.go:447] "Starting" logger="APIServer.ListenAndServe" addr=":8081"
I1105 11:48:56.733181 82228 logs.go:158] "Reading data from local file: testdata/success/input.json" source="agent"
I1105 11:48:56.733250 82228 logs.go:158] "Data saved to local file: /dev/null" source="agent"
I1105 11:48:56.733258 82228 run.go:461] "Shutting down" logger="APIServer.ListenAndServe" addr=":8081"
I1105 11:48:56.733332 82228 run.go:476] "Shutdown complete" logger="APIServer.ListenAndServe" addr=":8081"
PASS
run_test.go:69: STDERR
--- PASS: TestRunOneShot (0.03s)
PASS
ok github.com/jetstack/preflight/pkg/agent 0.052s |
Signed-off-by: Richard Wall <richard.wall@venafi.com>
Signed-off-by: Richard Wall <richard.wall@venafi.com>
wallrj
force-pushed
the
VC-35738/stop-the-server
branch
from
c214e58 to
46960df
Compare
maelvls
approved these changes
Nov 5, 2024
Member
maelvls
left a comment
maelvls
left a comment
There was a problem hiding this comment.
Well done with the test. Thanks. LGTM
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #601 I broke the
--one-shotfeature, because I added the HTTP server to the errgroup, but didn't provide a way to stop the HTTP server after the single gather loop.This PR introduces a simple function which starts the server and calls shutdown when the supplied context is cancelled.
Success:
Example of an error when the port is already occupied.