Skip to content

Commit 53cc8b6

Browse files
committed
feat(picod): support configurable shutdown timeout and cap execution request timeout
Signed-off-by: Abhinav Singh <abhinavsingh717073@gmail.com>
1 parent 02cb899 commit 53cc8b6

4 files changed

Lines changed: 62 additions & 6 deletions

File tree

cmd/picod/main.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"flag"
2222
"os/signal"
2323
"syscall"
24+
"time"
2425

2526
"k8s.io/klog/v2"
2627

@@ -30,14 +31,16 @@ import (
3031
func main() {
3132
port := flag.Int("port", 8080, "Port for the PicoD server to listen on")
3233
workspace := flag.String("workspace", "", "Root directory for file operations (default: current working directory)")
34+
shutdownTimeout := flag.Duration("shutdown-timeout", 90*time.Second, "Grace period for graceful shutdown")
3335

3436
// Initialize klog flags
3537
klog.InitFlags(nil)
3638
flag.Parse()
3739

3840
config := picod.Config{
39-
Port: *port,
40-
Workspace: *workspace,
41+
Port: *port,
42+
Workspace: *workspace,
43+
ShutdownTimeout: *shutdownTimeout,
4144
}
4245

4346
// Create a context that is canceled on SIGINT or SIGTERM.

pkg/picod/execute.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"time"
2828

2929
"github.com/gin-gonic/gin"
30+
"k8s.io/klog/v2"
3031
)
3132

3233
const (
@@ -84,6 +85,16 @@ func (s *Server) ExecuteHandler(c *gin.Context) {
8485
}
8586
}
8687

88+
// Cap timeout to shutdown grace period
89+
shutdownTimeout := s.config.ShutdownTimeout
90+
if shutdownTimeout <= 0 {
91+
shutdownTimeout = 90 * time.Second
92+
}
93+
if timeoutDuration > shutdownTimeout {
94+
klog.Infof("Requested timeout %v exceeds shutdown grace period; capping to %v", timeoutDuration, shutdownTimeout)
95+
timeoutDuration = shutdownTimeout
96+
}
97+
8798
// Create context with timeout
8899
ctx, cancel := context.WithTimeout(context.Background(), timeoutDuration)
89100
defer cancel()

pkg/picod/execute_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,40 @@ func TestExecuteHandler_TimeoutHandling(t *testing.T) {
395395
assert.Contains(t, resp.Stderr, "Command timed out")
396396
}
397397

398+
func TestExecuteHandler_TimeoutCapping(t *testing.T) {
399+
server, tmpDir := setupExecuteTestServer(t)
400+
defer os.RemoveAll(tmpDir)
401+
defer os.Unsetenv(PublicKeyEnvVar)
402+
403+
// Set server shutdown timeout very low
404+
server.config.ShutdownTimeout = 200 * time.Millisecond
405+
406+
// Command would take 2 seconds, but timeout is requested as 10s.
407+
// Since 10s exceeds ShutdownTimeout (200ms), it should be capped to 200ms
408+
// and therefore time out.
409+
req := ExecuteRequest{
410+
Command: []string{"sleep", "2"},
411+
Timeout: "10s",
412+
}
413+
body, _ := json.Marshal(req)
414+
415+
w := httptest.NewRecorder()
416+
c, _ := gin.CreateTestContext(w)
417+
c.Request, _ = http.NewRequest("POST", "/api/execute", bytes.NewBuffer(body))
418+
c.Request.Header.Set("Content-Type", "application/json")
419+
420+
server.ExecuteHandler(c)
421+
422+
assert.Equal(t, http.StatusOK, w.Code)
423+
424+
var resp ExecuteResponse
425+
err := json.Unmarshal(w.Body.Bytes(), &resp)
426+
require.NoError(t, err)
427+
// It should have timed out due to capping
428+
assert.Equal(t, TimeoutExitCode, resp.ExitCode)
429+
assert.Contains(t, resp.Stderr, "Command timed out")
430+
}
431+
398432
func TestExecuteHandler_EnvironmentVariables(t *testing.T) {
399433
server, tmpDir := setupExecuteTestServer(t)
400434
defer os.RemoveAll(tmpDir)

pkg/picod/server.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ import (
2929

3030
// Config defines server configuration
3131
type Config struct {
32-
Port int `json:"port"`
33-
Workspace string `json:"workspace"`
32+
Port int `json:"port"`
33+
Workspace string `json:"workspace"`
34+
ShutdownTimeout time.Duration `json:"shutdown_timeout"`
3435
}
3536

3637
// Server defines the PicoD HTTP server
@@ -44,6 +45,9 @@ type Server struct {
4445

4546
// NewServer creates a new PicoD server instance
4647
func NewServer(config Config) *Server {
48+
if config.ShutdownTimeout <= 0 {
49+
config.ShutdownTimeout = 90 * time.Second
50+
}
4751
s := &Server{
4852
config: config,
4953
startTime: time.Now(),
@@ -110,8 +114,12 @@ func (s *Server) Run(ctx context.Context) error {
110114
idleConnsClosed := make(chan struct{})
111115
go func() {
112116
<-ctx.Done()
113-
// Allow enough time for in-flight commands to finish (default timeout is 60s).
114-
shutdownCtx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
117+
// Allow enough time for in-flight commands to finish.
118+
shutdownTimeout := s.config.ShutdownTimeout
119+
if shutdownTimeout <= 0 {
120+
shutdownTimeout = 90 * time.Second
121+
}
122+
shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownTimeout)
115123
defer cancel()
116124
if err := server.Shutdown(shutdownCtx); err != nil {
117125
klog.Errorf("HTTP server shutdown error: %v", err)

0 commit comments

Comments
 (0)