Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions tests/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"path/filepath"
"strings"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -22,6 +23,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/envtest/komega"
)

var (
timeout = 5 * time.Second
)

// LoadTestSuiteSpecs recursively walks the given paths looking for any file with the suffix `.testsuite.yaml`.
// It then loads these files in SuiteSpec structs ready for the generator to generate the test cases.
func LoadTestSuiteSpecs(paths ...string) ([]SuiteSpec, error) {
Expand Down Expand Up @@ -137,7 +142,7 @@ func GenerateTestSuite(suiteSpec SuiteSpec) {
// Remove the CRD and wait for it to be removed from the API.
// If we don't wait then subsequent tests may fail.
Expect(envtest.UninstallCRDs(cfg, crdOptions)).ToNot(HaveOccurred())
Eventually(komega.Get(crd)).Should(Not(Succeed()))
Eventually(komega.Get(crd), timeout).Should(Not(Succeed()))
})

generateOnCreateTable(suiteSpec.Tests.OnCreate)
Expand Down Expand Up @@ -253,7 +258,7 @@ func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec, crdFileName string)
// Use an eventually here, so that we retry until the sential correctly applies.
Eventually(func() error {
return k8sClient.Create(ctx, initialObj)
}).Should(Succeed(), "initial object should create successfully")
}, timeout).Should(Succeed(), "initial object should create successfully")
Comment on lines 258 to +261
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Minor typo in comment: "sential" → "sentinel".

The retry logic is correct—retrying Create with GenerateName until the CRD schema update propagates is safe.

📝 Proposed fix
-		// Use an eventually here, so that we retry until the sential correctly applies.
+		// Use an eventually here, so that we retry until the sentinel correctly applies.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Use an eventually here, so that we retry until the sential correctly applies.
Eventually(func() error {
return k8sClient.Create(ctx, initialObj)
}).Should(Succeed(), "initial object should create successfully")
}, timeout).Should(Succeed(), "initial object should create successfully")
// Use an eventually here, so that we retry until the sentinel correctly applies.
Eventually(func() error {
return k8sClient.Create(ctx, initialObj)
}, timeout).Should(Succeed(), "initial object should create successfully")
🤖 Prompt for AI Agents
In `@tests/generator.go` around lines 258 - 261, Fix the typo in the comment above
the Eventually block: change "sential" to "sentinel" in the comment that
explains the retry logic around Eventually(func() error { return
k8sClient.Create(ctx, initialObj) }, timeout).Should(Succeed(), ...), leaving
the code (Eventually, k8sClient.Create, initialObj, GenerateName behavior)
unchanged.


if initialStatus != nil {
Expect(unstructured.SetNestedField(initialObj.Object, initialStatus, "status")).To(Succeed(), "should be able to restore initial status")
Expand Down Expand Up @@ -281,7 +286,7 @@ func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec, crdFileName string)
updatedObj.Object["sentinel"] = initialObj.GetUID() + "+restored"

return k8sClient.Update(ctx, updatedObj)
}).Should(Succeed(), "Sentinel should be persisted")
}, timeout).Should(Succeed(), "Sentinel should be persisted")

// Drop the sentinel field now we know the rest of the CRD schema is up to date.
originalCRD.Spec = originalCRDSpec
Expand Down
2 changes: 1 addition & 1 deletion tests/hack/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ REPO_ROOT=$(dirname "${BASH_SOURCE}")/..
OPENSHIFT_CI=${OPENSHIFT_CI:-""}
ARTIFACT_DIR=${ARTIFACT_DIR:-""}
GINKGO=${GINKGO:-"go run ${REPO_ROOT}/vendor/github.com/onsi/ginkgo/v2/ginkgo"}
GINKGO_ARGS=${GINKGO_ARGS:-"-r -v --randomize-all --randomize-suites --keep-going --timeout=30m"}
GINKGO_ARGS=${GINKGO_ARGS:-"-r -v --randomize-all --randomize-suites --keep-going --timeout=60m -p"}

Choose a reason for hiding this comment

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

Action required

1. Parallel envtest explosion 🐞 Bug ⛯ Reliability

• The added -p flag enables Ginkgo parallel mode with an auto-detected process count, which scales
  with CPU cores.
• In parallel mode, each process executes BeforeSuite, and this suite’s BeforeSuite starts a
  full envtest.Environment (kube-apiserver + etcd), so -p can start many control planes
  simultaneously.
• This can overwhelm CI resources (CPU/memory/ports) and make tests flaky or slower, defeating the
  intent of enabling parallelism by default.
Agent Prompt
### Issue description
`tests/hack/test.sh` now always runs `ginkgo` with `-p`, which makes Ginkgo auto-select a parallel process count based on CPU cores. This suite starts an `envtest.Environment` in `BeforeSuite`, so parallel mode can start multiple kube-apiserver/etcd control planes concurrently, causing CI resource exhaustion/flakes.

### Issue Context
Ginkgo v2 auto-computes `ComputedProcs()` from CPU count when `Parallel` is enabled. In parallel mode, each process runs `BeforeSuite`, and this repo’s `BeforeSuite` starts envtest.

### Fix Focus Areas
- tests/hack/test.sh[10-12]

### Suggested approach
- Remove `-p` from the default `GINKGO_ARGS`, OR
- Replace `-p` with a bounded `--procs=${GINKGO_PROCS:-1}` (and optionally allow users/CI to set `GINKGO_PROCS` to a small safe number like 2), OR
- Enable `-p` only in specific environments where resources are known to be sufficient (and still cap `--procs`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

GINKGO_EXTRA_ARGS=${GINKGO_EXTRA_ARGS:-""}

# Ensure that some home var is set and that it's not the root.
Expand Down