Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure OS environment variables are wired into list command as well #29582

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Ensure OS environment variables are wired into list command as well #29582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 15 additions & 8 deletions pkg/test/extensions/binary.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,27 @@ func (b *TestBinary) Info(ctx context.Context) (*ExtensionInfo, error) {
return b.info, nil
}

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

binLogger := logrus.WithField("binary", binName)
binLogger.Info("Listing tests")
binLogger.Infof("OTE API version is: %s", b.info.APIVersion)
envFlags = b.filterToApplicableEnvironmentFlags(envFlags)
clusterEnvFlags = b.filterToApplicableEnvironmentFlags(clusterEnvFlags)
command := exec.Command(b.binaryPath, "list", "-o", "jsonl")
binLogger.Infof("adding the following applicable flags to the list command: %s", envFlags.String())
command.Args = append(command.Args, envFlags.ArgStrings()...)
if len(osEnv) > 0 {
command.Env = osEnv
}

binLogger.Infof("adding the following applicable flags to the list command: %s", clusterEnvFlags.String())
command.Args = append(command.Args, clusterEnvFlags.ArgStrings()...)
testList, err := runWithTimeout(ctx, command, 10*time.Minute)
if err != nil {
return nil, fmt.Errorf("failed running '%s list': %w\nOutput: %s", b.binaryPath, err, testList)
Expand Down Expand Up @@ -504,7 +511,7 @@ func (binaries TestBinaries) ListImages(ctx context.Context, parallelism int) ([

// ListTests extracts the tests from all TestBinaries using the specified parallelism,
// and passes the provided EnvironmentFlags for proper filtering of results.
func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int, envFlags EnvironmentFlags) (ExtensionTestSpecs, error) {
func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int, osEnv []string, clusterEnvFlags EnvironmentFlags) (ExtensionTestSpecs, error) {
var (
allTests ExtensionTestSpecs
mu sync.Mutex
Expand Down Expand Up @@ -538,7 +545,7 @@ func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int, env
if !ok {
return // Channel was closed
}
tests, err := binary.ListTests(ctx, envFlags)
tests, err := binary.ListTests(ctx, osEnv, clusterEnvFlags)
if err != nil {
errCh <- err
}
Expand Down
25 changes: 14 additions & 11 deletions pkg/test/ginkgo/cmd_runsuite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ import (

"github.com/onsi/ginkgo/v2"
configv1 "github.com/openshift/api/config/v1"
"github.com/sirupsen/logrus"
"github.com/spf13/pflag"
"golang.org/x/mod/semver"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"
e2e "k8s.io/kubernetes/test/e2e/framework"

"github.com/openshift/origin/pkg/clioptions/clusterdiscovery"
"github.com/openshift/origin/pkg/clioptions/clusterinfo"
"github.com/openshift/origin/pkg/defaultmonitortests"
Expand All @@ -28,14 +37,6 @@ import (
"github.com/openshift/origin/pkg/riskanalysis"
"github.com/openshift/origin/pkg/test/extensions"
"github.com/openshift/origin/pkg/test/ginkgo/junitapi"
"github.com/sirupsen/logrus"
"github.com/spf13/pflag"
"golang.org/x/mod/semver"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"
e2e "k8s.io/kubernetes/test/e2e/framework"
)

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

envFlags, err := determineEnvironmentFlags(upgrade, o.DryRun)
osEnv := updateEnvVars(o.AsEnv())

clusterEnvFlags, err := determineEnvironmentFlags(upgrade, o.DryRun)
if err != nil {
return fmt.Errorf("could not determine environment flags: %w", err)
}
logrus.WithField("flags", envFlags.String()).Infof("Determined all potential environment flags")
logrus.WithField("flags", clusterEnvFlags.String()).Infof("Determined all potential environment flags")

externalTestSpecs, err := externalBinaries.ListTests(listContext, defaultBinaryParallelism, envFlags)
externalTestSpecs, err := externalBinaries.ListTests(listContext, defaultBinaryParallelism, osEnv, clusterEnvFlags)
if err != nil {
return err
}
Expand Down