From 9830cc911fff55093090acbfa64578db57c73d66 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 4 Feb 2021 20:48:45 -0500 Subject: [PATCH] test/e2e: Allow zones to be passed to e2e cloud provider Instead of allowing the cloud provider to guess at the zones that should be applied for a cluster under test, allow the explicit list of zones to consider to be passed as a new test context flag -gce-zones. Only the GCE test cloud provider recognizes this value because only the GCE test cloud provider makes assumptions about zones for verifying values, and the default assumptions for GKE do not always match non-GKE providers. --- .../src/k8s.io/component-base/cli/flag/BUILD | 2 + .../cli/flag/string_slice_flag.go | 55 +++++++++++ .../cli/flag/string_slice_flag_test.go | 93 +++++++++++++++++++ test/e2e/framework/providers/gce/gce.go | 24 ++++- test/e2e/framework/test_context.go | 4 +- 5 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 staging/src/k8s.io/component-base/cli/flag/string_slice_flag.go create mode 100644 staging/src/k8s.io/component-base/cli/flag/string_slice_flag_test.go diff --git a/staging/src/k8s.io/component-base/cli/flag/BUILD b/staging/src/k8s.io/component-base/cli/flag/BUILD index 6efdc3379d9..9c52db98989 100644 --- a/staging/src/k8s.io/component-base/cli/flag/BUILD +++ b/staging/src/k8s.io/component-base/cli/flag/BUILD @@ -15,6 +15,7 @@ go_test( "map_string_bool_test.go", "map_string_string_test.go", "namedcertkey_flag_test.go", + "string_slice_flag_test.go", ], embed = [":go_default_library"], deps = ["//vendor/github.com/spf13/pflag:go_default_library"], @@ -36,6 +37,7 @@ go_library( "omitempty.go", "sectioned.go", "string_flag.go", + "string_slice_flag.go", "tristate.go", ], importmap = "k8s.io/kubernetes/vendor/k8s.io/component-base/cli/flag", diff --git a/staging/src/k8s.io/component-base/cli/flag/string_slice_flag.go b/staging/src/k8s.io/component-base/cli/flag/string_slice_flag.go new file mode 100644 index 00000000000..7b2e0f61695 --- /dev/null +++ b/staging/src/k8s.io/component-base/cli/flag/string_slice_flag.go @@ -0,0 +1,55 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flag + +import ( + goflag "flag" + "strings" + + "github.com/spf13/pflag" +) + +// StringSlice implements goflag.Value and plfag.Value, +// and allows set to be invoked repeatedly to accumulate values. +type StringSlice struct { + value *[]string + changed bool +} + +func NewStringSlice(s *[]string) *StringSlice { + return &StringSlice{value: s} +} + +var _ goflag.Value = &StringSlice{} +var _ pflag.Value = &StringSlice{} + +func (s *StringSlice) String() string { + return strings.Join(*s.value, " ") +} + +func (s *StringSlice) Set(val string) error { + if s.value == nil || !s.changed { + *s.value = make([]string, 0) + } + *s.value = append(*s.value, val) + s.changed = true + return nil +} + +func (StringSlice) Type() string { + return "sliceString" +} diff --git a/staging/src/k8s.io/component-base/cli/flag/string_slice_flag_test.go b/staging/src/k8s.io/component-base/cli/flag/string_slice_flag_test.go new file mode 100644 index 00000000000..6e5ef240679 --- /dev/null +++ b/staging/src/k8s.io/component-base/cli/flag/string_slice_flag_test.go @@ -0,0 +1,93 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flag + +import ( + "fmt" + "reflect" + "strings" + "testing" + + "github.com/spf13/pflag" +) + +func TestStringSlice(t *testing.T) { + tests := []struct { + args []string + def []string + expected []string + parseError string + changed bool + }{ + { + args: []string{}, + expected: nil, + }, + { + args: []string{"a"}, + expected: []string{"a"}, + changed: true, + }, + { + args: []string{"a", "b"}, + expected: []string{"a", "b"}, + changed: true, + }, + { + def: []string{"a"}, + args: []string{"a", "b"}, + expected: []string{"a", "b"}, + changed: true, + }, + { + def: []string{"a", "b"}, + args: []string{"a", "b"}, + expected: []string{"a", "b"}, + changed: true, + }, + } + for i, test := range tests { + fs := pflag.NewFlagSet("testStringSlice", pflag.ContinueOnError) + var s []string + s = append(s, test.def...) + + v := NewStringSlice(&s) + fs.Var(v, "slice", "usage") + + args := []string{} + for _, a := range test.args { + args = append(args, fmt.Sprintf("--slice=%s", a)) + } + + err := fs.Parse(args) + if test.parseError != "" { + if err == nil { + t.Errorf("%d: expected error %q, got nil", i, test.parseError) + } else if !strings.Contains(err.Error(), test.parseError) { + t.Errorf("%d: expected error %q, got %q", i, test.parseError, err) + } + } else if err != nil { + t.Errorf("%d: expected nil error, got %v", i, err) + } + if !reflect.DeepEqual(s, test.expected) { + t.Errorf("%d: expected %+v, got %+v", i, test.expected, s) + } + if v.changed != test.changed { + t.Errorf("%d: expected %t got %t", i, test.changed, v.changed) + } + } +} diff --git a/test/e2e/framework/providers/gce/gce.go b/test/e2e/framework/providers/gce/gce.go index 271a70760a9..05b0b003c83 100644 --- a/test/e2e/framework/providers/gce/gce.go +++ b/test/e2e/framework/providers/gce/gce.go @@ -19,6 +19,7 @@ package gce import ( "context" "fmt" + "math/rand" "net/http" "os/exec" "regexp" @@ -47,6 +48,21 @@ func factory() (framework.ProviderInterface, error) { framework.Logf("Fetching cloud provider for %q\r", framework.TestContext.Provider) zone := framework.TestContext.CloudConfig.Zone region := framework.TestContext.CloudConfig.Region + allowedZones := framework.TestContext.CloudConfig.Zones + + // ensure users don't specify a zone outside of the requested zones + if len(zone) > 0 && len(allowedZones) > 0 { + var found bool + for _, allowedZone := range allowedZones { + if zone == allowedZone { + found = true + break + } + } + if !found { + return nil, fmt.Errorf("the provided zone %q must be included in the list of allowed zones %v", zone, allowedZones) + } + } var err error if region == "" { @@ -59,6 +75,9 @@ func factory() (framework.ProviderInterface, error) { if !framework.TestContext.CloudConfig.MultiZone { managedZones = []string{zone} } + if len(allowedZones) > 0 { + managedZones = allowedZones + } gceCloud, err := gcecloud.CreateGCECloud(&gcecloud.CloudConfig{ APIEndpoint: framework.TestContext.CloudConfig.APIEndpoint, @@ -79,7 +98,10 @@ func factory() (framework.ProviderInterface, error) { return nil, fmt.Errorf("Error building GCE/GKE provider: %v", err) } - // Arbitrarily pick one of the zones we have nodes in + // Arbitrarily pick one of the zones we have nodes in, looking at prepopulated zones first. + if framework.TestContext.CloudConfig.Zone == "" && len(managedZones) > 0 { + framework.TestContext.CloudConfig.Zone = managedZones[rand.Intn(len(managedZones))] + } if framework.TestContext.CloudConfig.Zone == "" && framework.TestContext.CloudConfig.MultiZone { zones, err := gceCloud.GetAllZonesFromCloudProvider() if err != nil { diff --git a/test/e2e/framework/test_context.go b/test/e2e/framework/test_context.go index b23942da406..d939a86e6a9 100644 --- a/test/e2e/framework/test_context.go +++ b/test/e2e/framework/test_context.go @@ -229,7 +229,8 @@ type NodeTestContextType struct { type CloudConfig struct { APIEndpoint string ProjectID string - Zone string // for multizone tests, arbitrarily chosen zone + Zone string // for multizone tests, arbitrarily chosen zone + Zones []string // for multizone tests, use this set of zones instead of querying the cloud provider. Must include Zone. Region string MultiZone bool MultiMaster bool @@ -339,6 +340,7 @@ func RegisterClusterFlags(flags *flag.FlagSet) { flags.StringVar(&cloudConfig.APIEndpoint, "gce-api-endpoint", "", "The GCE APIEndpoint being used, if applicable") flags.StringVar(&cloudConfig.ProjectID, "gce-project", "", "The GCE project being used, if applicable") flags.StringVar(&cloudConfig.Zone, "gce-zone", "", "GCE zone being used, if applicable") + flags.Var(cliflag.NewStringSlice(&cloudConfig.Zones), "gce-zones", "The set of zones to use in a multi-zone test instead of querying the cloud provider.") flags.StringVar(&cloudConfig.Region, "gce-region", "", "GCE region being used, if applicable") flags.BoolVar(&cloudConfig.MultiZone, "gce-multizone", false, "If true, start GCE cloud provider with multizone support.") flags.BoolVar(&cloudConfig.MultiMaster, "gce-multimaster", false, "If true, the underlying GCE/GKE cluster is assumed to be multi-master.")