Skip to content

[BUG]: flaky operator unit test: leaked mirrored NodeWrights fail "should not return if there are no skyhooks" #677

Closed

Description

@rice-riley

Summary

make unit-tests in operator/ fails intermittently on

[FAIL] skyhook controller tests [It] should not return if there are no skyhooks

with

error validating nodewright [mirror-basic]: error package test-package's image 'alpine:3.21.0' contains an inline tag '3.21.0'. Do not embed a tag in the image; the package version supplies the tag

Seen on PR #673 (an agent-only change) in Operator CI run 35919699766, job unit-tests (k8s-1.37.0), Ginkgo random seed 1790197369. The same job passed on the two previous heads of that PR and on every recent main run.

Root cause

Three specs in operator/internal/controller/mirror_controller_test.go create a legacy Skyhook and register a DeferCleanup that deletes only the legacy object:

  • mirrors a new legacy Skyhook into a stamped NodeWright with equal spec (mirror-basic)
  • propagates legacy spec updates and advances the generation stamp (mirror-update)
  • does not rewrite the target in steady state (mirror-steady)

The mirror controller deliberately does not delete the mirrored NodeWright when the legacy object goes away (the spec does not delete the mirrored target when the legacy object is deleted asserts exactly that), so those three NodeWrights stay in the shared envtest apiserver for the rest of the suite.

should not return if there are no skyhooks in skyhook_controller_test.go:97 then calls operator.Reconcile and expects no error. Reconcile lists every NodeWright through dal.GetSkyhooks and validates each one; the leaked fixtures carry Image: "alpine:3.21.0", which fails the inline-tag check, so Reconcile returns the validation error and the spec fails.

It is order-dependent: Ginkgo randomizes top-level container order per seed, and the spec only fails when Mirror controller runs before skyhook controller tests. In the failing run the mirror specs finished at 21:03:47 and the failing spec ran at 21:03:50.

Proposed fix

Have the three mirror specs also clean up the NodeWright they cause to be created, matching what the later specs in the same file already do:

DeferCleanup(func() {
    _ = k8sClient.Delete(ctx, &nwv1.NodeWright{ObjectMeta: metav1.ObjectMeta{Name: name}})
})

Alternatively, or additionally, should not return if there are no skyhooks could assert its own precondition (list NodeWrights and expect none) so a leak from any other suite produces a message that names the leaked object rather than a validation error.

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

Metadata

Metadata

Assignees

Labels

component/testsEnd-to-end / chainsaw test suites (k8s-tests)

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions