Skip to content

test: verify upgrade drain honors grace period and aborts on failure - #3227

Merged
talos-bot merged 1 commit into
siderolabs:mainfrom
oguzkilcan:test/upgrade-drain-grace-period
Aug 10, 2026
Merged

test: verify upgrade drain honors grace period and aborts on failure#3227
talos-bot merged 1 commit into
siderolabs:mainfrom
oguzkilcan:test/upgrade-drain-grace-period

Conversation

@oguzkilcan

Copy link
Copy Markdown
Member

Before Talos 1.13 the drain happened inside machined as part of the upgrade sequence. It evicted every pod with a hardcoded 60 second grace period, and it logged eviction failures instead of returning them, so the node rebooted whether or not the workload had actually finished shutting down.

Omni now runs the drain itself for cluster on 1.13 and up, passing each pod's own terminationGracePeriodSeconds through and refusing to reboot when the drain does not complete.

Nothing covered that end to end. The new suite parks a two-replica workload behind a PodDisruptionBudget, has it ignore SIGTERM for six minutes, then upgrades the cluster and rebuilds the drain timeline by sampling pod, node and ClusterMachineConfigStatus state. It checks that the eviction carried the pod's full grace period rather than a fixed one, that the drain failure surfaced instead of being swallowed, and that the node's boot ID only changed once the pod was gone.

Copilot AI review requested due to automatic review settings August 5, 2026 13:05
@oguzkilcan oguzkilcan added the integration/e2e-drain Triggers all e2e graceful drain tests for Omni label Aug 5, 2026
@github-project-automation github-project-automation Bot moved this to To Do in Planning Aug 5, 2026
@talos-bot talos-bot moved this from To Do to In Review in Planning Aug 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an end-to-end integration suite that validates Omni-managed upgrade draining on Talos ≥ 1.13: eviction uses each pod’s terminationGracePeriodSeconds, drain failures surface (instead of being swallowed), and nodes do not reboot until workloads are fully gone. This complements the existing integration suite coverage by testing the full upgrade + drain timeline with real Kubernetes workload behavior.

Changes:

  • Registers a new integration sub-suite DrainTerminationGracePeriod in the main TestIntegration suite list.
  • Introduces a new integration test (internal/integration/drain_test.go) that deploys a slow-terminating workload with a PDB, runs an upgrade, samples cluster/Omni state, and asserts drain/grace-period/reboot ordering.
  • Adds a dedicated CI job + cron workflow for running this new slow e2e suite and wires it into Slack notifications via regenerated Kres outputs.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/integration/integration_test.go Adds the new DrainTerminationGracePeriod suite to TestIntegration.
internal/integration/drain_test.go New integration test that deploys a slow termination workload and asserts drain honors grace period and blocks reboot until completion.
.kres.yaml Adds a new Kres-defined e2e job configuration for the drain grace period suite.
.github/workflows/slack-notify.yaml Regenerated to include the new cron workflow in notifications.
.github/workflows/slack-notify-ci-failure.yaml Regenerated to include the new cron workflow in CI failure notifications.
.github/workflows/e2e-drain-grace-period-cron.yaml New generated cron workflow to run the drain grace period suite on a schedule.
.github/workflows/ci.yaml Adds a label-triggered CI job to run the drain grace period e2e suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +524 to +527
// The whole point of draining before rebooting: the machine only goes down once the pod is gone.
assert.True(t, node.rebootedAt.After(pod.goneAt),
"node %q rebooted at %s, before the pod finished terminating at %s",
pod.nodeName, node.rebootedAt, pod.goneAt)
@oguzkilcan
oguzkilcan force-pushed the test/upgrade-drain-grace-period branch from 3c4f4c5 to 73b8b15 Compare August 6, 2026 19:27
Before Talos 1.13 the drain happened inside machined as part of the upgrade sequence. It evicted every pod with a hardcoded 60 second grace period, and it logged eviction failures instead of returning them, so the node rebooted whether or not the workload had actually finished shutting down.

Omni now runs the drain itself for cluster on 1.13 and up, passing each pod's own terminationGracePeriodSeconds through and refusing to reboot when the drain does not complete.

Nothing covered that end to end. The new suite parks a two-replica workload behind a PodDisruptionBudget, has it ignore SIGTERM for six minutes, then upgrades the cluster and rebuilds the drain timeline by sampling pod, node and ClusterMachineConfigStatus state. It checks that the eviction carried the pod's full grace period rather than a fixed one, that the drain failure surfaced instead of being swallowed, and that the node's boot ID only changed once the pod was gone.

Signed-off-by: Oguz Kilcan <oguz.kilcan@siderolabs.com>
@oguzkilcan

Copy link
Copy Markdown
Member Author

/m

@talos-bot
talos-bot merged commit 38299fa into siderolabs:main Aug 10, 2026
39 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Done in Planning Aug 10, 2026
@oguzkilcan
oguzkilcan deleted the test/upgrade-drain-grace-period branch August 10, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration/e2e-drain Triggers all e2e graceful drain tests for Omni

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants