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.

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

Pass applicable environment flags to the external binary #29506

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions pkg/clioptions/clusterdiscovery/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type ClusterState struct {
NetworkSpec *operatorv1.NetworkSpec
ControlPlaneTopology *configv1.TopologyMode
OptionalCapabilities []configv1.ClusterVersionCapability
Version *configv1.ClusterVersion
}

// DiscoverClusterState creates a ClusterState based on a live cluster
Expand Down Expand Up @@ -152,6 +153,7 @@ func DiscoverClusterState(clientConfig *rest.Config) (*ClusterState, error) {
if err != nil {
return nil, err
}
state.Version = clusterVersion
state.OptionalCapabilities = clusterVersion.Status.Capabilities.EnabledCapabilities

return state, nil
Expand Down
37 changes: 29 additions & 8 deletions pkg/test/extensions/binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/mod/semver"
kapierrs "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -76,15 +77,20 @@ func (b *TestBinary) Info(ctx context.Context) (*ExtensionInfo, error) {
return b.info, nil
}

// ListTests returns which tests this binary advertises. Eventually, it should take an environment struct
// to provide to the binary so it can determine for itself which tests are relevant.
func (b *TestBinary) ListTests(ctx context.Context) (ExtensionTestSpecs, error) {
// 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) {
var tests ExtensionTestSpecs
start := time.Now()
binName := filepath.Base(b.binaryPath)

logrus.Infof("Listing tests for %s", binName)
binLogger := logrus.WithField("binary", binName)
binLogger.Info("Listing tests")
binLogger.Infof("OTE API version is: %s", b.info.APIVersion)
envFlags = b.filterToApplicableEnvironmentFlags(envFlags)
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()...)
testList, err := runWithTimeout(ctx, command, 10*time.Minute)
if err != nil {
return nil, fmt.Errorf("failed running '%s list': %w", b.binaryPath, err)
Expand All @@ -107,7 +113,7 @@ func (b *TestBinary) ListTests(ctx context.Context) (ExtensionTestSpecs, error)
extensionTestSpec.Binary = b
tests = append(tests, extensionTestSpec)
}
logrus.Infof("Listed %d tests for %s in %v", len(tests), binName, time.Since(start))
binLogger.Infof("Listed %d tests in %v", len(tests), time.Since(start))
return tests, nil
}

Expand Down Expand Up @@ -391,8 +397,9 @@ func (binaries TestBinaries) Info(ctx context.Context, parallelism int) ([]*Exte
return infos, nil
}

// ListTests extracts the tests from all TestBinaries using the specified parallelism.
func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int) (ExtensionTestSpecs, error) {
// 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) {
var (
allTests ExtensionTestSpecs
mu sync.Mutex
Expand Down Expand Up @@ -426,7 +433,7 @@ func (binaries TestBinaries) ListTests(ctx context.Context, parallelism int) (Ex
if !ok {
return // Channel was closed
}
tests, err := binary.ListTests(ctx)
tests, err := binary.ListTests(ctx, envFlags)
if err != nil {
errCh <- err
}
Expand Down Expand Up @@ -492,3 +499,17 @@ func safeComponentPath(c *Component) string {
safePathRegexp.ReplaceAllString(c.Name, "_"),
)
}

// filterToApplicableEnvironmentFlags filters the provided envFlags to only those that are applicable to the
// APIVersion of OTE within the external binary.
func (b *TestBinary) filterToApplicableEnvironmentFlags(envFlags EnvironmentFlags) EnvironmentFlags {
apiVersion := b.info.APIVersion
filtered := EnvironmentFlags{}
for _, flag := range envFlags {
if semver.Compare(apiVersion, flag.SinceVersion) >= 0 {
filtered = append(filtered, flag)
}
}

return filtered
}
111 changes: 111 additions & 0 deletions pkg/test/extensions/types.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package extensions

import (
"fmt"
"strings"
"time"

"k8s.io/apimachinery/pkg/util/sets"

configv1 "github.com/openshift/api/config/v1"
)

// ExtensionInfo represents an extension to openshift-tests.
Expand Down Expand Up @@ -165,3 +169,110 @@ func (dbt *DBTime) UnmarshalJSON(b []byte) error {
*dbt = (DBTime)(parsedTime)
return nil
}

// EnvironmentFlagName enumerates each possible EnvironmentFlag's name to be passed to the external binary
type EnvironmentFlagName string

const (
platform EnvironmentFlagName = "platform"
network EnvironmentFlagName = "network"
networkStack EnvironmentFlagName = "network-stack"
upgrade EnvironmentFlagName = "upgrade"
topology EnvironmentFlagName = "topology"
architecture EnvironmentFlagName = "architecture"
fact EnvironmentFlagName = "fact"
version EnvironmentFlagName = "version"
)

type EnvironmentFlagsBuilder struct {
flags EnvironmentFlags
}

func (e *EnvironmentFlagsBuilder) AddPlatform(value string) *EnvironmentFlagsBuilder {
e.flags = append(e.flags, newEnvironmentFlag(platform, value))
return e
}

func (e *EnvironmentFlagsBuilder) AddNetwork(value string) *EnvironmentFlagsBuilder {
e.flags = append(e.flags, newEnvironmentFlag(network, value))
return e
}

func (e *EnvironmentFlagsBuilder) AddNetworkStack(value string) *EnvironmentFlagsBuilder {
e.flags = append(e.flags, newEnvironmentFlag(networkStack, value))
return e
}

func (e *EnvironmentFlagsBuilder) AddUpgrade(value string) *EnvironmentFlagsBuilder {
e.flags = append(e.flags, newEnvironmentFlag(upgrade, value))
return e
}

func (e *EnvironmentFlagsBuilder) AddTopology(value *configv1.TopologyMode) *EnvironmentFlagsBuilder {
if value != nil {
e.flags = append(e.flags, newEnvironmentFlag(topology, string(*value)))
}
return e
}

func (e *EnvironmentFlagsBuilder) AddArchitecture(value string) *EnvironmentFlagsBuilder {
e.flags = append(e.flags, newEnvironmentFlag(architecture, value))
return e
}

func (e *EnvironmentFlagsBuilder) AddFact(value string) *EnvironmentFlagsBuilder {
e.flags = append(e.flags, newEnvironmentFlag(fact, value))
return e
}

func (e *EnvironmentFlagsBuilder) AddVersion(value string) *EnvironmentFlagsBuilder {
e.flags = append(e.flags, newEnvironmentFlag(version, value))
return e
}

func (e *EnvironmentFlagsBuilder) Build() EnvironmentFlags {
return e.flags
}

// EnvironmentFlag contains the info required to build an argument to pass to the external binary for the given Name
type EnvironmentFlag struct {
Name EnvironmentFlagName
Value string
SinceVersion string
}

// newEnvironmentFlag creates an EnvironmentFlag including the determination of the SinceVersion
func newEnvironmentFlag(name EnvironmentFlagName, value string) EnvironmentFlag {
return EnvironmentFlag{Name: name, Value: value, SinceVersion: EnvironmentFlagVersions[name]}
}

func (ef EnvironmentFlag) ArgString() string {
return fmt.Sprintf("%s=%s", ef.Name, ef.Value)
}

type EnvironmentFlags []EnvironmentFlag

// ArgStrings properly formats all EnvironmentFlags as a list of argument strings to pass to the external binary
func (ef EnvironmentFlags) ArgStrings() []string {
var argStrings []string
for _, flag := range ef {
argStrings = append(argStrings, flag.ArgString())
}
return argStrings
}

func (ef EnvironmentFlags) String() string {
return strings.Join(ef.ArgStrings(), ", ")
}

// EnvironmentFlagVersions holds the "Since" version metadata for each flag.
var EnvironmentFlagVersions = map[EnvironmentFlagName]string{
platform: "v1.0",
network: "v1.0",
networkStack: "v1.0",
upgrade: "v1.0",
topology: "v1.0",
architecture: "v1.0",
fact: "v1.0", //TODO(sgoeddel): this will be set in a later version
version: "v1.0",
}
82 changes: 73 additions & 9 deletions pkg/test/ginkgo/cmd_runsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,8 @@ import (
"time"

"github.com/onsi/ginkgo/v2"
"github.com/sirupsen/logrus"
"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/origin/pkg/clioptions/clusterdiscovery"
"github.com/openshift/origin/pkg/clioptions/clusterinfo"
"github.com/openshift/origin/pkg/defaultmonitortests"
"github.com/openshift/origin/pkg/monitor"
Expand All @@ -33,6 +28,14 @@ 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 @@ -160,13 +163,20 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, junitSuiteName string, mon
logrus.Infof("Discovered %d extensions", len(extensionsInfo))
for _, e := range extensionsInfo {
id := fmt.Sprintf("%s:%s:%s", e.Component.Product, e.Component.Kind, e.Component.Name)
logrus.Infof("Extension %s found in %s:%s", id, e.Source.SourceImage, e.Source.SourceBinary)
logrus.Infof("Extension %s found in %s:%s using API version %s", id, e.Source.SourceImage, e.Source.SourceBinary, e.APIVersion)
}

// List tests from all available binaries and convert them to origin's testCase format
listContext, listContextCancel := context.WithTimeout(context.Background(), 10*time.Minute)
defer listContextCancel()
externalTestSpecs, err := externalBinaries.ListTests(listContext, defaultBinaryParallelism)

envFlags, 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")

externalTestSpecs, err := externalBinaries.ListTests(listContext, defaultBinaryParallelism, envFlags)
if err != nil {
return err
}
Expand Down Expand Up @@ -683,3 +693,57 @@ outerLoop:
}
return matches, nil
}

func determineEnvironmentFlags(upgrade bool, dryRun bool) (extensions.EnvironmentFlags, error) {
clientConfig, err := e2e.LoadConfig(true)
if err != nil {
logrus.WithError(err).Error("error calling e2e.LoadConfig")
return nil, err
}
clusterState, err := clusterdiscovery.DiscoverClusterState(clientConfig)
if err != nil {
logrus.WithError(err).Error("error Discovering Cluster State")
return nil, err
}
config, err := clusterdiscovery.DecodeProvider(os.Getenv("TEST_PROVIDER"), dryRun, true, clusterState)
if err != nil {
logrus.WithError(err).Error("error determining information about the cluster")
return nil, err
}

upgradeType := "None"
if upgrade {
upgradeType = determineUpgradeType(clusterState.Version.Status)
}

envFlagBuilder := &extensions.EnvironmentFlagsBuilder{}
return envFlagBuilder.
AddPlatform(config.ProviderName).
AddNetwork(config.NetworkPlugin).
AddNetworkStack(config.IPFamily).
AddTopology(clusterState.ControlPlaneTopology).
AddUpgrade(upgradeType).
AddArchitecture(clusterState.Masters.Items[0].Status.NodeInfo.Architecture). //TODO(sgoeddel): eventually, we may need to check every node and pass "multi" as the value if any of them differ from the masters
AddVersion(clusterState.Version.Status.Desired.Version).
Build(), nil
}

func determineUpgradeType(versionStatus configv1.ClusterVersionStatus) string {
history := versionStatus.History
version := versionStatus.Desired.Version
if len(history) > 1 { // If there aren't at least 2 versions in the history we don't have any way to determine the upgrade type
mostRecent := history[1] // history[0] will be the desired version, so check one further back
current := fmt.Sprintf("v%s", version)
last := fmt.Sprintf("v%s", mostRecent.Version)
if semver.Compare(semver.Major(current), semver.Major(last)) > 0 {
return "Major"
}
if semver.Compare(semver.MajorMinor(current), semver.MajorMinor(last)) > 0 {
return "Minor"
}

return "Micro"
}

return "Unknown"
}