From af2057f9ed66082391a2af1800f16af4334809fe Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 21 Jan 2021 09:38:45 -0500 Subject: [PATCH 1/7] Add unit tests for "openshift-tests --provider" --- cmd/openshift-tests/e2e.go | 2 +- cmd/openshift-tests/openshift-tests.go | 2 +- cmd/openshift-tests/provider.go | 25 ++- cmd/openshift-tests/provider_test.go | 263 +++++++++++++++++++++++++ test/extended/util/cloud/cloud.go | 93 +++++---- 5 files changed, 340 insertions(+), 45 deletions(-) create mode 100644 cmd/openshift-tests/provider_test.go diff --git a/cmd/openshift-tests/e2e.go b/cmd/openshift-tests/e2e.go index 5b406b981189..13b1ddd9d3a8 100644 --- a/cmd/openshift-tests/e2e.go +++ b/cmd/openshift-tests/e2e.go @@ -390,7 +390,7 @@ func isStandardEarlyOrLateTest(name string) bool { // suiteWithInitializedProviderPreSuite loads the provider info, but does not // exclude any tests specific to that provider. func suiteWithInitializedProviderPreSuite(opt *runOptions) error { - config, err := decodeProvider(opt.Provider, opt.DryRun, true) + config, err := decodeProvider(opt.Provider, opt.DryRun, true, nil) if err != nil { return err } diff --git a/cmd/openshift-tests/openshift-tests.go b/cmd/openshift-tests/openshift-tests.go index 3deea3866612..084650eeeb63 100644 --- a/cmd/openshift-tests/openshift-tests.go +++ b/cmd/openshift-tests/openshift-tests.go @@ -406,7 +406,7 @@ func newRunTestCommand() *cobra.Command { ginkgo.GlobalSuite().ClearBeforeSuiteNode() ginkgo.GlobalSuite().ClearAfterSuiteNode() - config, err := decodeProvider(os.Getenv("TEST_PROVIDER"), testOpt.DryRun, false) + config, err := decodeProvider(os.Getenv("TEST_PROVIDER"), testOpt.DryRun, false, nil) if err != nil { return err } diff --git a/cmd/openshift-tests/provider.go b/cmd/openshift-tests/provider.go index 5824ae10f3e0..d060569ae7fa 100644 --- a/cmd/openshift-tests/provider.go +++ b/cmd/openshift-tests/provider.go @@ -90,7 +90,7 @@ func getProviderMatchFn(config *exutilcloud.ClusterConfiguration) TestNameMatche return matchFn } -func decodeProvider(provider string, dryRun, discover bool) (*exutilcloud.ClusterConfiguration, error) { +func decodeProvider(provider string, dryRun, discover bool, clusterState *exutilcloud.ClusterState) (*exutilcloud.ClusterConfiguration, error) { switch provider { case "none": return &exutilcloud.ClusterConfiguration{ProviderName: "skeleton"}, nil @@ -107,11 +107,17 @@ func decodeProvider(provider string, dryRun, discover bool) (*exutilcloud.Cluste fallthrough case "azure", "aws", "baremetal", "gce", "vsphere": - clientConfig, err := e2e.LoadConfig(true) - if err != nil { - return nil, err + if clusterState == nil { + clientConfig, err := e2e.LoadConfig(true) + if err != nil { + return nil, err + } + clusterState, err = exutilcloud.DiscoverClusterState(clientConfig) + if err != nil { + return nil, err + } } - config, err := exutilcloud.LoadConfig(clientConfig) + config, err := exutilcloud.LoadConfig(clusterState) if err != nil { return nil, err } @@ -136,8 +142,13 @@ func decodeProvider(provider string, dryRun, discover bool) (*exutilcloud.Cluste // object that can be overridden var config *exutilcloud.ClusterConfiguration if discover { - if clientConfig, err := e2e.LoadConfig(true); err == nil { - config, _ = exutilcloud.LoadConfig(clientConfig) + if clusterState == nil { + if clientConfig, err := e2e.LoadConfig(true); err == nil { + clusterState, _ = exutilcloud.DiscoverClusterState(clientConfig) + } + } + if clusterState != nil { + config, _ = exutilcloud.LoadConfig(clusterState) } } if config == nil { diff --git a/cmd/openshift-tests/provider_test.go b/cmd/openshift-tests/provider_test.go new file mode 100644 index 000000000000..e79e64bee4c0 --- /dev/null +++ b/cmd/openshift-tests/provider_test.go @@ -0,0 +1,263 @@ +package main + +import ( + "os" + "testing" + + configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" + exutilcloud "github.com/openshift/origin/test/extended/util/cloud" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" +) + +var gcePlatform = &configv1.PlatformStatus{ + Type: configv1.GCPPlatformType, + GCP: &configv1.GCPPlatformStatus{ + ProjectID: "openshift-gce-devel-ci", + Region: "us-east1", + }, +} + +var awsPlatform = &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + Region: "us-east-2", + }, +} + +var vspherePlatform = &configv1.PlatformStatus{ + Type: configv1.VSpherePlatformType, +} + +var noPlatform = &configv1.PlatformStatus{ + Type: configv1.NonePlatformType, +} + +var gceMasters = &corev1.NodeList{ + Items: []corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "master-1", + Labels: map[string]string{ + "failure-domain.beta.kubernetes.io/zone": "us-east1-a", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "master-2", + Labels: map[string]string{ + "failure-domain.beta.kubernetes.io/zone": "us-east1-b", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "master-3", + Labels: map[string]string{ + "failure-domain.beta.kubernetes.io/zone": "us-east1-c", + }, + }, + }, + }, +} + +var simpleMasters = &corev1.NodeList{ + Items: []corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "master-1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "master-2", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "master-3", + }, + }, + }, +} + +var nonMasters = &corev1.NodeList{ + Items: []corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "worker-1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "worker-2", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "worker-3", + }, + }, + }, +} + +var sdnConfig = &operatorv1.NetworkSpec{ + DefaultNetwork: operatorv1.DefaultNetworkDefinition{ + Type: operatorv1.NetworkTypeOpenShiftSDN, + OpenShiftSDNConfig: &operatorv1.OpenShiftSDNConfig{}, + }, +} + +var multitenantConfig = &operatorv1.NetworkSpec{ + DefaultNetwork: operatorv1.DefaultNetworkDefinition{ + Type: operatorv1.NetworkTypeOpenShiftSDN, + OpenShiftSDNConfig: &operatorv1.OpenShiftSDNConfig{ + Mode: operatorv1.SDNModeMultitenant, + }, + }, +} + +var ovnKubernetesConfig = &operatorv1.NetworkSpec{ + DefaultNetwork: operatorv1.DefaultNetworkDefinition{ + Type: operatorv1.NetworkTypeOVNKubernetes, + }, +} + +var e2eTests = map[string]string{ + "everyone": "[Skipped:Wednesday]", + "not-gce": "[Skipped:gce]", + "not-aws": "[Skipped:aws]", + "not-sdn": "[Skipped:Network/OpenShiftSDN]", + "not-multitenant": "[Skipped:Network/OpenShiftSDN/Multitenant]", + "online": "[Skipped:Disconnected]", +} + +func TestDecodeProvider(t *testing.T) { + var testCases = []struct { + name string + provider string + + discoveredPlatform *configv1.PlatformStatus + discoveredMasters *corev1.NodeList + discoveredNetwork *operatorv1.NetworkSpec + + expectedConfig string + runTests sets.String + }{ + { + name: "simple GCE", + provider: "", + discoveredPlatform: gcePlatform, + discoveredMasters: gceMasters, + discoveredNetwork: sdnConfig, + expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + runTests: sets.NewString("everyone", "not-aws", "not-multitenant", "online"), + }, + { + name: "GCE multitenant", + provider: "", + discoveredPlatform: gcePlatform, + discoveredMasters: gceMasters, + discoveredNetwork: multitenantConfig, + expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN","OpenShiftSDN/Multitenant"]}`, + runTests: sets.NewString("everyone", "not-aws", "online"), + }, + { + name: "simple non-cloud", + provider: "", + discoveredPlatform: noPlatform, + discoveredMasters: simpleMasters, + discoveredNetwork: sdnConfig, + expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":0,"MultiMaster":false,"MultiZone":false,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), + }, + { + name: "simple override", + provider: "vsphere", + discoveredPlatform: vspherePlatform, + discoveredMasters: simpleMasters, + discoveredNetwork: sdnConfig, + // NB: It does not actually use the passed-in Provider value + expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), + }, + { + name: "json simple override", + provider: `{"type": "openstack"}`, + discoveredPlatform: noPlatform, + discoveredMasters: simpleMasters, + discoveredNetwork: sdnConfig, + expectedConfig: `{"type":"openstack","ProjectID":"","Region":"","Zone":"","NumNodes":0,"MultiMaster":false,"MultiZone":false,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), + }, + { + name: "complex override", + provider: `{"type":"aws","region":"us-east-2","zone":"us-east-2a","multimaster":false,"multizone":true}`, + discoveredPlatform: awsPlatform, + discoveredMasters: simpleMasters, + discoveredNetwork: ovnKubernetesConfig, + // NB: MultiMaster and MultiZone are set from discovery, with the + // JSON being ignored + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OVNKubernetes"]}`, + runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), + }, + { + name: "complex override without discovery", + provider: `{"type":"aws","region":"us-east-2","zone":"us-east-2a","multimaster":false,"multizone":true}`, + discoveredPlatform: nil, + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":0,"MultiMaster":false,"MultiZone":true,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":null}`, + runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), + }, + { + name: "disconnected", + provider: `{"type":"none","disconnected":true}`, + discoveredPlatform: noPlatform, + discoveredMasters: simpleMasters, + discoveredNetwork: ovnKubernetesConfig, + expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":0,"MultiMaster":false,"MultiZone":false,"Zones":null,"ConfigFile":"","Disconnected":true,"NetworkPluginIDs":["OVNKubernetes"]}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-sdn", "not-multitenant"), + }, + } + + // Unset these to keep decodeProvider from returning "local" + os.Unsetenv("KUBE_SSH_USER") + os.Unsetenv("LOCAL_SSH_KEY") + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + discover := tc.discoveredPlatform != nil + var testState *exutilcloud.ClusterState + if discover { + testState = &exutilcloud.ClusterState{ + PlatformStatus: tc.discoveredPlatform, + Masters: tc.discoveredMasters, + NonMasters: nonMasters, + NetworkSpec: tc.discoveredNetwork, + } + } + config, err := decodeProvider(tc.provider, false, discover, testState) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + configJSON := config.ToJSONString() + if configJSON != tc.expectedConfig { + t.Fatalf("Generated config:\n%s\ndoes not match expected:\n%s\n", configJSON, tc.expectedConfig) + } + matchFn := getProviderMatchFn(config) + + runTests := sets.NewString() + for name, tags := range e2eTests { + if matchFn(name + " " + tags) { + runTests.Insert(name) + } + } + if !runTests.Equal(tc.runTests) { + t.Fatalf("Matched tests:\n%v\ndid not match expected:\n%v\n", runTests.List(), tc.runTests.List()) + } + }) + } +} diff --git a/test/extended/util/cloud/cloud.go b/test/extended/util/cloud/cloud.go index 264dbc478054..428e310df7dd 100644 --- a/test/extended/util/cloud/cloud.go +++ b/test/extended/util/cloud/cloud.go @@ -6,12 +6,14 @@ import ( "fmt" "io/ioutil" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" configclient "github.com/openshift/client-go/config/clientset/versioned" operatorclient "github.com/openshift/client-go/operator/clientset/versioned" "github.com/openshift/origin/test/extended/util/azure" @@ -43,10 +45,17 @@ func (c *ClusterConfiguration) ToJSONString() string { return string(out) } -// LoadConfig uses the cluster to setup the cloud provider config. -func LoadConfig(clientConfig *rest.Config) (*ClusterConfiguration, error) { - // LoadClientset but don't set the UserAgent to include the current test name because - // we don't run any test yet and this call panics +// ClusterState provides information about the cluster that is used to generate +// ClusterConfiguration +type ClusterState struct { + PlatformStatus *configv1.PlatformStatus + Masters *corev1.NodeList + NonMasters *corev1.NodeList + NetworkSpec *operatorv1.NetworkSpec +} + +// DiscoverClusterState creates a ClusterState based on a live cluster +func DiscoverClusterState(clientConfig *rest.Config) (*ClusterState, error) { coreClient, err := clientset.NewForConfig(clientConfig) if err != nil { return nil, err @@ -60,50 +69,62 @@ func LoadConfig(clientConfig *rest.Config) (*ClusterConfiguration, error) { return nil, err } - var networkPluginIDs []string - if networkConfig, err := operatorClient.OperatorV1().Networks().Get(context.Background(), "cluster", metav1.GetOptions{}); err == nil { - networkPluginIDs = append(networkPluginIDs, string(networkConfig.Spec.DefaultNetwork.Type)) - if networkConfig.Spec.DefaultNetwork.OpenShiftSDNConfig != nil && networkConfig.Spec.DefaultNetwork.OpenShiftSDNConfig.Mode != "" { - networkPluginIDs = append(networkPluginIDs, string(networkConfig.Spec.DefaultNetwork.Type)+"/"+string(networkConfig.Spec.DefaultNetwork.OpenShiftSDNConfig.Mode)) - } - - } + state := &ClusterState{} infra, err := configClient.ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{}) if err != nil { return nil, err } - p := infra.Status.PlatformStatus - if p == nil { + state.PlatformStatus = infra.Status.PlatformStatus + if state.PlatformStatus == nil { return nil, fmt.Errorf("status.platformStatus must be set") } - if p.Type == configv1.NonePlatformType { - return &ClusterConfiguration{ - NetworkPluginIDs: networkPluginIDs, - }, nil - } - masters, err := coreClient.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{ + state.Masters, err = coreClient.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master=", }) if err != nil { return nil, err } - zones := sets.NewString() - for _, node := range masters.Items { - zones.Insert(node.Labels["failure-domain.beta.kubernetes.io/zone"]) - } - zones.Delete("") - nonMasters, err := coreClient.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{ + state.NonMasters, err = coreClient.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{ LabelSelector: "!node-role.kubernetes.io/master", }) if err != nil { return nil, err } + networkConfig, err := operatorClient.OperatorV1().Networks().Get(context.Background(), "cluster", metav1.GetOptions{}) + if err != nil { + return nil, err + } + state.NetworkSpec = &networkConfig.Spec + + return state, nil +} + +// LoadConfig generates a ClusterConfiguration based on a detected or hard-coded ClusterState +func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { + var networkPluginIDs []string + networkPluginIDs = append(networkPluginIDs, string(state.NetworkSpec.DefaultNetwork.Type)) + if state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig != nil && state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode != "" { + networkPluginIDs = append(networkPluginIDs, string(state.NetworkSpec.DefaultNetwork.Type)+"/"+string(state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode)) + } + + if state.PlatformStatus.Type == configv1.NonePlatformType { + return &ClusterConfiguration{ + NetworkPluginIDs: networkPluginIDs, + }, nil + } + + zones := sets.NewString() + for _, node := range state.Masters.Items { + zones.Insert(node.Labels["failure-domain.beta.kubernetes.io/zone"]) + } + zones.Delete("") + config := &ClusterConfiguration{ - MultiMaster: len(masters.Items) > 1, + MultiMaster: len(state.Masters.Items) > 1, MultiZone: zones.Len() > 1, Zones: zones.List(), NetworkPluginIDs: networkPluginIDs, @@ -111,23 +132,23 @@ func LoadConfig(clientConfig *rest.Config) (*ClusterConfiguration, error) { if zones.Len() > 0 { config.Zone = zones.List()[0] } - if len(nonMasters.Items) == 0 { - config.NumNodes = len(nonMasters.Items) + if len(state.NonMasters.Items) == 0 { + config.NumNodes = len(state.NonMasters.Items) } else { - config.NumNodes = len(masters.Items) + config.NumNodes = len(state.Masters.Items) } switch { - case p.AWS != nil: + case state.PlatformStatus.AWS != nil: config.ProviderName = "aws" - config.Region = p.AWS.Region + config.Region = state.PlatformStatus.AWS.Region - case p.GCP != nil: + case state.PlatformStatus.GCP != nil: config.ProviderName = "gce" - config.ProjectID = p.GCP.ProjectID - config.Region = p.GCP.Region + config.ProjectID = state.PlatformStatus.GCP.ProjectID + config.Region = state.PlatformStatus.GCP.Region - case p.Azure != nil: + case state.PlatformStatus.Azure != nil: config.ProviderName = "azure" data, err := azure.LoadConfigFile() From dce3913f1825411b3a279784f57a063034341b0b Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 20 Jan 2021 09:00:32 -0500 Subject: [PATCH 2/7] Rename test/extended/util/cloud to test/extended/util/cluster It already contains non-cloud-specific network info, and will soon contain more. --- cmd/openshift-tests/openshift-tests.go | 4 +-- cmd/openshift-tests/provider.go | 26 +++++++++---------- cmd/openshift-tests/provider_test.go | 6 ++--- .../{cloud/cloud.go => cluster/cluster.go} | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) rename test/extended/util/{cloud/cloud.go => cluster/cluster.go} (99%) diff --git a/cmd/openshift-tests/openshift-tests.go b/cmd/openshift-tests/openshift-tests.go index 084650eeeb63..76c5d83e1788 100644 --- a/cmd/openshift-tests/openshift-tests.go +++ b/cmd/openshift-tests/openshift-tests.go @@ -29,7 +29,7 @@ import ( testginkgo "github.com/openshift/origin/pkg/test/ginkgo" "github.com/openshift/origin/pkg/version" exutil "github.com/openshift/origin/test/extended/util" - "github.com/openshift/origin/test/extended/util/cloud" + "github.com/openshift/origin/test/extended/util/cluster" ) func main() { @@ -204,7 +204,7 @@ type runOptions struct { TestOptions []string // Shared by initialization code - config *cloud.ClusterConfiguration + config *cluster.ClusterConfiguration } func (opt *runOptions) AsEnv() []string { diff --git a/cmd/openshift-tests/provider.go b/cmd/openshift-tests/provider.go index d060569ae7fa..1a13a26a7080 100644 --- a/cmd/openshift-tests/provider.go +++ b/cmd/openshift-tests/provider.go @@ -13,7 +13,7 @@ import ( e2e "k8s.io/kubernetes/test/e2e/framework" exutil "github.com/openshift/origin/test/extended/util" - exutilcloud "github.com/openshift/origin/test/extended/util/cloud" + exutilcluster "github.com/openshift/origin/test/extended/util/cluster" // Initialize baremetal as a provider _ "github.com/openshift/origin/test/extended/util/baremetal" @@ -31,7 +31,7 @@ import ( type TestNameMatchesFunc func(name string) bool -func initializeTestFramework(context *e2e.TestContextType, config *exutilcloud.ClusterConfiguration, dryRun bool) error { +func initializeTestFramework(context *e2e.TestContextType, config *exutilcluster.ClusterConfiguration, dryRun bool) error { // update context with loaded config context.Provider = config.ProviderName context.CloudConfig = e2e.CloudConfig{ @@ -67,7 +67,7 @@ func initializeTestFramework(context *e2e.TestContextType, config *exutilcloud.C return nil } -func getProviderMatchFn(config *exutilcloud.ClusterConfiguration) TestNameMatchesFunc { +func getProviderMatchFn(config *exutilcluster.ClusterConfiguration) TestNameMatchesFunc { // given the configuration we have loaded, skip tests that our provider, disconnected status, // or our network plugin should exclude var skips []string @@ -90,19 +90,19 @@ func getProviderMatchFn(config *exutilcloud.ClusterConfiguration) TestNameMatche return matchFn } -func decodeProvider(provider string, dryRun, discover bool, clusterState *exutilcloud.ClusterState) (*exutilcloud.ClusterConfiguration, error) { +func decodeProvider(provider string, dryRun, discover bool, clusterState *exutilcluster.ClusterState) (*exutilcluster.ClusterConfiguration, error) { switch provider { case "none": - return &exutilcloud.ClusterConfiguration{ProviderName: "skeleton"}, nil + return &exutilcluster.ClusterConfiguration{ProviderName: "skeleton"}, nil case "": if _, ok := os.LookupEnv("KUBE_SSH_USER"); ok { if _, ok := os.LookupEnv("LOCAL_SSH_KEY"); ok { - return &exutilcloud.ClusterConfiguration{ProviderName: "local"}, nil + return &exutilcluster.ClusterConfiguration{ProviderName: "local"}, nil } } if dryRun { - return &exutilcloud.ClusterConfiguration{ProviderName: "skeleton"}, nil + return &exutilcluster.ClusterConfiguration{ProviderName: "skeleton"}, nil } fallthrough @@ -112,12 +112,12 @@ func decodeProvider(provider string, dryRun, discover bool, clusterState *exutil if err != nil { return nil, err } - clusterState, err = exutilcloud.DiscoverClusterState(clientConfig) + clusterState, err = exutilcluster.DiscoverClusterState(clientConfig) if err != nil { return nil, err } } - config, err := exutilcloud.LoadConfig(clusterState) + config, err := exutilcluster.LoadConfig(clusterState) if err != nil { return nil, err } @@ -140,19 +140,19 @@ func decodeProvider(provider string, dryRun, discover bool, clusterState *exutil } // attempt to load the default config, then overwrite with any values from the passed // object that can be overridden - var config *exutilcloud.ClusterConfiguration + var config *exutilcluster.ClusterConfiguration if discover { if clusterState == nil { if clientConfig, err := e2e.LoadConfig(true); err == nil { - clusterState, _ = exutilcloud.DiscoverClusterState(clientConfig) + clusterState, _ = exutilcluster.DiscoverClusterState(clientConfig) } } if clusterState != nil { - config, _ = exutilcloud.LoadConfig(clusterState) + config, _ = exutilcluster.LoadConfig(clusterState) } } if config == nil { - config = &exutilcloud.ClusterConfiguration{ + config = &exutilcluster.ClusterConfiguration{ ProviderName: providerInfo.Type, ProjectID: providerInfo.ProjectID, Region: providerInfo.Region, diff --git a/cmd/openshift-tests/provider_test.go b/cmd/openshift-tests/provider_test.go index e79e64bee4c0..ffdac937d95a 100644 --- a/cmd/openshift-tests/provider_test.go +++ b/cmd/openshift-tests/provider_test.go @@ -6,7 +6,7 @@ import ( configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" - exutilcloud "github.com/openshift/origin/test/extended/util/cloud" + exutilcluster "github.com/openshift/origin/test/extended/util/cluster" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -230,9 +230,9 @@ func TestDecodeProvider(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { discover := tc.discoveredPlatform != nil - var testState *exutilcloud.ClusterState + var testState *exutilcluster.ClusterState if discover { - testState = &exutilcloud.ClusterState{ + testState = &exutilcluster.ClusterState{ PlatformStatus: tc.discoveredPlatform, Masters: tc.discoveredMasters, NonMasters: nonMasters, diff --git a/test/extended/util/cloud/cloud.go b/test/extended/util/cluster/cluster.go similarity index 99% rename from test/extended/util/cloud/cloud.go rename to test/extended/util/cluster/cluster.go index 428e310df7dd..412bfb1bb01a 100644 --- a/test/extended/util/cloud/cloud.go +++ b/test/extended/util/cluster/cluster.go @@ -1,4 +1,4 @@ -package cloud +package cluster import ( "context" From 34223cbeaf835c56e59f6f0abebe4e0aa95b5e9f Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 21 Jan 2021 16:04:19 -0500 Subject: [PATCH 3/7] Move test skip/match handling into ClusterConfig --- cmd/openshift-tests/e2e.go | 6 +++--- cmd/openshift-tests/provider.go | 26 -------------------------- cmd/openshift-tests/provider_test.go | 2 +- test/extended/util/cluster/cluster.go | 23 +++++++++++++++++++++++ 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/cmd/openshift-tests/e2e.go b/cmd/openshift-tests/e2e.go index 13b1ddd9d3a8..1482cc3dbd64 100644 --- a/cmd/openshift-tests/e2e.go +++ b/cmd/openshift-tests/e2e.go @@ -401,13 +401,13 @@ func suiteWithInitializedProviderPreSuite(opt *runOptions) error { } // suiteWithProviderPreSuite ensures that the suite filters out tests from providers -// that aren't relevant (see getProviderMatchFn) by loading the provider info from the -// cluster or flags. +// that aren't relevant (see exutilcluster.ClusterConfig.MatchFn) by loading the +// provider info from the cluster or flags. func suiteWithProviderPreSuite(opt *runOptions) error { if err := suiteWithInitializedProviderPreSuite(opt); err != nil { return err } - opt.MatchFn = getProviderMatchFn(opt.config) + opt.MatchFn = opt.config.MatchFn() return nil } diff --git a/cmd/openshift-tests/provider.go b/cmd/openshift-tests/provider.go index 1a13a26a7080..541e55fbc11a 100644 --- a/cmd/openshift-tests/provider.go +++ b/cmd/openshift-tests/provider.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "os" - "strings" "github.com/onsi/ginkgo" "github.com/onsi/gomega" @@ -29,8 +28,6 @@ import ( _ "k8s.io/kubernetes/test/e2e/lifecycle" ) -type TestNameMatchesFunc func(name string) bool - func initializeTestFramework(context *e2e.TestContextType, config *exutilcluster.ClusterConfiguration, dryRun bool) error { // update context with loaded config context.Provider = config.ProviderName @@ -67,29 +64,6 @@ func initializeTestFramework(context *e2e.TestContextType, config *exutilcluster return nil } -func getProviderMatchFn(config *exutilcluster.ClusterConfiguration) TestNameMatchesFunc { - // given the configuration we have loaded, skip tests that our provider, disconnected status, - // or our network plugin should exclude - var skips []string - skips = append(skips, fmt.Sprintf("[Skipped:%s]", config.ProviderName)) - for _, id := range config.NetworkPluginIDs { - skips = append(skips, fmt.Sprintf("[Skipped:Network/%s]", id)) - } - if config.Disconnected { - skips = append(skips, "[Skipped:Disconnected]") - } - - matchFn := func(name string) bool { - for _, skip := range skips { - if strings.Contains(name, skip) { - return false - } - } - return true - } - return matchFn -} - func decodeProvider(provider string, dryRun, discover bool, clusterState *exutilcluster.ClusterState) (*exutilcluster.ClusterConfiguration, error) { switch provider { case "none": diff --git a/cmd/openshift-tests/provider_test.go b/cmd/openshift-tests/provider_test.go index ffdac937d95a..ad2faebd2b1f 100644 --- a/cmd/openshift-tests/provider_test.go +++ b/cmd/openshift-tests/provider_test.go @@ -247,7 +247,7 @@ func TestDecodeProvider(t *testing.T) { if configJSON != tc.expectedConfig { t.Fatalf("Generated config:\n%s\ndoes not match expected:\n%s\n", configJSON, tc.expectedConfig) } - matchFn := getProviderMatchFn(config) + matchFn := config.MatchFn() runTests := sets.NewString() for name, tags := range e2eTests { diff --git a/test/extended/util/cluster/cluster.go b/test/extended/util/cluster/cluster.go index 412bfb1bb01a..b8c97928b8c2 100644 --- a/test/extended/util/cluster/cluster.go +++ b/test/extended/util/cluster/cluster.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "strings" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -168,3 +169,25 @@ func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { return config, nil } + +// MatchFn returns a function that tests if a named function should be run based on +// the cluster configuration +func (c *ClusterConfiguration) MatchFn() func(string) bool { + var skips []string + skips = append(skips, fmt.Sprintf("[Skipped:%s]", c.ProviderName)) + for _, id := range c.NetworkPluginIDs { + skips = append(skips, fmt.Sprintf("[Skipped:Network/%s]", id)) + } + if c.Disconnected { + skips = append(skips, "[Skipped:Disconnected]") + } + matchFn := func(name string) bool { + for _, skip := range skips { + if strings.Contains(name, skip) { + return false + } + } + return true + } + return matchFn +} From 56c2529bec14ef8cb52aecd12f61f0409c4e744f Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 21 Jan 2021 15:55:19 -0500 Subject: [PATCH 4/7] Remove a weird short-circuit case in test config detection We shouldn't avoid detecting NumNodes and MultiMaster just because Platform is None. --- cmd/openshift-tests/provider_test.go | 6 +++--- test/extended/util/cluster/cluster.go | 6 ------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/cmd/openshift-tests/provider_test.go b/cmd/openshift-tests/provider_test.go index ad2faebd2b1f..147f3eb7bb58 100644 --- a/cmd/openshift-tests/provider_test.go +++ b/cmd/openshift-tests/provider_test.go @@ -172,7 +172,7 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":0,"MultiMaster":false,"MultiZone":false,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), }, { @@ -191,7 +191,7 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"openstack","ProjectID":"","Region":"","Zone":"","NumNodes":0,"MultiMaster":false,"MultiZone":false,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + expectedConfig: `{"type":"openstack","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), }, { @@ -218,7 +218,7 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":0,"MultiMaster":false,"MultiZone":false,"Zones":null,"ConfigFile":"","Disconnected":true,"NetworkPluginIDs":["OVNKubernetes"]}`, + expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":true,"NetworkPluginIDs":["OVNKubernetes"]}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-sdn", "not-multitenant"), }, } diff --git a/test/extended/util/cluster/cluster.go b/test/extended/util/cluster/cluster.go index b8c97928b8c2..896ededc105d 100644 --- a/test/extended/util/cluster/cluster.go +++ b/test/extended/util/cluster/cluster.go @@ -112,12 +112,6 @@ func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { networkPluginIDs = append(networkPluginIDs, string(state.NetworkSpec.DefaultNetwork.Type)+"/"+string(state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode)) } - if state.PlatformStatus.Type == configv1.NonePlatformType { - return &ClusterConfiguration{ - NetworkPluginIDs: networkPluginIDs, - }, nil - } - zones := sets.NewString() for _, node := range state.Masters.Items { zones.Insert(node.Labels["failure-domain.beta.kubernetes.io/zone"]) From 695f0553d399818e54ad99c608682ebda601c7e6 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 20 Jan 2021 09:18:58 -0500 Subject: [PATCH 5/7] Simplify JSON --provider handling When the user passes "--provider ", rather than reading it into an e2e.CloudConfig, autodetecting the exutilcluster.ClusterConfig, and then merging the two structs together field by field, just do the autodetection first, and then unmarshal the JSON directly into the ClusterConfig (possibly overwriting some of the autodetected values). In addition to being simpler, this allows for overriding the networking-related values that are in ClusterConfig but not CloudConfig. --- cmd/openshift-tests/provider.go | 46 ++++----------------------- cmd/openshift-tests/provider_test.go | 15 ++++++--- test/extended/util/cluster/cluster.go | 3 +- 3 files changed, 20 insertions(+), 44 deletions(-) diff --git a/cmd/openshift-tests/provider.go b/cmd/openshift-tests/provider.go index 541e55fbc11a..84cd6fe5dcc2 100644 --- a/cmd/openshift-tests/provider.go +++ b/cmd/openshift-tests/provider.go @@ -102,18 +102,14 @@ func decodeProvider(provider string, dryRun, discover bool, clusterState *exutil default: var providerInfo struct { - Type string - Disconnected bool - e2e.CloudConfig `json:",inline"` + Type string } if err := json.Unmarshal([]byte(provider), &providerInfo); err != nil { - return nil, fmt.Errorf("provider must be a JSON object with the 'type' key at a minimum, and decode into a cloud config object: %v", err) + return nil, fmt.Errorf("provider must be a JSON object with the 'type' key at a minimum: %v", err) } if len(providerInfo.Type) == 0 { return nil, fmt.Errorf("provider must be a JSON object with the 'type' key") } - // attempt to load the default config, then overwrite with any values from the passed - // object that can be overridden var config *exutilcluster.ClusterConfiguration if discover { if clusterState == nil { @@ -124,41 +120,13 @@ func decodeProvider(provider string, dryRun, discover bool, clusterState *exutil if clusterState != nil { config, _ = exutilcluster.LoadConfig(clusterState) } - } - if config == nil { - config = &exutilcluster.ClusterConfiguration{ - ProviderName: providerInfo.Type, - ProjectID: providerInfo.ProjectID, - Region: providerInfo.Region, - Zone: providerInfo.Zone, - Zones: providerInfo.Zones, - NumNodes: providerInfo.NumNodes, - MultiMaster: providerInfo.MultiMaster, - MultiZone: providerInfo.MultiZone, - ConfigFile: providerInfo.ConfigFile, - } } else { - config.ProviderName = providerInfo.Type - if len(providerInfo.ProjectID) > 0 { - config.ProjectID = providerInfo.ProjectID - } - if len(providerInfo.Region) > 0 { - config.Region = providerInfo.Region - } - if len(providerInfo.Zone) > 0 { - config.Zone = providerInfo.Zone - } - if len(providerInfo.Zones) > 0 { - config.Zones = providerInfo.Zones - } - if len(providerInfo.ConfigFile) > 0 { - config.ConfigFile = providerInfo.ConfigFile - } - if providerInfo.NumNodes > 0 { - config.NumNodes = providerInfo.NumNodes - } + config = &exutilcluster.ClusterConfiguration{} + } + + if err := json.Unmarshal([]byte(provider), config); err != nil { + return nil, fmt.Errorf("provider must decode into the ClusterConfig object: %v", err) } - config.Disconnected = providerInfo.Disconnected return config, nil } } diff --git a/cmd/openshift-tests/provider_test.go b/cmd/openshift-tests/provider_test.go index 147f3eb7bb58..ee35630d2405 100644 --- a/cmd/openshift-tests/provider_test.go +++ b/cmd/openshift-tests/provider_test.go @@ -200,10 +200,8 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: awsPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - // NB: MultiMaster and MultiZone are set from discovery, with the - // JSON being ignored - expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OVNKubernetes"]}`, - runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":3,"MultiMaster":false,"MultiZone":true,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OVNKubernetes"]}`, + runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), }, { name: "complex override without discovery", @@ -221,6 +219,15 @@ func TestDecodeProvider(t *testing.T) { expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":true,"NetworkPluginIDs":["OVNKubernetes"]}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-sdn", "not-multitenant"), }, + { + name: "override network plugin", + provider: `{"type":"aws","networkPluginIDs":["Calico"]}`, + discoveredPlatform: awsPlatform, + discoveredMasters: simpleMasters, + discoveredNetwork: ovnKubernetesConfig, + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["Calico"]}`, + runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), + }, } // Unset these to keep decodeProvider from returning "local" diff --git a/test/extended/util/cluster/cluster.go b/test/extended/util/cluster/cluster.go index 896ededc105d..c9951474f21d 100644 --- a/test/extended/util/cluster/cluster.go +++ b/test/extended/util/cluster/cluster.go @@ -23,7 +23,8 @@ import ( type ClusterConfiguration struct { ProviderName string `json:"type"` - // These fields chosen to match the e2e configuration we fill + // These fields (and the "type" tag for ProviderName) chosen to match + // upstream's e2e.CloudConfig. ProjectID string Region string Zone string From c084cc7aee7453518b5c424e9d77970bac7b5cca Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 20 Jan 2021 09:38:40 -0500 Subject: [PATCH 6/7] Clean up network plugin detection/config Redo "NetworkPluginIDs" as "NetworkPlugin" and "NetworkPluginMode", which make more sense as part of a visible API. --- cmd/openshift-tests/provider_test.go | 20 +++++++-------- test/extended/util/cluster/cluster.go | 36 ++++++++++++++++----------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/cmd/openshift-tests/provider_test.go b/cmd/openshift-tests/provider_test.go index ee35630d2405..9d452ca9a04f 100644 --- a/cmd/openshift-tests/provider_test.go +++ b/cmd/openshift-tests/provider_test.go @@ -154,7 +154,7 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: gcePlatform, discoveredMasters: gceMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, runTests: sets.NewString("everyone", "not-aws", "not-multitenant", "online"), }, { @@ -163,7 +163,7 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: gcePlatform, discoveredMasters: gceMasters, discoveredNetwork: multitenantConfig, - expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN","OpenShiftSDN/Multitenant"]}`, + expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN","NetworkPluginMode":"Multitenant"}`, runTests: sets.NewString("everyone", "not-aws", "online"), }, { @@ -172,7 +172,7 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), }, { @@ -182,7 +182,7 @@ func TestDecodeProvider(t *testing.T) { discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, // NB: It does not actually use the passed-in Provider value - expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), }, { @@ -191,7 +191,7 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"openstack","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OpenShiftSDN"]}`, + expectedConfig: `{"type":"openstack","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), }, { @@ -200,14 +200,14 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: awsPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":3,"MultiMaster":false,"MultiZone":true,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["OVNKubernetes"]}`, + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":3,"MultiMaster":false,"MultiZone":true,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OVNKubernetes"}`, runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), }, { name: "complex override without discovery", provider: `{"type":"aws","region":"us-east-2","zone":"us-east-2a","multimaster":false,"multizone":true}`, discoveredPlatform: nil, - expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":0,"MultiMaster":false,"MultiZone":true,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":null}`, + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":0,"MultiMaster":false,"MultiZone":true,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPlugin":""}`, runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), }, { @@ -216,16 +216,16 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":true,"NetworkPluginIDs":["OVNKubernetes"]}`, + expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":true,"NetworkPlugin":"OVNKubernetes"}`, runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-sdn", "not-multitenant"), }, { name: "override network plugin", - provider: `{"type":"aws","networkPluginIDs":["Calico"]}`, + provider: `{"type":"aws","networkPlugin":"Calico"}`, discoveredPlatform: awsPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPluginIDs":["Calico"]}`, + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"Calico"}`, runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), }, } diff --git a/test/extended/util/cluster/cluster.go b/test/extended/util/cluster/cluster.go index c9951474f21d..5cb445fb5fe1 100644 --- a/test/extended/util/cluster/cluster.go +++ b/test/extended/util/cluster/cluster.go @@ -34,9 +34,14 @@ type ClusterConfiguration struct { Zones []string ConfigFile string - // Network-related configurations - Disconnected bool - NetworkPluginIDs []string + // Disconnected is set for test jobs without external internet connectivity + Disconnected bool + + // NetworkPlugin is the "official" plugin name + NetworkPlugin string + // NetworkPluginMode is an optional sub-identifier for the NetworkPlugin. + // (Currently it is only used for OpenShiftSDN.) + NetworkPluginMode string `json:",omitempty"` } func (c *ClusterConfiguration) ToJSONString() string { @@ -107,12 +112,6 @@ func DiscoverClusterState(clientConfig *rest.Config) (*ClusterState, error) { // LoadConfig generates a ClusterConfiguration based on a detected or hard-coded ClusterState func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { - var networkPluginIDs []string - networkPluginIDs = append(networkPluginIDs, string(state.NetworkSpec.DefaultNetwork.Type)) - if state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig != nil && state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode != "" { - networkPluginIDs = append(networkPluginIDs, string(state.NetworkSpec.DefaultNetwork.Type)+"/"+string(state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode)) - } - zones := sets.NewString() for _, node := range state.Masters.Items { zones.Insert(node.Labels["failure-domain.beta.kubernetes.io/zone"]) @@ -120,10 +119,9 @@ func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { zones.Delete("") config := &ClusterConfiguration{ - MultiMaster: len(state.Masters.Items) > 1, - MultiZone: zones.Len() > 1, - Zones: zones.List(), - NetworkPluginIDs: networkPluginIDs, + MultiMaster: len(state.Masters.Items) > 1, + MultiZone: zones.Len() > 1, + Zones: zones.List(), } if zones.Len() > 0 { config.Zone = zones.List()[0] @@ -162,6 +160,11 @@ func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { config.ConfigFile = tmpFile.Name() } + config.NetworkPlugin = string(state.NetworkSpec.DefaultNetwork.Type) + if state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig != nil && state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode != "" { + config.NetworkPluginMode = string(state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode) + } + return config, nil } @@ -170,8 +173,11 @@ func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { func (c *ClusterConfiguration) MatchFn() func(string) bool { var skips []string skips = append(skips, fmt.Sprintf("[Skipped:%s]", c.ProviderName)) - for _, id := range c.NetworkPluginIDs { - skips = append(skips, fmt.Sprintf("[Skipped:Network/%s]", id)) + if c.NetworkPlugin != "" { + skips = append(skips, fmt.Sprintf("[Skipped:Network/%s]", c.NetworkPlugin)) + if c.NetworkPluginMode != "" { + skips = append(skips, fmt.Sprintf("[Skipped:Network/%s/%s]", c.NetworkPlugin, c.NetworkPluginMode)) + } } if c.Disconnected { skips = append(skips, "[Skipped:Disconnected]") From 6d7e8dcc59b21bc58138a6730eadb41851fda1b7 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 20 Jan 2021 09:59:23 -0500 Subject: [PATCH 7/7] Auto-skip IPv6-only/dual-stack-only/SCTP tests based on network config (For now we can't actually autodetect whether SCTP is available, but a test job could set HasSCTP via JSON --provider.) --- cmd/openshift-tests/provider_test.go | 48 ++++++++++++++++----------- go.mod | 1 + test/extended/util/cluster/cluster.go | 40 ++++++++++++++++++++++ vendor/modules.txt | 1 + 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/cmd/openshift-tests/provider_test.go b/cmd/openshift-tests/provider_test.go index 9d452ca9a04f..2c558449f26e 100644 --- a/cmd/openshift-tests/provider_test.go +++ b/cmd/openshift-tests/provider_test.go @@ -110,6 +110,7 @@ var sdnConfig = &operatorv1.NetworkSpec{ Type: operatorv1.NetworkTypeOpenShiftSDN, OpenShiftSDNConfig: &operatorv1.OpenShiftSDNConfig{}, }, + ServiceNetwork: []string{"172.30.0.0/16"}, } var multitenantConfig = &operatorv1.NetworkSpec{ @@ -119,12 +120,17 @@ var multitenantConfig = &operatorv1.NetworkSpec{ Mode: operatorv1.SDNModeMultitenant, }, }, + ServiceNetwork: []string{"172.30.0.0/16"}, } var ovnKubernetesConfig = &operatorv1.NetworkSpec{ DefaultNetwork: operatorv1.DefaultNetworkDefinition{ Type: operatorv1.NetworkTypeOVNKubernetes, }, + ServiceNetwork: []string{ + "172.30.0.0/16", + "fd02::/112", + }, } var e2eTests = map[string]string{ @@ -134,6 +140,10 @@ var e2eTests = map[string]string{ "not-sdn": "[Skipped:Network/OpenShiftSDN]", "not-multitenant": "[Skipped:Network/OpenShiftSDN/Multitenant]", "online": "[Skipped:Disconnected]", + "ipv4": "[Feature:Networking-IPv4]", + "ipv6": "[Feature:Networking-IPv6]", + "dual-stack": "[Feature:IPv6DualStackAlpha]", + "sctp": "[Feature:SCTPConnectivity]", } func TestDecodeProvider(t *testing.T) { @@ -154,8 +164,8 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: gcePlatform, discoveredMasters: gceMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, - runTests: sets.NewString("everyone", "not-aws", "not-multitenant", "online"), + expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN","HasIPv4":true,"HasIPv6":false,"HasSCTP":false}`, + runTests: sets.NewString("everyone", "not-aws", "not-multitenant", "online", "ipv4"), }, { name: "GCE multitenant", @@ -163,8 +173,8 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: gcePlatform, discoveredMasters: gceMasters, discoveredNetwork: multitenantConfig, - expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN","NetworkPluginMode":"Multitenant"}`, - runTests: sets.NewString("everyone", "not-aws", "online"), + expectedConfig: `{"type":"gce","ProjectID":"openshift-gce-devel-ci","Region":"us-east1","Zone":"us-east1-a","NumNodes":3,"MultiMaster":true,"MultiZone":true,"Zones":["us-east1-a","us-east1-b","us-east1-c"],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN","NetworkPluginMode":"Multitenant","HasIPv4":true,"HasIPv6":false,"HasSCTP":false}`, + runTests: sets.NewString("everyone", "not-aws", "online", "ipv4"), }, { name: "simple non-cloud", @@ -172,8 +182,8 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, - runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), + expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN","HasIPv4":true,"HasIPv6":false,"HasSCTP":false}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online", "ipv4"), }, { name: "simple override", @@ -182,8 +192,8 @@ func TestDecodeProvider(t *testing.T) { discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, // NB: It does not actually use the passed-in Provider value - expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, - runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), + expectedConfig: `{"type":"skeleton","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN","HasIPv4":true,"HasIPv6":false,"HasSCTP":false}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online", "ipv4"), }, { name: "json simple override", @@ -191,23 +201,23 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: sdnConfig, - expectedConfig: `{"type":"openstack","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN"}`, - runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online"), + expectedConfig: `{"type":"openstack","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OpenShiftSDN","HasIPv4":true,"HasIPv6":false,"HasSCTP":false}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-multitenant", "online", "ipv4"), }, { - name: "complex override", + name: "complex override, dual-stack", provider: `{"type":"aws","region":"us-east-2","zone":"us-east-2a","multimaster":false,"multizone":true}`, discoveredPlatform: awsPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":3,"MultiMaster":false,"MultiZone":true,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OVNKubernetes"}`, - runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":3,"MultiMaster":false,"MultiZone":true,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"OVNKubernetes","HasIPv4":true,"HasIPv6":true,"HasSCTP":false}`, + runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online", "ipv4", "ipv6", "dual-stack"), }, { name: "complex override without discovery", provider: `{"type":"aws","region":"us-east-2","zone":"us-east-2a","multimaster":false,"multizone":true}`, discoveredPlatform: nil, - expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":0,"MultiMaster":false,"MultiZone":true,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPlugin":""}`, + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"us-east-2a","NumNodes":0,"MultiMaster":false,"MultiZone":true,"Zones":null,"ConfigFile":"","Disconnected":false,"NetworkPlugin":"","HasIPv4":false,"HasIPv6":false,"HasSCTP":false}`, runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), }, { @@ -216,17 +226,17 @@ func TestDecodeProvider(t *testing.T) { discoveredPlatform: noPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":true,"NetworkPlugin":"OVNKubernetes"}`, - runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-sdn", "not-multitenant"), + expectedConfig: `{"type":"none","ProjectID":"","Region":"","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":true,"NetworkPlugin":"OVNKubernetes","HasIPv4":true,"HasIPv6":true,"HasSCTP":false}`, + runTests: sets.NewString("everyone", "not-gce", "not-aws", "not-sdn", "not-multitenant", "ipv4", "ipv6", "dual-stack"), }, { name: "override network plugin", - provider: `{"type":"aws","networkPlugin":"Calico"}`, + provider: `{"type":"aws","networkPlugin":"Calico","hasIPv4":false,"hasIPv6":true,"hasSCTP":true}`, discoveredPlatform: awsPlatform, discoveredMasters: simpleMasters, discoveredNetwork: ovnKubernetesConfig, - expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"Calico"}`, - runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online"), + expectedConfig: `{"type":"aws","ProjectID":"","Region":"us-east-2","Zone":"","NumNodes":3,"MultiMaster":true,"MultiZone":false,"Zones":[],"ConfigFile":"","Disconnected":false,"NetworkPlugin":"Calico","HasIPv4":false,"HasIPv6":true,"HasSCTP":true}`, + runTests: sets.NewString("everyone", "not-gce", "not-sdn", "not-multitenant", "online", "ipv6", "sctp"), }, } diff --git a/go.mod b/go.mod index 0c28bc3b882d..b6e6ef58f327 100644 --- a/go.mod +++ b/go.mod @@ -70,6 +70,7 @@ require ( k8s.io/kubelet v0.20.0 k8s.io/kubernetes v1.20.0 k8s.io/legacy-cloud-providers v0.20.0 + k8s.io/utils v0.0.0-20201110183641-67b214c5f920 sigs.k8s.io/yaml v1.2.0 ) diff --git a/test/extended/util/cluster/cluster.go b/test/extended/util/cluster/cluster.go index 5cb445fb5fe1..b4459b86b1e1 100644 --- a/test/extended/util/cluster/cluster.go +++ b/test/extended/util/cluster/cluster.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + utilnet "k8s.io/utils/net" configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" @@ -42,6 +43,14 @@ type ClusterConfiguration struct { // NetworkPluginMode is an optional sub-identifier for the NetworkPlugin. // (Currently it is only used for OpenShiftSDN.) NetworkPluginMode string `json:",omitempty"` + + // HasIPv4 and HasIPv6 determine whether IPv4-specific, IPv6-specific, + // and dual-stack-specific tests are run + HasIPv4 bool + HasIPv6 bool + + // HasSCTP determines whether SCTP connectivity tests can be run in the cluster + HasSCTP bool } func (c *ClusterConfiguration) ToJSONString() string { @@ -165,6 +174,19 @@ func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { config.NetworkPluginMode = string(state.NetworkSpec.DefaultNetwork.OpenShiftSDNConfig.Mode) } + // Determine IP configuration + for _, cidr := range state.NetworkSpec.ServiceNetwork { + if utilnet.IsIPv6CIDRString(cidr) { + config.HasIPv6 = true + } else { + config.HasIPv4 = true + } + } + + // FIXME: detect SCTP availability; there's no explicit config for it, so we'd + // have to scan MachineConfig objects to figure this out? For now, callers can + // can just manually override with --provider... + return config, nil } @@ -173,15 +195,33 @@ func LoadConfig(state *ClusterState) (*ClusterConfiguration, error) { func (c *ClusterConfiguration) MatchFn() func(string) bool { var skips []string skips = append(skips, fmt.Sprintf("[Skipped:%s]", c.ProviderName)) + if c.NetworkPlugin != "" { skips = append(skips, fmt.Sprintf("[Skipped:Network/%s]", c.NetworkPlugin)) if c.NetworkPluginMode != "" { skips = append(skips, fmt.Sprintf("[Skipped:Network/%s/%s]", c.NetworkPlugin, c.NetworkPluginMode)) } } + if c.Disconnected { skips = append(skips, "[Skipped:Disconnected]") } + + if !c.HasIPv4 { + skips = append(skips, "[Feature:Networking-IPv4]") + } + if !c.HasIPv6 { + skips = append(skips, "[Feature:Networking-IPv6]") + } + if !c.HasIPv4 || !c.HasIPv6 { + // lack of "]" is intentional; this matches multiple tags + skips = append(skips, "[Feature:IPv6DualStack") + } + + if !c.HasSCTP { + skips = append(skips, "[Feature:SCTPConnectivity]") + } + matchFn := func(name string) bool { for _, skip := range skips { if strings.Contains(name, skip) { diff --git a/vendor/modules.txt b/vendor/modules.txt index b9175ee6106b..d0cfeb95e406 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2870,6 +2870,7 @@ k8s.io/mount-utils k8s.io/sample-apiserver/pkg/apis/wardle k8s.io/sample-apiserver/pkg/apis/wardle/v1alpha1 # k8s.io/utils v0.0.0-20201110183641-67b214c5f920 +## explicit k8s.io/utils/buffer k8s.io/utils/clock k8s.io/utils/exec