fix(status): withhold cluster readiness when CNI pods are unhealthy by canonicalayush · Pull Request #93 · canonical/k8sd

canonicalayush · 2026-09-18T12:39:45Z

Problem

getClusterStatus withholds Ready=true whenever the CNI workload pods, the Cilium agent and the Cilium operator, are missing or not in a Running plus Ready=True state. k8s status --wait-ready reports cluster status: ready while Cilium pods are crash-looping or have not been created yet.

Changes

pkg/k8sd/api/cluster.go

Add a CNI readiness gate. When config.Network.GetEnabled(), call features.StatusChecks.CheckNetwork and withhold readiness on error, mirroring the existing DNS gate.
Move the readiness logic out of the handler into Endpoints.clusterIsReady, covering HasReadyNodes plus the dns and network gates.
Use config.DNS.GetEnabled() so the dns check reads the same way as the network one.

pkg/k8sd/api/cluster_test.go, new file

TestClusterIsReadyNetworkGate covers three cases: network enabled with Cilium ready, network enabled with Cilium missing, and network disabled with Cilium missing. The last one confirms the disabled path short-circuits rather than passing by accident.

pkg/k8sd/features/cilium/status_test.go

Add a ciliumAgentRunningNotReady case to TestCheckNetwork, where the agent pod is present and Running but Ready=False.

Integration coverage lives in canonical/k8s-snap#2841.

Fixes canonical/k8s-snap#1789

Checklist

Code changes follow repository conventions
Unit tests cover the readiness states the gate depends on
No breaking wire changes to k8s-snap-api

A node reports Ready as soon as kubelet finds a CNI conflist on disk, which occurs before cilium-agent is serving. Consequently, reports on clusters where Cilium is broken or crash-looping. Extend StatusInterface with CheckClusterReady to verify that CNI workload pods (cilium-operator and cilium agent) are Running and Ready when network is enabled. Gate getClusterStatus readiness on this check and log reason when readiness is withheld. Fixes canonical/k8s-snap#1789 Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

canonicalayush · 2026-09-21T07:21:13Z

Unit Tests

TestGetClusterStatusNetworkGate - all three cases through the registered getClusterStatus handler:

networkEnabledCiliumReady : Ready=true,
networkEnabledCiliumMissing : Ready=false
networkDisabledCiliumMissing : Ready=true

TestCheckNetwork — all four subtests including the added ciliumAgentRunningNotReady (agent pod Running with Ready=False, the crash-loop signature from Cluster status is show as ready even when Cilium pods are crash-looping k8s-snap#1789).

Integration coverage lives in canonical/k8s-snap#2841.

Refine the CNI readiness check introduced for canonical/k8s-snap#1789: - Call features.StatusChecks.CheckNetwork directly in getClusterStatus when config.Network.GetEnabled() is true, removing the intermediate CheckClusterReady wrapper method and interface method. - Switch log level to log.Error(err, "network pods are not ready") to match the adjacent DNS check log idiom. - Ensure the network check evaluates independently of prior ready flag state. - Add TestGetClusterStatusNetworkGate in pkg/k8sd/api/cluster_test.go testing cluster status response for ready/missing CNI pods and disabled network. - Add ciliumAgentRunningNotReady subtest in pkg/k8sd/features/cilium/status_test.go. Fixes canonical/k8s-snap#1789 Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

Replaces the smoke-test kube-system assertion, which asserted more than the daemon gate guarantees, with a scoped guard plus a dedicated test that breaks the Cilium operator, the Cilium agent, and the agent's readiness probe and asserts `k8s status` withholds readiness while the node is still Ready. Ref: #1789 Depends-On: canonical/k8sd#93 Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

REVERT BEFORE MERGE, once canonical/k8sd#93 lands on k8sd main. Ref: canonical/k8sd#93 Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

Replaces the smoke-test kube-system assertion, which asserted more than the daemon gate guarantees, with a scoped guard plus a dedicated test that breaks the Cilium operator, the Cilium agent, and the agent's readiness probe and asserts `k8s status` withholds readiness while the node is still Ready. Ref: #1789 Depends-On: canonical/k8sd#93 Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

REVERT BEFORE MERGE, once canonical/k8sd#93 lands on k8sd main. Ref: canonical/k8sd#93 Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

louiseschmidtgen

Thanks for the great work on this! You're already showing great understanding of our cilium feature, its wiring and the associated tests thoroughly test the change.
A few comments before we go for the merge:

Extract the readiness logic from getClusterStatus into Endpoints.clusterIsReady and test it directly as an internal test, which drops the testenv and registered handler machinery. Returns (bool, error) so a HasReadyNodes failure still yields an InternalError. Also drops the unused HelmClient from the cilium status test. Ref: canonical/k8s-snap#1789 Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

golangci-lint's godot rule requires declaration comments to end in a period. Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

louiseschmidtgen

This looks good, nice work on cleaning up those tests.
One little nit then I'll review your k8s-snap changes once ready and we can merge this PR :)

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified, and all readiness assessments approve the changes.

Review effort: Lite
Findings: None

What changed in this PR

Adds CNI readiness checks so cluster status remains unready until enabled Cilium components are running and ready.

Changes:

Centralizes node, DNS, and network readiness checks.
Preserves readiness errors from node checks.
Adds regression tests for missing and unready Cilium pods.

File	Description
`pkg/k8sd/features/cilium/status_test.go`	Tests a running but unready Cilium agent.
`pkg/k8sd/api/cluster.go`	Implements combined cluster readiness gates.
`pkg/k8sd/api/cluster_test.go`	Tests enabled and disabled network readiness scenarios.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…usterStatus Be consistent within getClusterStatus by using the log-wrapped ctx everywhere instead of mixing r.Context() calls. This ensures all database operations include the 'endpoint' logging field. Note: r.Context() is preserved inside the Transaction lambda on line 52 where ctx is shadowed by the lambda parameter.

louiseschmidtgen

The implementation and the test LGTM. Leaving the final approval of this behaviour change to @canonical-berkayoz.
@canonicalayush please add a note in k8s status and in our alternative CNI tutorial on this change. That can be in this PR or in a separate one- I leave that up to you.

…d readiness in case network feature is enabled.

canonicalayush · 2026-09-30T17:35:07Z

Hi @louiseschmidtgen, I have added a small note on k8s status here. Please let me know if this looks good, or if it should be more verbose. I will open a new PR for the alternative CNI tutorial and link it to this PR.

Copilot

Copilot review overview

🟢 Approval recommended

The remaining requested error-path coverage is a minor nit and does not block approval.

Review effort: Lite
Findings: None

bschimke95

Thanks @canonicalayush
The code looks good and implements the agreed behavior but please trim your PR descriptions and comments down for this AI slop to something a human can/will read. No human reviewer cares about how many unittests your agent ran. :)

canonicalayush mentioned this pull request Sep 21, 2026

test(status): reproduce #1789 CNI readiness gate in integration tests canonical/k8s-snap#2841

Draft

2 of 3 tasks

canonicalayush marked this pull request as ready for review September 21, 2026 10:22

louiseschmidtgen reviewed Sep 23, 2026

View reviewed changes

Comment thread pkg/k8sd/api/cluster.go Outdated

Comment thread pkg/k8sd/api/cluster.go Outdated

Comment thread pkg/k8sd/api/cluster_test.go Outdated

Comment thread pkg/k8sd/features/cilium/status_test.go Outdated

canonicalayush added 2 commits September 24, 2026 11:08

style(status): end clusterIsReady doc comment with a period

e01a61e

golangci-lint's godot rule requires declaration comments to end in a period. Signed-off-by: Ayush Patra <ayush.patra@canonical.com>

louiseschmidtgen reviewed Sep 25, 2026

View reviewed changes

louiseschmidtgen requested a lite review from Copilot September 25, 2026 10:20

Copilot started reviewing on behalf of louiseschmidtgen September 25, 2026 10:20 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

louiseschmidtgen reviewed Sep 25, 2026

View reviewed changes

Comment thread pkg/k8sd/api/cluster.go

canonicalayush force-pushed the fix/cilium-status-readiness branch from 797fbff to f6bce00 Compare September 25, 2026 11:22

louiseschmidtgen reviewed Sep 29, 2026

View reviewed changes

feat(cli) : added note for k8s status command to wait for CNI workloa…

95fcafa

…d readiness in case network feature is enabled.

canonicalayush requested a lite review from Copilot September 30, 2026 17:35

Copilot started reviewing on behalf of canonicalayush September 30, 2026 17:36 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

bschimke95 requested changes Oct 2, 2026

View reviewed changes

bschimke95 approved these changes Oct 2, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(status): withhold cluster readiness when CNI pods are unhealthy - #93

fix(status): withhold cluster readiness when CNI pods are unhealthy#93
canonicalayush wants to merge 6 commits into
mainfrom
fix/cilium-status-readiness

canonicalayush commented Sep 18, 2026 •

edited

Loading

Uh oh!

canonicalayush commented Sep 21, 2026 •

edited

Loading

Uh oh!

louiseschmidtgen left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

louiseschmidtgen left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

louiseschmidtgen left a comment

Uh oh!

canonicalayush commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

bschimke95 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

canonicalayush commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Checklist

Uh oh!

canonicalayush commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests

Uh oh!

louiseschmidtgen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

louiseschmidtgen left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

louiseschmidtgen left a comment

Choose a reason for hiding this comment

Uh oh!

canonicalayush commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

bschimke95 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

canonicalayush commented Sep 18, 2026 •

edited

Loading

canonicalayush commented Sep 21, 2026 •

edited

Loading