Skip to content

postStart hook commands timeout #1440

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,16 @@ type WorkspaceConfig struct {
RuntimeClassName *string `json:"runtimeClassName,omitempty"`
// CleanupCronJobConfig defines configuration options for a cron job that automatically cleans up stale DevWorkspaces.
CleanupCronJob *CleanupCronJobConfig `json:"cleanupCronJob,omitempty"`
// PostStartTimeout defines the maximum duration the PostStart hook can run
// before it is automatically failed. This timeout is used for the postStart lifecycle hook
// that is used to run commands in the workspace container. The timeout is specified in seconds.
// If not specified, the timeout is disabled (0 seconds).
// +kubebuilder:validation:Minimum=0
// +kubebuilder:default:=0
// +kubebuilder:validation:Optional
// +kubebuilder:validation:Type=integer
// +kubebuilder:validation:Format=int32
PostStartTimeout *int32 `json:"postStartTimeout,omitempty"`
}

type WebhookConfig struct {
Expand Down
5 changes: 5 additions & 0 deletions apis/controller/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion controllers/workspace/devworkspace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,9 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
&workspace.Spec.Template,
workspace.Config.Workspace.ContainerSecurityContext,
workspace.Config.Workspace.ImagePullPolicy,
workspace.Config.Workspace.DefaultContainerResources)
workspace.Config.Workspace.DefaultContainerResources,
workspace.Config.Workspace.PostStartTimeout,
)
if err != nil {
return r.failWorkspace(workspace, fmt.Sprintf("Error processing devfile: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions deploy/deployment/kubernetes/combined.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions deploy/deployment/openshift/combined.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/config/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,10 @@ func mergeConfig(from, to *controller.OperatorConfiguration) {
to.Workspace.CleanupCronJob.Schedule = from.Workspace.CleanupCronJob.Schedule
}
}

if from.Workspace.PostStartTimeout != nil {
to.Workspace.PostStartTimeout = from.Workspace.PostStartTimeout
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/library/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import (
// rewritten as Volumes are added to PodAdditions, in order to support e.g. using one PVC to hold all volumes
//
// Note: Requires DevWorkspace to be flattened (i.e. the DevWorkspace contains no Parent or Components of type Plugin)
func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, securityContext *corev1.SecurityContext, pullPolicy string, defaultResources *corev1.ResourceRequirements) (*v1alpha1.PodAdditions, error) {
func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, securityContext *corev1.SecurityContext, pullPolicy string, defaultResources *corev1.ResourceRequirements, postStartTimeout *int32) (*v1alpha1.PodAdditions, error) {
if !flatten.DevWorkspaceIsFlattened(workspace, nil) {
return nil, fmt.Errorf("devfile is not flattened")
}
Expand Down Expand Up @@ -77,7 +77,7 @@ func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, securi
podAdditions.Containers = append(podAdditions.Containers, *k8sContainer)
}

if err := lifecycle.AddPostStartLifecycleHooks(workspace, podAdditions.Containers); err != nil {
if err := lifecycle.AddPostStartLifecycleHooks(workspace, podAdditions.Containers, postStartTimeout); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/library/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestGetKubeContainersFromDevfile(t *testing.T) {
t.Run(tt.Name, func(t *testing.T) {
// sanity check that file is read correctly.
assert.True(t, len(tt.Input.Components) > 0, "Input defines no components")
gotPodAdditions, err := GetKubeContainersFromDevfile(tt.Input, nil, testImagePullPolicy, defaultResources)
gotPodAdditions, err := GetKubeContainersFromDevfile(tt.Input, nil, testImagePullPolicy, defaultResources, nil)
if tt.Output.ErrRegexp != nil && assert.Error(t, err) {
assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match")
} else {
Expand Down
127 changes: 104 additions & 23 deletions pkg/library/lifecycle/poststart.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,20 @@ import (
corev1 "k8s.io/api/core/v1"
)

const redirectOutputFmt = `{
%s
} 1>/tmp/poststart-stdout.txt 2>/tmp/poststart-stderr.txt
const (
// `tee` both stdout and stderr to files and to the main output streams.
redirectOutputFmt = `{
# This script block ensures its exit code is preserved
# while its stdout and stderr are tee'd.
_script_to_run() {
%s # This will be replaced by scriptWithTimeout
}
_script_to_run
} 1> >(tee -a "/tmp/poststart-stdout.txt") 2> >(tee -a "/tmp/poststart-stderr.txt" >&2)
`
)

func AddPostStartLifecycleHooks(wksp *dw.DevWorkspaceTemplateSpec, containers []corev1.Container) error {
func AddPostStartLifecycleHooks(wksp *dw.DevWorkspaceTemplateSpec, containers []corev1.Container, postStartTimeout *int32) error {
if wksp.Events == nil || len(wksp.Events.PostStart) == 0 {
return nil
}
Expand Down Expand Up @@ -54,7 +62,7 @@ func AddPostStartLifecycleHooks(wksp *dw.DevWorkspaceTemplateSpec, containers []
return fmt.Errorf("failed to process postStart event %s: %w", commands[0].Id, err)
}

postStartHandler, err := processCommandsForPostStart(commands)
postStartHandler, err := processCommandsForPostStart(commands, postStartTimeout)
if err != nil {
return fmt.Errorf("failed to process postStart event %s: %w", commands[0].Id, err)
}
Expand All @@ -68,38 +76,111 @@ func AddPostStartLifecycleHooks(wksp *dw.DevWorkspaceTemplateSpec, containers []
return nil
}

// processCommandsForPostStart builds a lifecycle handler that runs the provided command(s)
// The command has the format
//
// exec:
//
// command:
// - "/bin/sh"
// - "-c"
// - |
// cd <workingDir>
// <commandline>
func processCommandsForPostStart(commands []dw.Command) (*corev1.LifecycleHandler, error) {
var dwCommands []string
// buildUserScript takes a list of DevWorkspace commands and constructs a single
// shell script string that executes them sequentially.
func buildUserScript(commands []dw.Command) (string, error) {
var commandScriptLines []string
for _, command := range commands {
execCmd := command.Exec
if execCmd == nil {
// Should be caught by earlier validation, but good to be safe
return "", fmt.Errorf("exec command is nil for command ID %s", command.Id)
}
if len(execCmd.Env) > 0 {
return nil, fmt.Errorf("env vars in postStart command %s are unsupported", command.Id)
return "", fmt.Errorf("env vars in postStart command %s are unsupported", command.Id)
}
var singleCommandParts []string
if execCmd.WorkingDir != "" {
dwCommands = append(dwCommands, fmt.Sprintf("cd %s", execCmd.WorkingDir))
// Safely quote the working directory path
safeWorkingDir := strings.ReplaceAll(execCmd.WorkingDir, "'", `'\''`)
singleCommandParts = append(singleCommandParts, fmt.Sprintf("cd '%s'", safeWorkingDir))
}
if execCmd.CommandLine != "" {
singleCommandParts = append(singleCommandParts, execCmd.CommandLine)
}
dwCommands = append(dwCommands, execCmd.CommandLine)
if len(singleCommandParts) > 0 {
commandScriptLines = append(commandScriptLines, strings.Join(singleCommandParts, " && "))
}
}
return strings.Join(commandScriptLines, "\n"), nil
}

// generateScriptWithTimeout wraps a given user script with timeout logic,
// environment variable exports, and specific exit code handling.
// The killAfterDurationSeconds is hardcoded to 5s within this generated script.
// It conditionally prefixes the user script with the timeout command if available.
func generateScriptWithTimeout(escapedUserScript string, timeoutSeconds int32) string {
return fmt.Sprintf(`
export POSTSTART_TIMEOUT_DURATION="%d"
export POSTSTART_KILL_AFTER_DURATION="5"

_TIMEOUT_COMMAND_PART=""
_WAS_TIMEOUT_USED="false" # Use strings "true" or "false" for shell boolean

if command -v timeout >/dev/null 2>&1; then
echo "[postStart hook] Executing commands with timeout: ${POSTSTART_TIMEOUT_DURATION} seconds, kill after: ${POSTSTART_KILL_AFTER_DURATION} seconds" >&2
_TIMEOUT_COMMAND_PART="timeout --preserve-status --kill-after=${POSTSTART_KILL_AFTER_DURATION} ${POSTSTART_TIMEOUT_DURATION}"
_WAS_TIMEOUT_USED="true"
else
echo "[postStart hook] WARNING: 'timeout' utility not found. Executing commands without timeout." >&2
fi

# Execute the user's script
${_TIMEOUT_COMMAND_PART} /bin/sh -c '%s'
exit_code=$?

# Check the exit code based on whether timeout was attempted
if [ "$_WAS_TIMEOUT_USED" = "true" ]; then
if [ $exit_code -eq 143 ]; then # 128 + 15 (SIGTERM)
echo "[postStart hook] Commands terminated by SIGTERM (likely timed out after ${POSTSTART_TIMEOUT_DURATION}s). Exit code 143." >&2
elif [ $exit_code -eq 137 ]; then # 128 + 9 (SIGKILL)
echo "[postStart hook] Commands forcefully killed by SIGKILL (likely after --kill-after ${POSTSTART_KILL_AFTER_DURATION}s expired). Exit code 137." >&2
elif [ $exit_code -ne 0 ]; then # Catches any other non-zero exit code
echo "[postStart hook] Commands failed with exit code $exit_code." >&2
else
echo "[postStart hook] Commands completed successfully within the time limit." >&2
fi
else
if [ $exit_code -ne 0 ]; then
echo "[postStart hook] Commands failed with exit code $exit_code (no timeout)." >&2
else
echo "[postStart hook] Commands completed successfully (no timeout)." >&2
fi
fi

exit $exit_code
`, timeoutSeconds, escapedUserScript)
}

// processCommandsForPostStart processes a list of DevWorkspace commands
// and generates a corev1.LifecycleHandler for the PostStart lifecycle hook.
func processCommandsForPostStart(commands []dw.Command, postStartTimeout *int32) (*corev1.LifecycleHandler, error) {
if postStartTimeout == nil {
// The 'timeout' command treats 0 as "no timeout", so it is disabled by default.
defaultTimeout := int32(0)
postStartTimeout = &defaultTimeout
}

originalUserScript, err := buildUserScript(commands)
if err != nil {
return nil, fmt.Errorf("failed to build aggregated user script: %w", err)
}

joinedCommands := strings.Join(dwCommands, "\n")
// The user script needs 'set -e' to ensure it exits on error.
// This script is then passed to `sh -c '...'`, so single quotes within it must be escaped.
scriptToExecute := "set -e\n" + originalUserScript
escapedUserScriptForTimeoutWrapper := strings.ReplaceAll(scriptToExecute, "'", `'\''`)

fullScriptWithTimeout := generateScriptWithTimeout(escapedUserScriptForTimeoutWrapper, *postStartTimeout)

finalScriptForHook := fmt.Sprintf(redirectOutputFmt, fullScriptWithTimeout)

handler := &corev1.LifecycleHandler{
Exec: &corev1.ExecAction{
Command: []string{
"/bin/sh",
"-c",
fmt.Sprintf(redirectOutputFmt, joinedCommands),
finalScriptForHook,
},
},
}
Expand Down
Loading
Loading