Skip to content

Commit 0055a0b

Browse files
committed
Ensure OS environment variables are wired into list command as well
When getting the list of tests, we need to have origin's environment variables. Some users are setting TEST_PROVIDER when invoking openshift-tests, which for `run` this gets overwritten with the correct ones from the cluster environment[1], but this doesn't happen with `list`. An upcoming change is going to require initializing the test framework even to list tests, which means we'll need to have the TEST_PROVIDER correctly set. [1] https://github.com/openshift/origin/blob/main/pkg/test/ginkgo/test_runner.go#L380-L403
1 parent a1e274c commit 0055a0b

File tree

2 files changed

+29
-19
lines changed

2 files changed

+29
-19
lines changed

pkg/test/extensions/binary.go

+15-8
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,27 @@ func (b *TestBinary) Info(ctx context.Context) (*ExtensionInfo, error) {
9090
return b.info, nil
9191
}
9292

93-
// ListTests takes a list of EnvironmentFlags to pass to the command so it can determine for itself which tests are relevant.
94-
// returns which tests this binary advertises.
95-
func (b *TestBinary) ListTests(ctx context.Context, envFlags EnvironmentFlags) (ExtensionTestSpecs, error) {
93+
// ListTests returns a list of *relevant* tests provided by the extension. The word "environment" is overloaded here.
94+
// This function takes both environment variables and cluster environment flags. The environment variables pass
95+
// information from origin to kube, such as the TEST_PROVIDER configuration. This is a legacy way of passing data from
96+
// origin to an extension and should only be used by kube. Newer extensions should instead rely on the cluster environment
97+
// flags so it can determine for itself which tests are relevant and only return those.
98+
func (b *TestBinary) ListTests(ctx context.Context, osEnv []string, clusterEnvFlags EnvironmentFlags) (ExtensionTestSpecs, error) {
9699
var tests ExtensionTestSpecs
97100
start := time.Now()
98101
binName := filepath.Base(b.binaryPath)
99102

100103
binLogger := logrus.WithField("binary", binName)
101104
binLogger.Info("Listing tests")
102105
binLogger.Infof("OTE API version is: %s", b.info.APIVersion)
103-
envFlags = b.filterToApplicableEnvironmentFlags(envFlags)
106+
clusterEnvFlags = b.filterToApplicableEnvironmentFlags(clusterEnvFlags)
104107
command := exec.Command(b.binaryPath, "list", "-o", "jsonl")
105-
binLogger.Infof("adding the following applicable flags to the list command: %s", envFlags.String())
106-
command.Args = append(command.Args, envFlags.ArgStrings()...)
108+
if len(osEnv) > 0 {
109+
command.Env = osEnv
110+
}
111+
112+
binLogger.Infof("adding the following applicable flags to the list command: %s", clusterEnvFlags.String())
113+
command.Args = append(command.Args, clusterEnvFlags.ArgStrings()...)
107114
testList, err := runWithTimeout(ctx, command, 10*time.Minute)
108115
if err != nil {
109116
return nil, fmt.Errorf("failed running '%s list': %w\nOutput: %s", b.binaryPath, err, testList)
@@ -504,7 +511,7 @@ func (binaries TestBinaries) ListImages(ctx context.Context, parallelism int) ([
504511

505512
// ListTests extracts the tests from all TestBinaries using the specified parallelism,
506513
// and passes the provided EnvironmentFlags for proper filtering of results.
507-
func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int, envFlags EnvironmentFlags) (ExtensionTestSpecs, error) {
514+
func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int, osEnv []string, clusterEnvFlags EnvironmentFlags) (ExtensionTestSpecs, error) {
508515
var (
509516
allTests ExtensionTestSpecs
510517
mu sync.Mutex
@@ -538,7 +545,7 @@ func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int, env
538545
if !ok {
539546
return // Channel was closed
540547
}
541-
tests, err := binary.ListTests(ctx, envFlags)
548+
tests, err := binary.ListTests(ctx, osEnv, clusterEnvFlags)
542549
if err != nil {
543550
errCh <- err
544551
}

pkg/test/ginkgo/cmd_runsuite.go

+14-11
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ import (
1919

2020
"github.com/onsi/ginkgo/v2"
2121
configv1 "github.com/openshift/api/config/v1"
22+
"github.com/sirupsen/logrus"
23+
"github.com/spf13/pflag"
24+
"golang.org/x/mod/semver"
25+
"k8s.io/apimachinery/pkg/util/sets"
26+
"k8s.io/cli-runtime/pkg/genericclioptions"
27+
"k8s.io/client-go/discovery"
28+
"k8s.io/client-go/rest"
29+
e2e "k8s.io/kubernetes/test/e2e/framework"
30+
2231
"github.com/openshift/origin/pkg/clioptions/clusterdiscovery"
2332
"github.com/openshift/origin/pkg/clioptions/clusterinfo"
2433
"github.com/openshift/origin/pkg/defaultmonitortests"
@@ -28,14 +37,6 @@ import (
2837
"github.com/openshift/origin/pkg/riskanalysis"
2938
"github.com/openshift/origin/pkg/test/extensions"
3039
"github.com/openshift/origin/pkg/test/ginkgo/junitapi"
31-
"github.com/sirupsen/logrus"
32-
"github.com/spf13/pflag"
33-
"golang.org/x/mod/semver"
34-
"k8s.io/apimachinery/pkg/util/sets"
35-
"k8s.io/cli-runtime/pkg/genericclioptions"
36-
"k8s.io/client-go/discovery"
37-
"k8s.io/client-go/rest"
38-
e2e "k8s.io/kubernetes/test/e2e/framework"
3940
)
4041

4142
const (
@@ -170,13 +171,15 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, junitSuiteName string, mon
170171
listContext, listContextCancel := context.WithTimeout(context.Background(), 10*time.Minute)
171172
defer listContextCancel()
172173

173-
envFlags, err := determineEnvironmentFlags(upgrade, o.DryRun)
174+
osEnv := updateEnvVars(o.AsEnv())
175+
176+
clusterEnvFlags, err := determineEnvironmentFlags(upgrade, o.DryRun)
174177
if err != nil {
175178
return fmt.Errorf("could not determine environment flags: %w", err)
176179
}
177-
logrus.WithField("flags", envFlags.String()).Infof("Determined all potential environment flags")
180+
logrus.WithField("flags", clusterEnvFlags.String()).Infof("Determined all potential environment flags")
178181

179-
externalTestSpecs, err := externalBinaries.ListTests(listContext, defaultBinaryParallelism, envFlags)
182+
externalTestSpecs, err := externalBinaries.ListTests(listContext, defaultBinaryParallelism, osEnv, clusterEnvFlags)
180183
if err != nil {
181184
return err
182185
}

0 commit comments

Comments
 (0)