feat: add snapshot post server hook#3933
Conversation
mfranczy
force-pushed
the
snapshots-handler
branch
from
bd472f1 to
acfab5c
Compare
Signed-off-by: Marcin Franczyk <marcin0franczyk@gmail.com>
mfranczy
force-pushed
the
snapshots-handler
branch
from
acfab5c to
3523740
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3523740390
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return | ||
| } | ||
| if err := client.Run(r.Context(), ctx.Config); err != nil { | ||
| fail(w, r, http.StatusInternalServerError, err) |
There was a problem hiding this comment.
This path converts client-side validation failures into 500 responses: client.Run/client.List validate user-provided snapshot options (for example, unsupported type or missing storage fields), but errors from that validation are currently returned as internal server errors. That makes malformed requests look like transient server faults and can trigger incorrect retry behavior in API clients. These validation errors should be mapped to a 4xx status (typically 400).
Useful? React with 👍 / 👎.
| fail(w, r, http.StatusBadRequest, err) | ||
| return | ||
| } | ||
| if err := client.Run(r.Context(), ctx.Config); err != nil { |
There was a problem hiding this comment.
Passing ctx.Config directly into client.Run introduces a concurrency hazard for the new HTTP endpoint: snapshot.Client.Run mutates the provided config in standalone mode (HostNamespace and standalone constants), and HTTP handlers are executed concurrently. Multiple snapshot POST requests can therefore race on shared process state and cause nondeterministic behavior. The handler should pass an isolated copy (or Run should stop mutating input config).
Useful? React with 👍 / 👎.
1 participant
What issue type does this pull request address? (keep at least one, remove the others)
/kind feature
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>if possible)resolves #
Please provide a short message that should be published in the vcluster release notes
Fixed an issue where vcluster ...
What else do we need to know?
Implemented a vCluster snapshot HTTP hook that exposes snapshot operations without requiring command exec.
The API now uses a single
/vcluster/snapshots*prefix:/vcluster/snaphots- probe/vcluster/snapshots- snapshot create/vcluster/snapshots/list- snapshot list/vcluster/snapshots/request- snapshot request create (async snapshot creation)/vcluster/snapshots/request/delete- snapshot delete (async snapshot deletion)/vcluster/snapshots/request/{name}- snapshot request getDirect snapshot routes operate on storage immediately, while request routes create/read k8s request resources that are reconciled asynchronously by the snapshot controller.
E2E Tests
Default Test Execution
The mandatory PR suite runs automatically. Only specify additional test suites below if needed.
Adding New Test Suites
When adding a new ginkgo test suite:
Additional test suites
Additional test suite(s) that will be executed before the mandatory PR suite: