Skip to content

[P1] Flaky e2e: wstunnel spec asserts before the controller has processed the patch #45

Description

@jacaudi

internal/it's wstunnel spec fails intermittently with an empty container name:

Expected
    <string>: 
to equal
    <string>: wstunnel

internal/it/it_test.go:80, assertion at :99. It is a race in the test, not a product bug — the sidecar is rendered correctly, the assertion just runs before it exists.

This is currently the blocker on restoring an e2e stage to CI (#44): a suite that is intermittently red cannot be wired into ci, which is now the single required status check.

Where the race actually is

The spec patches the Wireguard CR, then waits on the Deployment:

_, err := KubectlPatch("wireguard/vpn", TestNamespace, "merge",
    `{"spec":{"tunnel":{"enabled":true,"port":8443}}}`)
...
waitForDeploymentTobeReady("vpn-dep", TestNamespace)

An earlier reading of this blamed waitForDeploymentTobeReady for waiting only on readiness. That is no longer accurate — it already guards on generation (internal/it/suite_test.go:58):

return deployment.Status.ObservedGeneration >= deployment.Generation &&
    deployment.Status.ReadyReplicas == desired &&
    deployment.Status.UpdatedReplicas == desired

The problem is subtler, and that guard cannot fix it. All three terms are evaluated against the Deployment's own generation — and at the moment of the wait, the controller may not have touched the Deployment yet. Its metadata.generation is still the pre-patch value, so ObservedGeneration >= Generation is trivially satisfied, and the pre-patch pod is already ready with UpdatedReplicas == desired. The helper returns immediately, having successfully waited for a rollout that has not been authored.

A generation guard can detect an in-flight update. It cannot detect an update that does not exist yet. The test's own comment — that this wait "is the real validation that the wstunnel container actually starts" — is wrong for the same reason.

The signal that does exist

The controller stamps the CR's generation onto its status conditions (internal/controller/wireguard_controller.go:156):

cond.ObservedGeneration = latest.GetGeneration()

So the correct barrier is on the Wireguard CR, not the Deployment: after patching, wait until a status condition reports observedGeneration >= metadata.generation of the CR. That is the point at which the controller has demonstrably processed this patch. The existing waitForDeploymentTobeReady call then does its intended job.

Suggested shape:

  1. Capture the CR's metadata.generation returned by the patch.
  2. Eventually until the CR's Ready/Degraded condition carries an observedGeneration at least that value.
  3. Then waitForDeploymentTobeReady, unchanged.
  4. Then assert on the container.

Waiting directly for the wstunnel container to appear would also work and is simpler, but it only fixes this one spec; step 2 is reusable by any spec that patches a CR and then asserts on what the controller produced — which is most of them.

Notes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions