diff --git a/go.mod b/go.mod index 54a8efe9ce61..73f08f348737 100644 --- a/go.mod +++ b/go.mod @@ -63,7 +63,7 @@ require ( github.com/onsi/ginkgo/v2 v2.25.1 github.com/onsi/gomega v1.38.2 github.com/opencontainers/go-digest v1.0.0 - github.com/openshift-eng/openshift-tests-extension v0.0.0-20260127124016-0fed2b824818 + github.com/openshift-eng/openshift-tests-extension v0.0.0-20260626105913-1f81f3df939a github.com/openshift-kni/commatrix v0.0.5-0.20251111204857-e5a931eff73f github.com/openshift/api v0.0.0-20260327065519-582dc3d316b7 github.com/openshift/apiserver-library-go v0.0.0-20260303173613-cd3676268d31 diff --git a/go.sum b/go.sum index eebe0aed491c..8288843e5f73 100644 --- a/go.sum +++ b/go.sum @@ -874,8 +874,8 @@ github.com/opencontainers/runtime-spec v1.2.1 h1:S4k4ryNgEpxW1dzyqffOmhI1BHYcjzU github.com/opencontainers/runtime-spec v1.2.1/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/opencontainers/selinux v1.13.0 h1:Zza88GWezyT7RLql12URvoxsbLfjFx988+LGaWfbL84= github.com/opencontainers/selinux v1.13.0/go.mod h1:XxWTed+A/s5NNq4GmYScVy+9jzXhGBVEOAyucdRUY8s= -github.com/openshift-eng/openshift-tests-extension v0.0.0-20260127124016-0fed2b824818 h1:jJLE/aCAqDf8U4wc3bE1IEKgIxbb0ICjCNVFA49x/8s= -github.com/openshift-eng/openshift-tests-extension v0.0.0-20260127124016-0fed2b824818/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M= +github.com/openshift-eng/openshift-tests-extension v0.0.0-20260626105913-1f81f3df939a h1:9kpy75Nbn/TZV8N7mLh1euQNrX5DFASwjTWzKzIPUnI= +github.com/openshift-eng/openshift-tests-extension v0.0.0-20260626105913-1f81f3df939a/go.mod h1:pHOS9c6BjZv91OkkHyIHAOWnYhxwcxWQkyYGEvPyUCE= github.com/openshift-kni/commatrix v0.0.5-0.20251111204857-e5a931eff73f h1:E72Zoc+JImPehBrXkgaCbIDbSFuItvyX6RCaZ0FQE5k= github.com/openshift-kni/commatrix v0.0.5-0.20251111204857-e5a931eff73f/go.mod h1:cDVdp0eda7EHE6tLuSeo4IqPWdAX/KJK+ogBirIGtsI= github.com/openshift/api v0.0.0-20260327065519-582dc3d316b7 h1:7AmoMSqTryaZu65nij6EACe8+DmlMlmR1giaUx5S5sQ= diff --git a/pkg/test/extensions/binary.go b/pkg/test/extensions/binary.go index 94eba6fe2ec5..f8ed3a8789f3 100644 --- a/pkg/test/extensions/binary.go +++ b/pkg/test/extensions/binary.go @@ -335,6 +335,36 @@ var extensionBinaries = []TestBinary{ }, } +// extractJSON finds the first JSON object or array in output, skipping any non-JSON log lines +// that precede it. This is necessary because some extension binaries output warnings or debug +// logging to stdout before the JSON payload. +func extractJSON(output []byte) ([]byte, error) { + jsonBegins := -1 + lines := bytes.Split(output, []byte("\n")) + for i, line := range lines { + trimmed := bytes.TrimSpace(line) + if len(trimmed) > 0 && (trimmed[0] == '{' || trimmed[0] == '[') { + jsonBegins = 0 + for j := 0; j < i; j++ { + jsonBegins += len(lines[j]) + 1 // +1 for the newline character + } + jsonBegins += len(line) - len(trimmed) // Add any leading whitespace + break + } + } + + if jsonBegins == -1 { + return nil, fmt.Errorf("no valid JSON found in output: %s", string(output)) + } + + var raw json.RawMessage + dec := json.NewDecoder(bytes.NewReader(output[jsonBegins:])) + if err := dec.Decode(&raw); err != nil { + return nil, fmt.Errorf("no valid JSON found in output: %w", err) + } + return raw, nil +} + // Info returns information about this particular extension. func (b *TestBinary) Info(ctx context.Context) (*Extension, error) { if b.info != nil { @@ -352,30 +382,13 @@ func (b *TestBinary) Info(ctx context.Context) (*Extension, error) { logrus.Errorf("Command output for %s: %s", binName, string(infoJson)) return nil, fmt.Errorf("failed running '%s info': %w\nOutput: %s", b.binaryPath, err, infoJson) } - // Some binaries may output logging that includes JSON-like data, so we need to find the first line that starts with '{' - jsonBegins := -1 - lines := bytes.Split(infoJson, []byte("\n")) - for i, line := range lines { - trimmed := bytes.TrimSpace(line) - if bytes.HasPrefix(trimmed, []byte("{")) { - // Calculate the byte offset of this line in the original output - jsonBegins = 0 - for j := 0; j < i; j++ { - jsonBegins += len(lines[j]) + 1 // +1 for the newline character - } - jsonBegins += len(line) - len(trimmed) // Add any leading whitespace - break - } - } - - jsonEnds := bytes.LastIndexByte(infoJson, '}') - if jsonBegins == -1 || jsonEnds == -1 || jsonBegins > jsonEnds { + jsonData, err := extractJSON(infoJson) + if err != nil { logrus.Errorf("No valid JSON found in output from %s info command", binName) logrus.Errorf("Raw output from %s: %s", binName, string(infoJson)) return nil, fmt.Errorf("no valid JSON found in output from '%s info' command", binName) } var info Extension - jsonData := infoJson[jsonBegins : jsonEnds+1] err = json.Unmarshal(jsonData, &info) if err != nil { logrus.Errorf("Failed to unmarshal JSON from %s: %v", binName, err) @@ -557,13 +570,27 @@ func (b *TestBinary) ListImages(ctx context.Context) (ImageSet, error) { command := exec.Command(b.binaryPath, "images") output, err := runWithTimeout(ctx, command, 10*time.Minute) if err != nil { - return nil, fmt.Errorf("failed running '%s list': %w\nOutput: %s", b.binaryPath, err, output) + return nil, fmt.Errorf("failed running '%s images': %w\nOutput: %s", b.binaryPath, err, output) + } + + jsonData, err := extractJSON(output) + if err != nil { + // Extensions that have no images may output "null" instead of an array + if bytes.Contains(output, []byte("null")) { + logrus.Infof("Extension %q reported null images, treating as empty", binName) + return ImageSet{}, nil + } + logrus.Errorf("No valid JSON found in output from %s images command", binName) + logrus.Errorf("Raw output from %s: %s", binName, string(output)) + return nil, fmt.Errorf("no valid JSON found in output from '%s images' command", binName) } var images []Image - err = json.Unmarshal(output, &images) + err = json.Unmarshal(jsonData, &images) if err != nil { - return nil, err + logrus.Errorf("Failed to unmarshal JSON from %s: %v", binName, err) + logrus.Errorf("JSON data from %s: %s", binName, string(jsonData)) + return nil, errors.Wrapf(err, "couldn't unmarshal extension images from %s: %s", binName, string(jsonData)) } result := make(ImageSet, len(images)) diff --git a/pkg/test/extensions/extract_json_test.go b/pkg/test/extensions/extract_json_test.go new file mode 100644 index 000000000000..e3a0ffae0b12 --- /dev/null +++ b/pkg/test/extensions/extract_json_test.go @@ -0,0 +1,70 @@ +package extensions + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExtractJSON(t *testing.T) { + tests := []struct { + name string + input string + want string + wantErr bool + }{ + { + name: "clean JSON object", + input: `{"key": "value"}`, + want: `{"key": "value"}`, + }, + { + name: "clean JSON array", + input: `[{"index": 1}]`, + want: `[{"index": 1}]`, + }, + { + name: "warning lines before JSON object", + input: "W0414 08:58:39.856273 46367 controller.go:47] some warning\nW0414 08:58:39.865859 46367 feature_gate.go:352] another warning\n{\"key\": \"value\"}\n", + want: `{"key": "value"}`, + }, + { + name: "warning lines before JSON array", + input: "W0414 08:58:39.856273 46367 controller.go:47] some warning\n[{\"index\": 1}]\n", + want: `[{"index": 1}]`, + }, + { + name: "log lines after JSON are ignored", + input: "{\"key\": \"value\"}\nW0414 trailing log with } brace\n", + want: `{"key": "value"}`, + }, + { + name: "no JSON in output", + input: "W0414 just warnings\nI0414 and info lines\n", + wantErr: true, + }, + { + name: "empty output", + input: "", + wantErr: true, + }, + { + name: "null is not a JSON object or array", + input: "W0414 warning\nnull\n", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := extractJSON([]byte(tt.input)) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.JSONEq(t, tt.want, string(got)) + }) + } +} diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runsuite.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runsuite.go index 998fd70409e7..a30b05e86703 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runsuite.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runsuite.go @@ -3,6 +3,7 @@ package cmdrun import ( "context" "encoding/json" + "errors" "fmt" "os" "os/signal" @@ -10,7 +11,6 @@ import ( "syscall" "time" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/openshift-eng/openshift-tests-extension/pkg/extension" @@ -74,7 +74,7 @@ func NewRunSuiteCommand(registry *extension.Registry) *cobra.Command { } suite, err := ext.GetSuite(args[0]) if err != nil { - return errors.Wrapf(err, "couldn't find suite: %s", args[0]) + return fmt.Errorf("couldn't find suite %q: %w", args[0], err) } compositeWriter := extensiontests.NewCompositeResultWriter() @@ -88,7 +88,7 @@ func NewRunSuiteCommand(registry *extension.Registry) *cobra.Command { if opts.junitPath != "" { junitWriter, err := extensiontests.NewJUnitResultWriter(opts.junitPath, suite.Name) if err != nil { - return errors.Wrap(err, "couldn't create junit writer") + return fmt.Errorf("couldn't create junit writer: %w", err) } compositeWriter.AddWriter(junitWriter) } @@ -96,7 +96,7 @@ func NewRunSuiteCommand(registry *extension.Registry) *cobra.Command { if opts.htmlPath != "" { htmlWriter, err := extensiontests.NewHTMLResultWriter(opts.htmlPath, suite.Name) if err != nil { - return errors.Wrap(err, "couldn't create html writer") + return fmt.Errorf("couldn't create html writer: %w", err) } compositeWriter.AddWriter(htmlWriter) } @@ -111,7 +111,15 @@ func NewRunSuiteCommand(registry *extension.Registry) *cobra.Command { specs, err := ext.GetSpecs().Filter(suite.Qualifiers) if err != nil { - return errors.Wrap(err, "couldn't filter specs") + return fmt.Errorf("couldn't filter specs: %w", err) + } + + if suite.TestTimeout != nil { + for _, spec := range specs { + if spec.Timeout == 0 { + spec.Timeout = *suite.TestTimeout + } + } } concurrency := opts.concurrencyFlags.MaxConcurency diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runtest.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runtest.go index c62021e7ecec..86e10e02e29d 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runtest.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runtest.go @@ -23,6 +23,7 @@ func NewRunTestCommand(registry *extension.Registry) *cobra.Command { concurrencyFlags *flags.ConcurrencyFlags nameFlags *flags.NamesFlags outputFlags *flags.OutputFlags + timeout time.Duration }{ componentFlags: flags.NewComponentFlags(), nameFlags: flags.NewNamesFlags(), @@ -37,6 +38,11 @@ func NewRunTestCommand(registry *extension.Registry) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx, cancelCause := context.WithCancelCause(context.Background()) defer cancelCause(errors.New("exiting")) + if opts.timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, opts.timeout) + defer cancel() + } abortCh := make(chan os.Signal, 2) go func() { @@ -104,6 +110,7 @@ func NewRunTestCommand(registry *extension.Registry) *cobra.Command { return err }, } + cmd.Flags().DurationVar(&opts.timeout, "timeout", 0, "Maximum duration for the test. When set, the test context will have a deadline, causing blocking operations like PollUntilDone to fail cleanly instead of hanging until the parent kills the process.") opts.componentFlags.BindFlags(cmd.Flags()) opts.nameFlags.BindFlags(cmd.Flags()) opts.outputFlags.BindFlags(cmd.Flags()) diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/scheduler.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/scheduler.go new file mode 100644 index 000000000000..ea4e08182012 --- /dev/null +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/scheduler.go @@ -0,0 +1,199 @@ +package extensiontests + +import ( + "context" + "sync" + + "github.com/openshift-eng/openshift-tests-extension/pkg/util/sets" +) + +const defaultConflictGroup = "default" + +// Scheduler defines the interface for test scheduling. +// It manages scheduling based on isolation requirements (conflicts, taints, tolerations). +// +// Callers must follow a get-once, complete-once protocol: every non-nil spec returned by +// GetNextTestToRun must eventually be passed to MarkTestComplete exactly once, including +// when test execution panics. +type Scheduler interface { + // GetNextTestToRun blocks until a test is available, then returns it. + // Returns nil when all tests have been distributed (queue is empty) or context is cancelled. + // When a test is returned, it is atomically removed from queue and marked as running. + // This method can be safely called from multiple goroutines concurrently. + GetNextTestToRun(ctx context.Context) *ExtensionTestSpec + + // MarkTestComplete marks a test as complete, cleaning up its conflicts and taints. + // This may unblock other tests that were waiting. + // This method can be safely called from multiple goroutines concurrently. + MarkTestComplete(spec *ExtensionTestSpec) +} + +// testScheduler manages test scheduling based on conflicts, taints, and tolerations. +// It maintains an ordered queue of tests and provides thread-safe scheduling operations. +type testScheduler struct { + mu sync.Mutex + cond *sync.Cond // condition variable to signal when tests complete + tests []*ExtensionTestSpec + runningConflicts map[string]sets.Set[string] // tracks which conflicts are running per group: group -> set of conflicts + activeTaints map[string]int // tracks how many tests are currently applying each taint +} + +// NewScheduler creates a test scheduler. It accepts tests in any order and schedules +// them based on isolation requirements (conflicts, taints, tolerations). +func NewScheduler(tests []*ExtensionTestSpec) Scheduler { + ts := &testScheduler{ + tests: append([]*ExtensionTestSpec(nil), tests...), + runningConflicts: make(map[string]sets.Set[string]), + activeTaints: make(map[string]int), + } + ts.cond = sync.NewCond(&ts.mu) + return ts +} + +// GetNextTestToRun blocks until a test is available to run, or returns nil +// if all tests have been distributed or the context is cancelled. +// It continuously scans the queue and waits for state changes when no tests are runnable. +// When a test is returned, it is atomically removed from queue and marked as running. +func (ts *testScheduler) GetNextTestToRun(ctx context.Context) *ExtensionTestSpec { + ts.mu.Lock() + defer ts.mu.Unlock() + + // Set up context cancellation to wake up any waiting goroutine + done := make(chan struct{}) + defer close(done) + go func() { + select { + case <-ctx.Done(): + ts.mu.Lock() + ts.cond.Broadcast() + ts.mu.Unlock() + case <-done: + // Normal exit, nothing to do + } + }() + + for { + // Check if context is cancelled + if ctx.Err() != nil { + return nil + } + + // Check if all tests have been distributed + if len(ts.tests) == 0 { + return nil + } + + // Scan from beginning to find first runnable test + for i, spec := range ts.tests { + conflictGroup := getConflictGroup(spec) + + // Ensure the conflict group set exists + if ts.runningConflicts[conflictGroup] == nil { + ts.runningConflicts[conflictGroup] = sets.New[string]() + } + + // Check if any of the test's conflicts are currently running within its group + hasConflict := ts.hasActiveConflict(spec, conflictGroup) + + // Check if test can tolerate all currently active taints + canTolerate := ts.canTolerateTaints(spec) + + if !hasConflict && canTolerate { + isolation := &spec.Resources.Isolation + + // Found a runnable test - ATOMICALLY: + // 1. Mark conflicts as running + for _, conflict := range isolation.Conflict { + ts.runningConflicts[conflictGroup].Insert(conflict) + } + + // 2. Activate taints + for _, taint := range isolation.Taint { + ts.activeTaints[taint]++ + } + + // 3. Remove test from queue + ts.tests = append(ts.tests[:i], ts.tests[i+1:]...) + + // 4. Return the test (now safe to run) + return spec + } + } + + // No runnable test found, but tests still exist in queue - wait for state change + ts.cond.Wait() + } +} + +func getConflictGroup(_ *ExtensionTestSpec) string { + return defaultConflictGroup +} + +// hasActiveConflict checks if the spec has any conflicts with currently running tests. +func (ts *testScheduler) hasActiveConflict(spec *ExtensionTestSpec, conflictGroup string) bool { + for _, conflict := range spec.Resources.Isolation.Conflict { + if ts.runningConflicts[conflictGroup].Has(conflict) { + return true + } + } + return false +} + +// canTolerateTaints checks if a spec can tolerate all currently active taints. +func (ts *testScheduler) canTolerateTaints(spec *ExtensionTestSpec) bool { + // If no taints are active, any test can run + if len(ts.activeTaints) == 0 { + return true + } + + // Build a set of tolerations for efficient lookup + tolerations := sets.New(spec.Resources.Isolation.Toleration...) + + // Check if test tolerates all active taints + for taint, count := range ts.activeTaints { + // Skip taints with zero count (should be cleaned up but being defensive) + if count <= 0 { + continue + } + + if !tolerations.Has(taint) { + return false // Test cannot tolerate this active taint + } + } + return true +} + +// MarkTestComplete marks all conflicts and taints of a spec as no longer running/active +// and signals waiting workers that blocked tests may now be runnable. +// This should be called after a test completes execution. +func (ts *testScheduler) MarkTestComplete(spec *ExtensionTestSpec) { + ts.mu.Lock() + defer ts.mu.Unlock() + + if spec == nil { + ts.cond.Broadcast() + return + } + + isolation := &spec.Resources.Isolation + conflictGroup := getConflictGroup(spec) + + // Clean up conflicts within this group + if groupConflicts, exists := ts.runningConflicts[conflictGroup]; exists { + for _, conflict := range isolation.Conflict { + groupConflicts.Delete(conflict) + } + } + + // Clean up taints with reference counting + for _, taint := range isolation.Taint { + ts.activeTaints[taint]-- + if ts.activeTaints[taint] <= 0 { + delete(ts.activeTaints, taint) + } + } + + // Signal waiting workers that the state has changed + // Some blocked tests might now be runnable + ts.cond.Broadcast() +} diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go index e87809c8a2a1..56cdc0fded6a 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go @@ -196,8 +196,10 @@ func (specs ExtensionTestSpecs) Names() []string { // are written to the given ResultWriter after each spec has completed execution. BeforeEach, // BeforeAll, AfterEach, AfterAll hooks are executed when specified. "Each" hooks must be thread // safe. Returns an error if any test spec failed, indicating the quantity of failures. +// +// Tests are scheduled using isolation-aware scheduling that respects conflicts, taints, and +// tolerations defined in each spec's Resources.Isolation field. func (specs ExtensionTestSpecs) Run(ctx context.Context, w ResultWriter, maxConcurrent int) ([]*ExtensionTestResult, error) { - queue := make(chan *ExtensionTestSpec) terminalFailures := atomic.Int64{} nonTerminalFailures := atomic.Int64{} @@ -208,20 +210,15 @@ func (specs ExtensionTestSpecs) Run(ctx context.Context, w ResultWriter, maxConc } } - // Feed the queue - go func() { - specs.Walk(func(spec *ExtensionTestSpec) { - queue <- spec - }) - close(queue) - }() - // if we have only a single spec to run, we do that differently than running multiple. // multiple specs can run in parallel and do so by exec-ing back into the binary with `run-test` with a single test to execute. // This means that to avoid infinite recursion, when requesting a single test to run // we need to run it in process. runSingleSpec := len(specs) == 1 + // Create scheduler with isolation-aware scheduling + scheduler := NewScheduler(specs) + // Start consumers var wg sync.WaitGroup resultChan := make(chan *ExtensionTestResult, len(specs)) @@ -229,29 +226,39 @@ func (specs ExtensionTestSpecs) Run(ctx context.Context, w ResultWriter, maxConc wg.Add(1) go func() { defer wg.Done() - for spec := range queue { - for _, beforeEachTask := range spec.beforeEach { - beforeEachTask.Run(*spec) + for { + // Get next runnable test from scheduler (blocks until available or done) + spec := scheduler.GetNextTestToRun(ctx) + if spec == nil { + return // No more tests or context cancelled } - res := runSpec(ctx, spec, runSingleSpec) - if res.Result == ResultFailed { - if res.Lifecycle.IsTerminal() { - terminalFailures.Add(1) - } else { - nonTerminalFailures.Add(1) + func() { + defer scheduler.MarkTestComplete(spec) + + for _, beforeEachTask := range spec.beforeEach { + beforeEachTask.Run(*spec) } - } - for _, afterEachTask := range spec.afterEach { - afterEachTask.Run(res) - } + res := runSpec(ctx, spec, runSingleSpec) + if res.Result == ResultFailed { + if res.Lifecycle.IsTerminal() { + terminalFailures.Add(1) + } else { + nonTerminalFailures.Add(1) + } + } + + for _, afterEachTask := range spec.afterEach { + afterEachTask.Run(res) + } - // We can't assume the runner will set the name of a test; it may not know it. Even if - // it does, we may want to modify it (e.g. k8s-tests for annotations currently). - res.Name = spec.Name - w.Write(res) - resultChan <- res + // We can't assume the runner will set the name of a test; it may not know it. Even if + // it does, we may want to modify it (e.g. k8s-tests for annotations currently). + res.Name = spec.Name + w.Write(res) + resultChan <- res + }() } }() } diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go index cd23be81ff08..f3edf41a6e6d 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go @@ -2,6 +2,7 @@ package extensiontests import ( "context" + "time" "github.com/openshift-eng/openshift-tests-extension/pkg/dbtime" "github.com/openshift-eng/openshift-tests-extension/pkg/util/sets" @@ -59,6 +60,10 @@ type ExtensionTestSpec struct { // to the `ote-binary run-test "test name"` commmand and interpretting the result. RunParallel func(ctx context.Context) *ExtensionTestResult `json:"-"` + // Timeout is the maximum duration for this test. If set, it overrides the default 90-minute + // timeout used by SpawnProcessToRunTest. This is typically populated from Suite.TestTimeout. + Timeout time.Duration `json:"-"` + // Hook functions afterAll []*OneTimeTask beforeAll []*OneTimeTask diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/output.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/output.go index 24f49f6387b1..af62bbf13bac 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/output.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/output.go @@ -2,10 +2,11 @@ package flags import ( "encoding/json" + "errors" + "fmt" "reflect" "strings" - "github.com/pkg/errors" "github.com/spf13/pflag" ) @@ -90,6 +91,6 @@ func (o *OutputFlags) Marshal(v interface{}) ([]byte, error) { } return nil, errors.New("names format requires an array of structs") default: - return nil, errors.Errorf("invalid output format: %s", o.Output) + return nil, fmt.Errorf("invalid output format: %s", o.Output) } } diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/parallel.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/parallel.go index 890cebb094e7..b7f95838a815 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/parallel.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/parallel.go @@ -25,7 +25,7 @@ func SpawnProcessToRunTest(ctx context.Context, testName string, timeout time.Du stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} - command := exec.CommandContext(longerCtx, os.Args[0], "run-test", "--output=json", testName) + command := exec.CommandContext(longerCtx, os.Args[0], "run-test", "--output=json", fmt.Sprintf("--timeout=%s", timeout), testName) command.Stdout = stdout command.Stderr = stderr @@ -37,30 +37,22 @@ func SpawnProcessToRunTest(ctx context.Context, testName string, timeout time.Du } go func() { + // interrupt after timeout, or exit early if the process finishes first select { - // interrupt tests after timeout, and abort if they don't complete quick enough case <-time.After(timeout): - if command.Process != nil { - // we're not going to do anything with the err - _ = command.Process.Signal(syscall.SIGINT) - } - // if the process appears to be hung a significant amount of time after the timeout - // send an ABRT so we get a stack dump - select { - case <-time.After(time.Minute): - if command.Process != nil { - // we're not going to do anything with the err - _ = command.Process.Signal(syscall.SIGABRT) - } - case <-timeoutCtx.Done(): - if command.Process != nil { - _ = command.Process.Signal(syscall.SIGABRT) - } - } case <-timeoutCtx.Done(): - if command.Process != nil { - _ = command.Process.Signal(syscall.SIGINT) - } + } + if command.Process != nil { + _ = command.Process.Signal(syscall.SIGINT) + } + // Canceled means the process exited and the context was cancelled — no need to escalate + if timeoutCtx.Err() == context.Canceled { + return + } + // if the process is hung, send SIGABRT after a grace period for a stack dump + <-time.After(time.Minute) + if command.Process != nil { + _ = command.Process.Signal(syscall.SIGABRT) } }() @@ -74,7 +66,7 @@ func SpawnProcessToRunTest(ctx context.Context, testName string, timeout time.Du } fmt.Fprintf(stderr, "Command Error: %v\n", cmdErr) - fmt.Fprintf(stderr, "Deserializaion Error: %v\n", parseErr) + fmt.Fprintf(stderr, "Deserialization Error: %v\n", parseErr) return newTestResult(testName, result, start, time.Now(), stdout, stderr) } @@ -83,28 +75,61 @@ func newTestResultFromOutput(stdout *bytes.Buffer) (*extensiontests.ExtensionTes return nil, errors.New("no output from command") } + jsonData, err := extractJSON(stdout.Bytes()) + if err != nil { + return nil, err + } + // when the command runs correctly, we get json or json slice output retArray := []extensiontests.ExtensionTestResult{} - if arrayItemErr := json.Unmarshal(stdout.Bytes(), &retArray); arrayItemErr == nil { + if arrayItemErr := json.Unmarshal(jsonData, &retArray); arrayItemErr == nil { if len(retArray) != 1 { - return nil, errors.New("expected 1 result, got %v results") + return nil, fmt.Errorf("expected 1 result, got %d results", len(retArray)) } return &retArray[0], nil } // when the command runs correctly, we get json output ret := &extensiontests.ExtensionTestResult{} - if singleItemErr := json.Unmarshal(stdout.Bytes(), ret); singleItemErr != nil { + if singleItemErr := json.Unmarshal(jsonData, ret); singleItemErr != nil { return nil, singleItemErr } return ret, nil } +// extractJSON finds the first JSON object or array in output, skipping any non-JSON +// lines that precede it (e.g. klog lines, Ginkgo reporter output). It also ignores +// trailing non-JSON content after the JSON payload. This is necessary because extension +// binaries may emit log output to stdout before or after the JSON result, which would +// otherwise cause deserialization failures. +func extractJSON(output []byte) ([]byte, error) { + lines := bytes.Split(output, []byte("\n")) + for i, line := range lines { + trimmed := bytes.TrimSpace(line) + if len(trimmed) > 0 && (trimmed[0] == '{' || trimmed[0] == '[') { + // Calculate byte offset to the start of the JSON content + offset := 0 + for j := 0; j < i; j++ { + offset += len(lines[j]) + 1 // +1 for the newline + } + + var raw json.RawMessage + dec := json.NewDecoder(bytes.NewReader(output[offset:])) + if err := dec.Decode(&raw); err != nil { + continue // not valid JSON, try next candidate line + } + return raw, nil + } + } + + return nil, fmt.Errorf("no JSON object or array found in output (%d bytes)", len(output)) +} + func newTestResult(name string, result extensiontests.Result, start, end time.Time, stdout, stderr *bytes.Buffer) *extensiontests.ExtensionTestResult { duration := end.Sub(start) dbStart := dbtime.DBTime(start) - dbEnd := dbtime.DBTime(start) + dbEnd := dbtime.DBTime(end) ret := &extensiontests.ExtensionTestResult{ Name: name, Lifecycle: "", // lifecycle is completed one level above this. diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go index e970d46ad417..90c5c2bd170d 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go @@ -11,7 +11,6 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/onsi/ginkgo/v2/types" "github.com/onsi/gomega" - "github.com/pkg/errors" "github.com/openshift-eng/openshift-tests-extension/pkg/util/sets" @@ -21,7 +20,7 @@ import ( func configureGinkgo() (*types.SuiteConfig, *types.ReporterConfig, error) { if !ginkgo.GetSuite().InPhaseBuildTree() { if err := ginkgo.GetSuite().BuildTree(); err != nil { - return nil, nil, errors.Wrapf(err, "couldn't build ginkgo tree") + return nil, nil, fmt.Errorf("couldn't build ginkgo tree: %w", err) } } @@ -57,7 +56,7 @@ func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite(selectFns ...ext.SelectFunc cwd, err := os.Getwd() if err != nil { - return nil, errors.Wrap(err, "couldn't get current working directory") + return nil, fmt.Errorf("couldn't get current working directory: %w", err) } ginkgo.GetSuite().WalkTests(func(name string, spec types.TestSpec) { @@ -81,22 +80,17 @@ func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite(selectFns ...ext.SelectFunc Name: spec.Text(), } - var summary types.SpecReport ginkgo.GetSuite().RunSpec(spec, ginkgo.Labels{}, "", cwd, ginkgo.GetFailer(), ginkgo.GetWriter(), *suiteConfig, *reporterConfig) - for _, report := range ginkgo.GetSuite().GetReport().SpecReports { - if report.NumAttempts > 0 { - summary = report - } - } + summary := findSpecReport(ginkgo.GetSuite().GetReport().SpecReports) result.Output = summary.CapturedGinkgoWriterOutput result.Error = summary.CapturedStdOutErr - switch { - case summary.State == types.SpecStatePassed: + switch summary.State { + case types.SpecStatePassed: result.Result = ext.ResultPassed - case summary.State == types.SpecStateSkipped, summary.State == types.SpecStatePending: + case types.SpecStateSkipped, types.SpecStatePending: result.Result = ext.ResultSkipped if len(summary.Failure.Message) > 0 { result.Output = fmt.Sprintf( @@ -115,7 +109,7 @@ func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite(selectFns ...ext.SelectFunc summary.Failure.ForwardedPanic, ) } - case summary.State == types.SpecStateFailed, summary.State == types.SpecStatePanicked, summary.State == types.SpecStateInterrupted, summary.State == types.SpecStateAborted: + case types.SpecStateFailed, types.SpecStatePanicked, types.SpecStateInterrupted, types.SpecStateAborted: result.Result = ext.ResultFailed var errors []string if len(summary.Failure.ForwardedPanic) > 0 { @@ -126,7 +120,7 @@ func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite(selectFns ...ext.SelectFunc } errors = append(errors, fmt.Sprintf("fail [%s:%d]: %s", lastFilenameSegment(summary.Failure.Location.FileName), summary.Failure.Location.LineNumber, summary.Failure.Message)) result.Error = strings.Join(errors, "\n") - case summary.State == types.SpecStateTimedout: + case types.SpecStateTimedout: result.Result = ext.ResultFailed var errors []string for _, additionalFailure := range summary.AdditionalFailures { @@ -137,16 +131,23 @@ func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite(selectFns ...ext.SelectFunc } errors = append(errors, fmt.Sprintf("fail [%s:%d]: %s", lastFilenameSegment(summary.Failure.Location.FileName), summary.Failure.Location.LineNumber, summary.Failure.Message)) result.Error = strings.Join(errors, "\n") + case types.SpecStateInvalid: + result.Result = ext.ResultFailed + result.Error = fmt.Sprintf("test produced no spec report; this is a bug in the test framework: %#v", summary) default: - panic(fmt.Sprintf("test produced unknown outcome: %#v", summary)) + result.Result = ext.ResultFailed + result.Error = fmt.Sprintf("test produced unknown outcome: %#v", summary) } return result }, - RunParallel: func(ctx context.Context) *ext.ExtensionTestResult { - // TODO pass through timeout and determine Lifecycle - return SpawnProcessToRunTest(ctx, name, 90*time.Minute) - }, + } + testCase.RunParallel = func(ctx context.Context) *ext.ExtensionTestResult { + timeout := 90 * time.Minute + if testCase.Timeout > 0 { + timeout = testCase.Timeout + } + return SpawnProcessToRunTest(ctx, name, timeout) } specs = append(specs, testCase) }) @@ -210,6 +211,25 @@ func lastFilenameSegment(filename string) string { return filename } +// findSpecReport selects the best matching spec report from the list of reports +// produced by RunSpec. It first looks for a report that was actually attempted +// (NumAttempts > 0), which covers passed/failed/panicked specs. If none is found +// (as happens with Pending or Skipped specs where Ginkgo never enters the +// execution loop), it falls back to the last report in the list. +func findSpecReport(reports types.SpecReports) types.SpecReport { + var summary types.SpecReport + for _, report := range reports { + if report.NumAttempts > 0 { + summary = report + } + } + // Pending/Skipped specs have NumAttempts==0; fall back to the last report + if summary.State == types.SpecStateInvalid && len(reports) > 0 { + summary = reports[len(reports)-1] + } + return summary +} + func collectAdditionalFailures(errors *[]string, suffix string, failure types.Failure) { if failure.IsZero() { return diff --git a/vendor/modules.txt b/vendor/modules.txt index 11d40fa3afdc..60f18fabb36f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1549,7 +1549,7 @@ github.com/opencontainers/runtime-spec/specs-go github.com/opencontainers/selinux/go-selinux github.com/opencontainers/selinux/go-selinux/label github.com/opencontainers/selinux/pkg/pwalkdir -# github.com/openshift-eng/openshift-tests-extension v0.0.0-20260127124016-0fed2b824818 +# github.com/openshift-eng/openshift-tests-extension v0.0.0-20260626105913-1f81f3df939a ## explicit; go 1.23.0 github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdinfo github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdlist