Skip to content

Add optional websocket keepalive pings for idle Exec sessions#2878

Draft
Copilot wants to merge 3 commits into
mainfrom
copilot/exec-timeout-fix
Draft

Add optional websocket keepalive pings for idle Exec sessions#2878
Copilot wants to merge 3 commits into
mainfrom
copilot/exec-timeout-fix

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

Exec websocket sessions can be dropped around the 5-minute mark in some proxy/network paths when no traffic is sent. This change adds an opt-in keepalive mechanism for Exec without changing default behavior.

  • API: opt-in exec keepalive

    • Added ExecOptions with pingIntervalMs?: number.
    • Extended Exec.exec(...) with an optional trailing options parameter.
  • Behavior: periodic websocket ping

    • When pingIntervalMs is a positive integer, Exec sends websocket ping frames on that interval while the socket is open.
    • Keepalive is ignored when the underlying socket does not expose ping().
  • Lifecycle: cleanup on termination

    • Keepalive timer is cleared on websocket close and error to avoid orphaned intervals.
  • Coverage: focused unit test

    • Added a targeted exec_test case validating that pings are emitted when configured and stop after close.
const exec = new Exec(kc);

await exec.exec(
  'ns',
  'pod',
  'container',
  ['sh', '-c', 'long-running-command'],
  stdout,
  stderr,
  stdin,
  false,
  undefined,
  { pingIntervalMs: 30_000 },
);

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 26, 2026
Copilot AI linked an issue May 26, 2026 that may be closed by this pull request
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 26, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Copilot
Once this PR has been reviewed and has the lgtm label, please ask for approval from brendandburns. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 26, 2026
Copilot AI changed the title [WIP] Implement optional ping-pong for exec timeout Add optional websocket keepalive pings for idle Exec sessions May 26, 2026
Copilot AI requested a review from brendandburns May 26, 2026 17:54
Copy link
Copy Markdown
Member

@mstruebing mstruebing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Will leave it in this state for now. to give others the chance to look at it.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exec timeout after 5 minutes

4 participants