From d348499f2d3bc0d951bf8d299e12bb9f5d053fee Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 7 Feb 2019 23:52:21 -0800 Subject: [PATCH] pkg/asset/machines/aws: Cache zone responses To protect us from allocating masters based on one zone request and workers based on a different zone request. Once we get the zone information from AWS, using the same response in both locations is more efficient and guards us against divergent responses. If a zone goes off- or online while we're working through asset generation, we'll get a more predicatable topology if we don't alter our zone pool along the way. If the change is a zone going offline, the cluster is probably going to fail. But it would have failed anyway. If the change is a zone coming online, this commit ensures we don't strand some workers (or masters, depending on which asset was rendered last) off in the new zone by themselves. Having stranded workers or masters would probably work, but might surprise customers who asked for, say, three of each type of machine. I prefer having the cached value live in an asset, because users could configure their desired zones by setting that asset to avoid the need for an AWS API request entirely. But Matthew points out that an asset increases the chances of the stored data going stale between installer invocations [1], so with this commit we're just caching in memory. [1]: https://github.com/openshift/installer/pull/1212#pullrequestreview-204315822 --- pkg/asset/machines/aws/zones.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/asset/machines/aws/zones.go b/pkg/asset/machines/aws/zones.go index 50788f115f6..24b080e5943 100644 --- a/pkg/asset/machines/aws/zones.go +++ b/pkg/asset/machines/aws/zones.go @@ -8,8 +8,16 @@ import ( awsutil "github.com/openshift/installer/pkg/asset/installconfig/aws" ) +var cache map[string][]string + // AvailabilityZones retrieves a list of availability zones for the given region. func AvailabilityZones(region string) ([]string, error) { + if cache == nil { + cache = map[string][]string{} + } else if zones, ok := cache[region]; ok { + return zones, nil + } + ec2Client, err := ec2Client(region) if err != nil { return nil, err @@ -18,6 +26,7 @@ func AvailabilityZones(region string) ([]string, error) { if err != nil { return nil, fmt.Errorf("cannot fetch availability zones: %v", err) } + cache[region] = zones return zones, nil }