From 8b3099ced7ed84f90f6ac04ae38eb33e07205853 Mon Sep 17 00:00:00 2001 From: Walter Fender Date: Mon, 1 Oct 2018 17:10:00 -0700 Subject: [PATCH] Differentiate multizone zonal from Regional Cluster. Fixed go format and unit test. Collapse lines. Switched to using regional throughout and added warning for HA Zonal. --- cluster/gce/gci/configure-helper.sh | 9 ++++++ pkg/cloudprovider/providers/gce/gce.go | 11 ++++++- .../providers/gce/gce_clusters.go | 29 +++++++++++-------- pkg/cloudprovider/providers/gce/gce_test.go | 17 +++++++++++ 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/cluster/gce/gci/configure-helper.sh b/cluster/gce/gci/configure-helper.sh index 739dee2446..5810eee8f0 100644 --- a/cluster/gce/gci/configure-helper.sh +++ b/cluster/gce/gci/configure-helper.sh @@ -661,6 +661,15 @@ EOF use_cloud_config="true" cat <>/etc/gce.conf multizone = ${MULTIZONE} +EOF + fi +# Multimaster indicates that the cluster is HA. +# Currently the only HA clusters are regional. +# If we introduce zonal multimaster this will need to be revisited. + if [[ -n "${MULTIMASTER:-}" ]]; then + use_cloud_config="true" + cat <>/etc/gce.conf +regional = ${MULTIMASTER} EOF fi if [[ -n "${GCE_ALPHA_FEATURES:-}" ]]; then diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index de9d784e8c..aac5468a96 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -114,6 +114,7 @@ type GCECloud struct { eventRecorder record.EventRecorder projectID string region string + regional bool localZone string // The zone in which we are running // managedZones will be set to the 1 zone if running a single zone cluster // it will be set to ALL zones in region for any multi-zone cluster @@ -174,6 +175,7 @@ type ConfigGlobal struct { SecondaryRangeName string `gcfg:"secondary-range-name"` NodeTags []string `gcfg:"node-tags"` NodeInstancePrefix string `gcfg:"node-instance-prefix"` + Regional bool `gcfg:"regional"` Multizone bool `gcfg:"multizone"` // ApiEndpoint is the GCE compute API endpoint to use. If this is blank, // then the default endpoint is used. @@ -202,6 +204,7 @@ type CloudConfig struct { ProjectID string NetworkProjectID string Region string + Regional bool Zone string ManagedZones []string NetworkName string @@ -332,9 +335,14 @@ func generateCloudConfig(configFile *ConfigFile) (cloudConfig *CloudConfig, err return nil, err } + // Determine if its a regional cluster + if configFile != nil && configFile.Global.Regional { + cloudConfig.Regional = true + } + // generate managedZones cloudConfig.ManagedZones = []string{cloudConfig.Zone} - if configFile != nil && configFile.Global.Multizone { + if configFile != nil && (configFile.Global.Multizone || configFile.Global.Regional) { cloudConfig.ManagedZones = nil // Use all zones in region } @@ -512,6 +520,7 @@ func CreateGCECloud(config *CloudConfig) (*GCECloud, error) { networkProjectID: netProjID, onXPN: onXPN, region: config.Region, + regional: config.Regional, localZone: config.Zone, managedZones: config.ManagedZones, networkURL: networkURL, diff --git a/pkg/cloudprovider/providers/gce/gce_clusters.go b/pkg/cloudprovider/providers/gce/gce_clusters.go index cdc685c58e..63b4bbeb69 100644 --- a/pkg/cloudprovider/providers/gce/gce_clusters.go +++ b/pkg/cloudprovider/providers/gce/gce_clusters.go @@ -45,22 +45,27 @@ func (gce *GCECloud) ListClusters(ctx context.Context) ([]string, error) { } func (gce *GCECloud) GetManagedClusters(ctx context.Context) ([]*container.Cluster, error) { - var location string - if len(gce.managedZones) > 1 { - // Multiple managed zones means this is a regional cluster - // so use the regional location and not the zone. - location = gce.region - } else if len(gce.managedZones) == 1 { - location = gce.managedZones[0] + managedClusters := []*container.Cluster{} + + if gce.regional { + var err error + managedClusters, err = gce.getClustersInLocation(gce.region) + if err != nil { + return nil, err + } + } else if len(gce.managedZones) >= 1 { + for _, zone := range gce.managedZones { + clusters, err := gce.getClustersInLocation(zone) + if err != nil { + return nil, err + } + managedClusters = append(managedClusters, clusters...) + } } else { return nil, errors.New(fmt.Sprintf("no zones associated with this cluster(%s)", gce.ProjectID())) } - clusters, err := gce.getClustersInLocation(location) - if err != nil { - return nil, err - } - return clusters, nil + return managedClusters, nil } func (gce *GCECloud) Master(ctx context.Context, clusterName string) (string, error) { diff --git a/pkg/cloudprovider/providers/gce/gce_test.go b/pkg/cloudprovider/providers/gce/gce_test.go index 870e39acfc..33b9e35f73 100644 --- a/pkg/cloudprovider/providers/gce/gce_test.go +++ b/pkg/cloudprovider/providers/gce/gce_test.go @@ -39,6 +39,7 @@ secondary-range-name = my-secondary-range node-tags = my-node-tag1 node-instance-prefix = my-prefix multizone = true +regional = true ` reader := strings.NewReader(s) config, err := readConfig(reader) @@ -57,6 +58,7 @@ multizone = true NodeTags: []string{"my-node-tag1"}, NodeInstancePrefix: "my-prefix", Multizone: true, + Regional: true, }} if !reflect.DeepEqual(expected, config) { @@ -328,6 +330,7 @@ func TestGenerateCloudConfigs(t *testing.T) { NodeTags: []string{"node-tag"}, NodeInstancePrefix: "node-prefix", Multizone: false, + Regional: false, ApiEndpoint: "", LocalZone: "us-central1-a", AlphaFeatures: []string{}, @@ -446,6 +449,20 @@ func TestGenerateCloudConfigs(t *testing.T) { return v }, }, + { + name: "Regional", + config: func() ConfigGlobal { + v := configBoilerplate + v.Regional = true + return v + }, + cloud: func() CloudConfig { + v := cloudBoilerplate + v.Regional = true + v.ManagedZones = nil + return v + }, + }, { name: "Secondary Range Name", config: func() ConfigGlobal {