Skip to content

Commit

Permalink
UPSTREAM: 98787: Allow zones to be passed to e2e cloud provider
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
smarterclayton authored and openshift-cherrypick-robot committed Feb 8, 2021
1 parent e405995 commit 5ee59a1
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 2 deletions.
2 changes: 2 additions & 0 deletions staging/src/k8s.io/component-base/cli/flag/BUILD
Expand Up @@ -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"],
Expand All @@ -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",
Expand Down
55 changes: 55 additions & 0 deletions 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"
}
@@ -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)
}
}
}
24 changes: 23 additions & 1 deletion test/e2e/framework/providers/gce/gce.go
Expand Up @@ -19,6 +19,7 @@ package gce
import (
"context"
"fmt"
"math/rand"
"net/http"
"os/exec"
"regexp"
Expand Down Expand Up @@ -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 == "" {
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/framework/test_context.go
Expand Up @@ -224,7 +224,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
Expand Down Expand Up @@ -334,6 +335,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.")
Expand Down

0 comments on commit 5ee59a1

Please sign in to comment.