Skip to content

Add one-shot mode for sealed execution#164

Open
alexellis wants to merge 1 commit intomasterfrom
one_shot
Open

Add one-shot mode for sealed execution#164
alexellis wants to merge 1 commit intomasterfrom
one_shot

Conversation

@alexellis
Copy link
Copy Markdown
Member

Description

Add one-shot mode for sealed execution

Motivation and Context

Sealed execution allows only one request to execute before the process exits, causing a restart, for a fresh process.

With SlicerVM, this means a hardware per request barrier.

How Has This Been Tested?

Tested e2e with a function from https://github.com/openfaas/async-concurrency-sample

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@reviewfn

This comment has been minimized.

Signed-off-by: Alex Ellis <alexellis2@gmail.com>
Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@reviewfn
Copy link
Copy Markdown

reviewfn bot commented Mar 10, 2026

AI Pull Request Overview

Summary

  • Adds one-shot mode feature allowing only one genuine invoke request per process for sealed execution
  • Implements graceful shutdown after first request, rejecting subsequent requests with 503
  • Excludes health and readiness endpoints from one-shot logic
  • Adds configuration via one_shot environment variable with boolean and numeric parsing
  • Includes comprehensive unit tests for the new functionality
  • Updates README with feature description and environment variable documentation
  • Performs dependency updates in go.mod and go.sum

Approval rating (1-10)

8

Summary per file

Summary per file
File path Summary
README.md Updated description and added one_shot env var docs
config/config.go Added OneShot bool field and env var parsing
config/config_test.go Added tests for one_shot parsing with true/1/0 values
go.mod Updated dependencies
go.sum Updated checksums
pkg/watchdog.go Core implementation of one-shot handler and shutdown logic
pkg/watchdog_test.go New unit tests for one-shot handler behavior
vendor/modules.txt Updated vendor dependencies

Overall Assessment

The implementation adds a valuable security feature for sealed execution environments, ensuring only one request is processed per process lifecycle. The code is well-structured with proper thread safety using atomics and sync primitives. Tests provide good coverage for the new functionality. The feature is opt-in via environment variable, minimizing risk of unintended behavior. However, the README description change appears unrelated to the PR's core purpose and may warrant clarification.

Detailed Review

Detailed Review

Potential Issues and Risks

  • README.md description change: The modification from "Reverse proxy for HTTP microservices and STDIO" to "Reverse proxy/middleware for functions using STDIO/HTTP" appears unrelated to the one-shot feature. This could be confusing and may not accurately reflect the project's current scope. Consider reverting or justifying this change separately.
  • Thread safety in makeOneShotHandler: While atomic.CompareAndSwapInt32 is used correctly for the served flag, the handler could potentially race if multiple requests arrive simultaneously at the exact moment of the swap. However, this is mitigated by the atomic operation and the 503 response for subsequent calls.
  • Drain timing: Setting drainDelay=0 in listenUntilShutdown for one-shot mode means immediate shutdown after the request completes. This assumes the single request will finish quickly, but if the request handler blocks or takes time, the process may not exit as expected. Consider adding a small grace period or ensuring the request completion is properly awaited.
  • Error handling in markUnhealthy: The function now ignores os.ErrNotExist when removing the lock file, which is appropriate. However, other removal errors are still logged but not handled - this may be acceptable for a watchdog process.
  • Dependency updates: The go.mod and go.sum changes appear to be routine updates, but without commit history context, it's unclear if these are intentional or side effects. The changes remove gorilla/mux and add kr/text, kr/pretty dependencies. Ensure these are not breaking changes for existing users.

Code Quality and Consistency

  • Handler wrapping order: The one-shot handler is applied before JWT authentication, which is correct as it should filter requests before auth processing.
  • Endpoint exclusion: Properly excludes health (//health), ready (//ready), and configurable readyEndpoint from one-shot logic, preventing false drains.
  • HTTP status codes: Uses 503 Service Unavailable for rejected requests, which is appropriate and follows HTTP standards.
  • Configuration parsing: Supports both boolean (true/false) and numeric (1/0) values for one_shot, consistent with existing config patterns in the codebase.
  • Logging: Appropriate logging for drain scheduling and rejection messages.

Testing and Validation

  • Unit tests: The new pkg/watchdog_test.go provides good coverage for one-shot behavior, including drain triggering, request rejection, and endpoint exclusion.
  • Config tests: Added parsing tests for various one_shot values, ensuring robustness.
  • E2E testing: PR mentions testing with async-concurrency-sample, which validates real-world usage.

Security Considerations

  • Sealed execution: The feature enables true one-request-per-process execution, enhancing security for sensitive workloads.
  • Request filtering: Only allows one genuine invoke request while permitting health checks, maintaining observability.
  • No secrets exposure: Implementation doesn't introduce any secret handling or external communication risks.

Performance and Efficiency

  • Minimal overhead: Atomic check adds negligible performance cost, appropriate for a watchdog component.
  • Memory usage: Uses sync.Once and channels efficiently, no memory leaks apparent.
  • Shutdown speed: Immediate drain for one-shot mode optimizes for quick process cycling in sealed environments.

Migration and Compatibility

  • Non-breaking: Feature is opt-in, existing deployments unaffected.
  • Configuration: Uses standard environment variable pattern consistent with other config options.
  • API compatibility: No changes to existing endpoints or request handling for non-one-shot mode.

Recommendations

  • Clarify the README description change or revert if unrelated.
  • Consider adding metrics or logging for one-shot mode usage to aid observability.
  • Add documentation example showing how to enable one-shot mode for sealed execution use cases.
  • Consider allowing configuration of the rejection response body for customization.

AI agent details.

Agent processing time: 1m2.035s
Environment preparation time: 5.762s
Total time from webhook: 1m11.611s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant