From 61b9c450c61122cbee6686496b6cf46dce8a22d9 Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Tue, 20 Feb 2024 17:55:39 +0000 Subject: [PATCH 01/14] made changes to support MIGs that use regional instance templates --- cherry_pick_pull.sh | 244 ++++++++++++++++++ .../gce/autoscaling_gce_client.go | 25 +- cluster-autoscaler/cloudprovider/gce/cache.go | 67 ++--- .../cloudprovider/gce/mig_info_provider.go | 32 +-- 4 files changed, 315 insertions(+), 53 deletions(-) create mode 100755 cherry_pick_pull.sh diff --git a/cherry_pick_pull.sh b/cherry_pick_pull.sh new file mode 100755 index 000000000000..92585bedfdf6 --- /dev/null +++ b/cherry_pick_pull.sh @@ -0,0 +1,244 @@ +#!/usr/bin/env bash + +# Copyright 2015 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. + +# Usage Instructions: https://karmada.io/docs/contributor/cherry-picks + +# Checkout a PR from GitHub. (Yes, this is sitting in a Git tree. How +# meta.) Assumes you care about pulls from remote "upstream" and +# checks them out to a branch named: +# automated-cherry-pick-of--- + +set -o errexit +set -o nounset +set -o pipefail + +REPO_ROOT="$(git rev-parse --show-toplevel)" +declare -r REPO_ROOT +cd "${REPO_ROOT}" + +STARTINGBRANCH=$(git symbolic-ref --short HEAD) +declare -r STARTINGBRANCH +declare -r REBASEMAGIC="${REPO_ROOT}/.git/rebase-apply" +DRY_RUN=${DRY_RUN:-""} +REGENERATE_DOCS=${REGENERATE_DOCS:-""} +UPSTREAM_REMOTE=${UPSTREAM_REMOTE:-upstream} +FORK_REMOTE=${FORK_REMOTE:-origin} +MAIN_REPO_ORG=${MAIN_REPO_ORG:-$(git remote get-url "$UPSTREAM_REMOTE" | awk '{gsub(/http[s]:\/\/|git@/,"")}1' | awk -F'[@:./]' 'NR==1{print $3}')} +MAIN_REPO_NAME=${MAIN_REPO_NAME:-$(git remote get-url "$UPSTREAM_REMOTE" | awk '{gsub(/http[s]:\/\/|git@/,"")}1' | awk -F'[@:./]' 'NR==1{print $4}')} + +if [[ -z ${GITHUB_USER:-} ]]; then + echo "Please export GITHUB_USER= (or GH organization, if that's where your fork lives)" + exit 1 +fi + +if ! command -v gh >/dev/null; then + echo "Can't find 'gh' tool in PATH, please install from https://github.com/cli/cli" + exit 1 +fi + +if [[ "$#" -lt 2 ]]; then + echo "${0} ...: cherry pick one or more onto and leave instructions for proposing pull request" + echo + echo " Checks out and handles the cherry-pick of (possibly multiple) for you." + echo " Examples:" + echo " $0 upstream/release-3.14 12345 # Cherry-picks PR 12345 onto upstream/release-3.14 and proposes that as a PR." + echo " $0 upstream/release-3.14 12345 56789 # Cherry-picks PR 12345, then 56789 and proposes the combination as a single PR." + echo + echo " Set the DRY_RUN environment var to skip git push and creating PR." + echo " This is useful for creating patches to a release branch without making a PR." + echo " When DRY_RUN is set the script will leave you in a branch containing the commits you cherry-picked." + echo + echo " Set UPSTREAM_REMOTE (default: upstream) and FORK_REMOTE (default: origin)" + echo " to override the default remote names to what you have locally." + echo + echo " For merge process info, see https://karmada.io/docs/contributor/cherry-picks" + exit 2 +fi + +# Checks if you are logged in. Will error/bail if you are not. +gh auth status + +if git_status=$(git status --porcelain --untracked=no 2>/dev/null) && [[ -n "${git_status}" ]]; then + echo "!!! Dirty tree. Clean up and try again." + exit 1 +fi + +if [[ -e "${REBASEMAGIC}" ]]; then + echo "!!! 'git rebase' or 'git am' in progress. Clean up and try again." + exit 1 +fi + +declare -r BRANCH="$1" +shift 1 +declare -r PULLS=("$@") + +function join { + local IFS="$1" + shift + echo "$*" +} +PULLDASH=$(join - "${PULLS[@]/#/#}") # Generates something like "#12345-#56789" +declare -r PULLDASH +PULLSUBJ=$(join " " "${PULLS[@]/#/#}") # Generates something like "#12345 #56789" +declare -r PULLSUBJ + +echo "+++ Updating remotes..." +git remote update "${UPSTREAM_REMOTE}" "${FORK_REMOTE}" + +if ! git log -n1 --format=%H "${BRANCH}" >/dev/null 2>&1; then + echo "!!! '${BRANCH}' not found. The second argument should be something like ${UPSTREAM_REMOTE}/release-1.0." + echo " (In particular, it needs to be a valid, existing remote branch that I can 'git checkout'.)" + exit 1 +fi + +NEWBRANCHREQ="automated-cherry-pick-of-${PULLDASH}" # "Required" portion for tools. +declare -r NEWBRANCHREQ +NEWBRANCH="$(echo "${NEWBRANCHREQ}-${BRANCH}" | sed 's/\//-/g')" +declare -r NEWBRANCH +NEWBRANCHUNIQ="${NEWBRANCH}-$(date +%s)" +declare -r NEWBRANCHUNIQ +echo "+++ Creating local branch ${NEWBRANCHUNIQ}" + +cleanbranch="" +gitamcleanup=false +function return_to_kansas { + if [[ "${gitamcleanup}" == "true" ]]; then + echo + echo "+++ Aborting in-progress git am." + git am --abort >/dev/null 2>&1 || true + fi + + # return to the starting branch and delete the PR text file + if [[ -z "${DRY_RUN}" ]]; then + echo + echo "+++ Returning you to the ${STARTINGBRANCH} branch and cleaning up." + git checkout -f "${STARTINGBRANCH}" >/dev/null 2>&1 || true + if [[ -n "${cleanbranch}" ]]; then + git branch -D "${cleanbranch}" >/dev/null 2>&1 || true + fi + fi +} +trap return_to_kansas EXIT + +SUBJECTS=() +function make-a-pr() { + local rel + rel="$(basename "${BRANCH}")" + echo + echo "+++ Creating a pull request on GitHub at ${GITHUB_USER}:${NEWBRANCH}" + + local numandtitle + numandtitle=$(printf '%s\n' "${SUBJECTS[@]}") + prtext=$( + cat <&2 + exit 1 + fi + done + + if [[ "${conflicts}" != "true" ]]; then + echo "!!! git am failed, likely because of an in-progress 'git am' or 'git rebase'" + exit 1 + fi + } + + # set the subject + subject=$(grep -m 1 "^Subject" "/tmp/${pull}.patch" | sed -e 's/Subject: \[PATCH//g' | sed 's/.*] //') + SUBJECTS+=("#${pull}: ${subject}") + + # remove the patch file from /tmp + rm -f "/tmp/${pull}.patch" +done +gitamcleanup=false + +if [[ -n "${DRY_RUN}" ]]; then + echo "!!! Skipping git push and PR creation because you set DRY_RUN." + echo "To return to the branch you were in when you invoked this script:" + echo + echo " git checkout ${STARTINGBRANCH}" + echo + echo "To delete this branch:" + echo + echo " git branch -D ${NEWBRANCHUNIQ}" + exit 0 +fi + +if git remote -v | grep ^"${FORK_REMOTE}" | grep "${MAIN_REPO_ORG}/${MAIN_REPO_NAME}.git"; then + echo "!!! You have ${FORK_REMOTE} configured as your ${MAIN_REPO_ORG}/${MAIN_REPO_NAME}.git" + echo "This isn't normal. Leaving you with push instructions:" + echo + echo "+++ First manually push the branch this script created:" + echo + echo " git push REMOTE ${NEWBRANCHUNIQ}:${NEWBRANCH}" + echo + echo "where REMOTE is your personal fork (maybe ${UPSTREAM_REMOTE}? Consider swapping those.)." + echo "OR consider setting UPSTREAM_REMOTE and FORK_REMOTE to different values." + echo + make-a-pr + cleanbranch="" + exit 0 +fi + +echo +echo "+++ I'm about to do the following to push to GitHub (and I'm assuming ${FORK_REMOTE} is your personal fork):" +echo +echo " git push ${FORK_REMOTE} ${NEWBRANCHUNIQ}:${NEWBRANCH}" +echo +read -p "+++ Proceed (anything but 'y' aborts the cherry-pick)? [y/n] " -r +if ! [[ "${REPLY}" =~ ^[yY]$ ]]; then + echo "Aborting." >&2 + exit 1 +fi + +git push "${FORK_REMOTE}" -f "${NEWBRANCHUNIQ}:${NEWBRANCH}" +make-a-pr \ No newline at end of file diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go index 39c5364e7b61..505566796cda 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go @@ -96,9 +96,9 @@ type AutoscalingGceClient interface { FetchAllMigs(zone string) ([]*gce.InstanceGroupManager, error) FetchMigTargetSize(GceRef) (int64, error) FetchMigBasename(GceRef) (string, error) - FetchMigInstances(GceRef) ([]cloudprovider.Instance, error) - FetchMigTemplateName(migRef GceRef) (string, error) - FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error) + FetchMigInstances(GceRef) ([]GceInstance, error) + FetchMigTemplateName(migRef GceRef) (InstanceTemplateNameType, error) + FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) FetchMigsWithName(zone string, filter *regexp.Regexp) ([]string, error) FetchZones(region string) ([]string, error) FetchAvailableCpuPlatforms() (map[string][]string, error) @@ -550,26 +550,33 @@ func (client *autoscalingGceClientV1) FetchAvailableCpuPlatforms() (map[string][ return availableCpuPlatforms, nil } -func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (string, error) { +func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (InstanceTemplateNameType, error) { registerRequest("instance_group_managers", "get") igm, err := client.gceService.InstanceGroupManagers.Get(migRef.Project, migRef.Zone, migRef.Name).Do() if err != nil { if err, ok := err.(*googleapi.Error); ok { if err.Code == http.StatusNotFound { - return "", errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error()) + return InstanceTemplateNameType{}, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error()) } } - return "", err + return InstanceTemplateNameType{}, err } templateUrl, err := url.Parse(igm.InstanceTemplate) + regional, _ := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) if err != nil { - return "", err + return InstanceTemplateNameType{}, err } _, templateName := path.Split(templateUrl.EscapedPath()) - return templateName, nil + return InstanceTemplateNameType{templateName, regional}, nil } -func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error) { +func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) { + if regional { + zoneHyphenIndex := strings.LastIndex(migRef.Zone, "-") + region := migRef.Zone[:zoneHyphenIndex] + registerRequest("region_instance_templates", "get") + return client.gceService.RegionInstanceTemplates.Get(migRef.Project, region, templateName).Do() + } registerRequest("instance_templates", "get") return client.gceService.InstanceTemplates.Get(migRef.Project, templateName).Do() } diff --git a/cluster-autoscaler/cloudprovider/gce/cache.go b/cluster-autoscaler/cloudprovider/gce/cache.go index 818f5d7aada9..6bbedce1d7d6 100644 --- a/cluster-autoscaler/cloudprovider/gce/cache.go +++ b/cluster-autoscaler/cloudprovider/gce/cache.go @@ -33,6 +33,11 @@ type MachineTypeKey struct { MachineTypeName string } +type InstanceTemplateNameType struct { + Name string + Regional bool +} + // GceCache is used for caching cluster resources state. // // It is needed to: @@ -56,34 +61,38 @@ type GceCache struct { cacheMutex sync.Mutex // Cache content. - migs map[GceRef]Mig - instances map[GceRef][]cloudprovider.Instance - instancesUpdateTime map[GceRef]time.Time - instancesToMig map[GceRef]GceRef - instancesFromUnknownMig map[GceRef]bool - resourceLimiter *cloudprovider.ResourceLimiter - autoscalingOptionsCache map[GceRef]map[string]string - machinesCache map[MachineTypeKey]MachineType - migTargetSizeCache map[GceRef]int64 - migBaseNameCache map[GceRef]string - instanceTemplateNameCache map[GceRef]string - instanceTemplatesCache map[GceRef]*gce.InstanceTemplate + migs map[GceRef]Mig + instances map[GceRef][]GceInstance + instancesUpdateTime map[GceRef]time.Time + instancesToMig map[GceRef]GceRef + instancesFromUnknownMig map[GceRef]bool + resourceLimiter *cloudprovider.ResourceLimiter + autoscalingOptionsCache map[GceRef]map[string]string + machinesCache map[MachineTypeKey]MachineType + migTargetSizeCache map[GceRef]int64 + migBaseNameCache map[GceRef]string + listManagedInstancesResultsCache map[GceRef]string + instanceTemplateNameCache map[GceRef]InstanceTemplateNameType + instanceTemplatesCache map[GceRef]*gce.InstanceTemplate + kubeEnvCache map[GceRef]KubeEnv } // NewGceCache creates empty GceCache. func NewGceCache() *GceCache { return &GceCache{ - migs: map[GceRef]Mig{}, - instances: map[GceRef][]cloudprovider.Instance{}, - instancesUpdateTime: map[GceRef]time.Time{}, - instancesToMig: map[GceRef]GceRef{}, - instancesFromUnknownMig: map[GceRef]bool{}, - autoscalingOptionsCache: map[GceRef]map[string]string{}, - machinesCache: map[MachineTypeKey]MachineType{}, - migTargetSizeCache: map[GceRef]int64{}, - migBaseNameCache: map[GceRef]string{}, - instanceTemplateNameCache: map[GceRef]string{}, - instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, + migs: map[GceRef]Mig{}, + instances: map[GceRef][]GceInstance{}, + instancesUpdateTime: map[GceRef]time.Time{}, + instancesToMig: map[GceRef]GceRef{}, + instancesFromUnknownMig: map[GceRef]bool{}, + autoscalingOptionsCache: map[GceRef]map[string]string{}, + machinesCache: map[MachineTypeKey]MachineType{}, + migTargetSizeCache: map[GceRef]int64{}, + migBaseNameCache: map[GceRef]string{}, + listManagedInstancesResultsCache: map[GceRef]string{}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, + kubeEnvCache: map[GceRef]KubeEnv{}, } } @@ -330,23 +339,23 @@ func (gc *GceCache) InvalidateAllMigTargetSizes() { } // GetMigInstanceTemplateName returns the cached instance template ref for a mig GceRef -func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (string, bool) { +func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (InstanceTemplateNameType, bool) { gc.cacheMutex.Lock() defer gc.cacheMutex.Unlock() - templateName, found := gc.instanceTemplateNameCache[ref] + instanceTemplateNameType, found := gc.instanceTemplateNameCache[ref] if found { klog.V(5).Infof("Instance template names cache hit for %s", ref) } - return templateName, found + return instanceTemplateNameType, found } // SetMigInstanceTemplateName sets instance template ref for a mig GceRef -func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, templateName string) { +func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, instanceTemplateNameType InstanceTemplateNameType) { gc.cacheMutex.Lock() defer gc.cacheMutex.Unlock() - gc.instanceTemplateNameCache[ref] = templateName + gc.instanceTemplateNameCache[ref] = instanceTemplateNameType } // InvalidateMigInstanceTemplateName clears the instance template ref cache for a mig GceRef @@ -366,7 +375,7 @@ func (gc *GceCache) InvalidateAllMigInstanceTemplateNames() { defer gc.cacheMutex.Unlock() klog.V(5).Infof("Instance template names cache invalidated") - gc.instanceTemplateNameCache = map[GceRef]string{} + gc.instanceTemplateNameCache = map[GceRef]InstanceTemplateNameType{} } // GetMigInstanceTemplate returns the cached gce.InstanceTemplate for a mig GceRef diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go index abee9c4660ce..c635b7bd1b40 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go @@ -21,6 +21,7 @@ import ( "fmt" "net/url" "path" + "regexp" "strings" "sync" "time" @@ -44,7 +45,7 @@ type MigInfoProvider interface { // GetMigBasename returns basename for given MIG ref GetMigBasename(migRef GceRef) (string, error) // GetMigInstanceTemplateName returns instance template name for given MIG ref - GetMigInstanceTemplateName(migRef GceRef) (string, error) + GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateNameType, error) // GetMigInstanceTemplate returns instance template for given MIG ref GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error) // GetMigMachineType returns machine type used by a MIG. @@ -240,44 +241,44 @@ func (c *cachingMigInfoProvider) GetMigBasename(migRef GceRef) (string, error) { return basename, nil } -func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (string, error) { +func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateNameType, error) { c.migInfoMutex.Lock() defer c.migInfoMutex.Unlock() - templateName, found := c.cache.GetMigInstanceTemplateName(migRef) + instanceTemplateNameType, found := c.cache.GetMigInstanceTemplateName(migRef) if found { - return templateName, nil + return instanceTemplateNameType, nil } err := c.fillMigInfoCache() - templateName, found = c.cache.GetMigInstanceTemplateName(migRef) + instanceTemplateNameType, found = c.cache.GetMigInstanceTemplateName(migRef) if err == nil && found { - return templateName, nil + return instanceTemplateNameType, nil } // fallback to querying for single mig - templateName, err = c.gceClient.FetchMigTemplateName(migRef) + instanceTemplateNameType, err = c.gceClient.FetchMigTemplateName(migRef) if err != nil { c.migLister.HandleMigIssue(migRef, err) - return "", err + return InstanceTemplateNameType{}, err } - c.cache.SetMigInstanceTemplateName(migRef, templateName) - return templateName, nil + c.cache.SetMigInstanceTemplateName(migRef, instanceTemplateNameType) + return instanceTemplateNameType, nil } func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error) { - templateName, err := c.GetMigInstanceTemplateName(migRef) + instanceTemplateNameType, err := c.GetMigInstanceTemplateName(migRef) if err != nil { return nil, err } template, found := c.cache.GetMigInstanceTemplate(migRef) - if found && template.Name == templateName { + if found && template.Name == instanceTemplateNameType.Name { return template, nil } - klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, templateName) - template, err = c.gceClient.FetchMigTemplate(migRef, templateName) + klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, instanceTemplateNameType.Name) + template, err = c.gceClient.FetchMigTemplate(migRef, instanceTemplateNameType.Name, instanceTemplateNameType.Regional) if err != nil { return nil, err } @@ -336,7 +337,8 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error { templateUrl, err := url.Parse(zoneMig.InstanceTemplate) if err == nil { _, templateName := path.Split(templateUrl.EscapedPath()) - c.cache.SetMigInstanceTemplateName(zoneMigRef, templateName) + regional, _ := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) + c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateNameType{templateName, regional}) } } } From 3228749fa9986d7bb6f91211de2003e7a53076a7 Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Tue, 20 Feb 2024 22:33:08 +0000 Subject: [PATCH 02/14] modified current unit tests to support the new modifications --- .../cloudprovider/gce/gce_manager_test.go | 10 ++- .../gce/mig_info_provider_test.go | 78 ++++++++++--------- 2 files changed, 46 insertions(+), 42 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go index 609e6af91f06..341bfaea3457 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go @@ -343,10 +343,12 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa {"us-central1-c", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1}, {"us-central1-f", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1}, }, - migTargetSizeCache: map[GceRef]int64{}, - instanceTemplateNameCache: map[GceRef]string{}, - instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, - migBaseNameCache: map[GceRef]string{}, + migTargetSizeCache: map[GceRef]int64{}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, + kubeEnvCache: map[GceRef]KubeEnv{}, + migBaseNameCache: map[GceRef]string{}, + listManagedInstancesResultsCache: map[GceRef]string{}, } migLister := NewMigLister(cache) manager := &gceManagerImpl{ diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index adf2240329fb..8e362f1d90ed 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -48,13 +48,14 @@ var ( ) type mockAutoscalingGceClient struct { - fetchMigs func(string) ([]*gce.InstanceGroupManager, error) - fetchMigTargetSize func(GceRef) (int64, error) - fetchMigBasename func(GceRef) (string, error) - fetchMigInstances func(GceRef) ([]cloudprovider.Instance, error) - fetchMigTemplateName func(GceRef) (string, error) - fetchMigTemplate func(GceRef, string) (*gce.InstanceTemplate, error) - fetchMachineType func(string, string) (*gce.MachineType, error) + fetchMigs func(string) ([]*gce.InstanceGroupManager, error) + fetchMigTargetSize func(GceRef) (int64, error) + fetchMigBasename func(GceRef) (string, error) + fetchMigInstances func(GceRef) ([]GceInstance, error) + fetchMigTemplateName func(GceRef) (InstanceTemplateNameType, error) + fetchMigTemplate func(GceRef, string) (*gce.InstanceTemplate, error) + fetchMachineType func(string, string) (*gce.MachineType, error) + fetchListManagedInstancesResults func(GceRef) (string, error) } func (client *mockAutoscalingGceClient) FetchMachineType(zone, machineName string) (*gce.MachineType, error) { @@ -81,12 +82,12 @@ func (client *mockAutoscalingGceClient) FetchMigInstances(migRef GceRef) ([]clou return client.fetchMigInstances(migRef) } -func (client *mockAutoscalingGceClient) FetchMigTemplateName(migRef GceRef) (string, error) { +func (client *mockAutoscalingGceClient) FetchMigTemplateName(migRef GceRef) (InstanceTemplateNameType, error) { return client.fetchMigTemplateName(migRef) } -func (client *mockAutoscalingGceClient) FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error) { - return client.fetchMigTemplate(migRef, templateName) +func (client *mockAutoscalingGceClient) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) { + return client.fetchMigTemplate(migRef, templateName, regional) } func (client *mockAutoscalingGceClient) FetchMigsWithName(_ string, _ *regexp.Regexp) ([]string, error) { @@ -771,7 +772,7 @@ func TestGetMigInstanceTemplateName(t *testing.T) { name string cache *GceCache fetchMigs func(string) ([]*gce.InstanceGroupManager, error) - fetchMigTemplateName func(GceRef) (string, error) + fetchMigTemplateName func(GceRef) (InstanceTemplateNameType, error) expectedTemplateName string expectedErr error }{ @@ -779,7 +780,7 @@ func TestGetMigInstanceTemplateName(t *testing.T) { name: "template name in cache", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, }, expectedTemplateName: templateName, }, @@ -793,14 +794,14 @@ func TestGetMigInstanceTemplateName(t *testing.T) { name: "cache fill without mig, fallback success", cache: emptyCache(), fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{}), - fetchMigTemplateName: fetchMigTemplateNameConst(templateName), + fetchMigTemplateName: fetchMigTemplateNameConst(InstanceTemplateNameType{templateName, false}), expectedTemplateName: templateName, }, { name: "cache fill failure, fallback success", cache: emptyCache(), fetchMigs: fetchMigsFail, - fetchMigTemplateName: fetchMigTemplateNameConst(templateName), + fetchMigTemplateName: fetchMigTemplateNameConst(InstanceTemplateNameType{templateName, false}), expectedTemplateName: templateName, }, { @@ -856,8 +857,8 @@ func TestGetMigInstanceTemplate(t *testing.T) { name string cache *GceCache fetchMigs func(string) ([]*gce.InstanceGroupManager, error) - fetchMigTemplateName func(GceRef) (string, error) - fetchMigTemplate func(GceRef, string) (*gce.InstanceTemplate, error) + fetchMigTemplateName func(GceRef) (InstanceTemplateNameType, error) + fetchMigTemplate func(GceRef, string, bool) (*gce.InstanceTemplate, error) expectedTemplate *gce.InstanceTemplate expectedCachedTemplate *gce.InstanceTemplate expectedErr error @@ -866,7 +867,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "template in cache", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): template}, }, expectedTemplate: template, @@ -876,7 +877,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache without template, fetch success", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), }, fetchMigTemplate: fetchMigTemplateConst(template), @@ -887,7 +888,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache with old template, fetch success", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, }, fetchMigTemplate: fetchMigTemplateConst(template), @@ -898,7 +899,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache without template, fetch failure", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), }, fetchMigTemplate: fetchMigTemplateFail, @@ -908,7 +909,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache with old template, fetch failure", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, }, fetchMigTemplate: fetchMigTemplateFail, @@ -1006,7 +1007,7 @@ func TestGetMigMachineType(t *testing.T) { }, } cache := &GceCache{ - instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): "template"}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{"template", false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{ mig.GceRef(): { Name: "template", @@ -1126,14 +1127,15 @@ func (f *fakeTime) Now() time.Time { func emptyCache() *GceCache { return &GceCache{ - migs: map[GceRef]Mig{mig.GceRef(): mig}, - instances: make(map[GceRef][]cloudprovider.Instance), - instancesUpdateTime: make(map[GceRef]time.Time), - migTargetSizeCache: make(map[GceRef]int64), - migBaseNameCache: make(map[GceRef]string), - instanceTemplateNameCache: make(map[GceRef]string), - instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), - instancesFromUnknownMig: make(map[GceRef]bool), + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instances: make(map[GceRef][]GceInstance), + instancesUpdateTime: make(map[GceRef]time.Time), + migTargetSizeCache: make(map[GceRef]int64), + migBaseNameCache: make(map[GceRef]string), + listManagedInstancesResultsCache: make(map[GceRef]string), + instanceTemplateNameCache: make(map[GceRef]InstanceTemplateNameType), + instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), + instancesFromUnknownMig: make(map[GceRef]bool), } } @@ -1190,22 +1192,22 @@ func fetchMigBasenameConst(basename string) func(GceRef) (string, error) { } } -func fetchMigTemplateNameFail(_ GceRef) (string, error) { - return "", errFetchMigTemplateName +func fetchMigTemplateNameFail(_ GceRef) (InstanceTemplateNameType, error) { + return InstanceTemplateNameType{}, errFetchMigTemplateName } -func fetchMigTemplateNameConst(templateName string) func(GceRef) (string, error) { - return func(GceRef) (string, error) { - return templateName, nil +func fetchMigTemplateNameConst(instanceTemplateNameType InstanceTemplateNameType) func(GceRef) (InstanceTemplateNameType, error) { + return func(GceRef) (InstanceTemplateNameType, error) { + return instanceTemplateNameType, nil } } -func fetchMigTemplateFail(_ GceRef, _ string) (*gce.InstanceTemplate, error) { +func fetchMigTemplateFail(_ GceRef, _ string, _ bool) (*gce.InstanceTemplate, error) { return nil, errFetchMigTemplate } -func fetchMigTemplateConst(template *gce.InstanceTemplate) func(GceRef, string) (*gce.InstanceTemplate, error) { - return func(GceRef, string) (*gce.InstanceTemplate, error) { +func fetchMigTemplateConst(template *gce.InstanceTemplate) func(GceRef, string, bool) (*gce.InstanceTemplate, error) { + return func(GceRef, string, bool) (*gce.InstanceTemplate, error) { return template, nil } } From cc099f76dcc928aad8406d31e1698035daa03fcd Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Tue, 27 Feb 2024 00:12:37 +0000 Subject: [PATCH 03/14] added comment to InstanceTemplateNameType --- cluster-autoscaler/cloudprovider/gce/cache.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cluster-autoscaler/cloudprovider/gce/cache.go b/cluster-autoscaler/cloudprovider/gce/cache.go index 6bbedce1d7d6..830a813cf13c 100644 --- a/cluster-autoscaler/cloudprovider/gce/cache.go +++ b/cluster-autoscaler/cloudprovider/gce/cache.go @@ -33,6 +33,8 @@ type MachineTypeKey struct { MachineTypeName string } +// InstanceTemplateNameType is used to store the name, and +// whether or not the instance template is regional type InstanceTemplateNameType struct { Name string Regional bool From 2f854f50690abeca06b349798b8b8813d4c405e4 Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Tue, 27 Feb 2024 00:23:01 +0000 Subject: [PATCH 04/14] Ran hack/go-fmtupdate.h on mig_info_provider_test.go --- .../cloudprovider/gce/mig_info_provider_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index 8e362f1d90ed..af5b44be85a3 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -780,7 +780,7 @@ func TestGetMigInstanceTemplateName(t *testing.T) { name: "template name in cache", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, }, expectedTemplateName: templateName, }, @@ -867,7 +867,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "template in cache", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): template}, }, expectedTemplate: template, @@ -877,7 +877,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache without template, fetch success", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), }, fetchMigTemplate: fetchMigTemplateConst(template), @@ -888,7 +888,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache with old template, fetch success", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, }, fetchMigTemplate: fetchMigTemplateConst(template), @@ -899,7 +899,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache without template, fetch failure", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), }, fetchMigTemplate: fetchMigTemplateFail, @@ -909,7 +909,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache with old template, fetch failure", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, }, fetchMigTemplate: fetchMigTemplateFail, @@ -1007,7 +1007,7 @@ func TestGetMigMachineType(t *testing.T) { }, } cache := &GceCache{ - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): InstanceTemplateNameType{"template", false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {"template", false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{ mig.GceRef(): { Name: "template", From a9f3c9aad745faa3ee1429c82770ad5e1b245323 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Wr=C3=B3blewski?= Date: Thu, 15 Feb 2024 14:58:07 +0000 Subject: [PATCH 05/14] Use KubeEnv in gce/templates.go --- cluster-autoscaler/cloudprovider/gce/kube_env.go | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 cluster-autoscaler/cloudprovider/gce/kube_env.go diff --git a/cluster-autoscaler/cloudprovider/gce/kube_env.go b/cluster-autoscaler/cloudprovider/gce/kube_env.go new file mode 100644 index 000000000000..e69de29bb2d1 From 4d28568e85f8e58d444f641c5954febf990ab2c8 Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Wed, 28 Feb 2024 18:37:30 +0000 Subject: [PATCH 06/14] rebased and resolved conflicts --- .../cloudprovider/gce/autoscaling_gce_client.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go index 505566796cda..6480c58bf88e 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go @@ -562,10 +562,14 @@ func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (Insta return InstanceTemplateNameType{}, err } templateUrl, err := url.Parse(igm.InstanceTemplate) - regional, _ := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) if err != nil { return InstanceTemplateNameType{}, err } + regional, err := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) + if err != nil { + return InstanceTemplateNameType{}, err + } + _, templateName := path.Split(templateUrl.EscapedPath()) return InstanceTemplateNameType{templateName, regional}, nil } From 974bf4829b1af13c19317d9d060c7a15ffafd4cf Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Wed, 28 Feb 2024 19:21:02 +0000 Subject: [PATCH 07/14] added fix for unit tests --- .../cloudprovider/gce/mig_info_provider_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index af5b44be85a3..fa72c88d895a 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -829,14 +829,14 @@ func TestGetMigInstanceTemplateName(t *testing.T) { migLister := NewMigLister(tc.cache) provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second) - templateName, err := provider.GetMigInstanceTemplateName(mig.GceRef()) - cachedTemplateName, found := tc.cache.GetMigInstanceTemplateName(mig.GceRef()) + instanceTemplateNameType, err := provider.GetMigInstanceTemplateName(mig.GceRef()) + cachedInstanceTemplateNameType, found := tc.cache.GetMigInstanceTemplateName(mig.GceRef()) assert.Equal(t, tc.expectedErr, err) assert.Equal(t, tc.expectedErr == nil, found) if tc.expectedErr == nil { - assert.Equal(t, tc.expectedTemplateName, templateName) - assert.Equal(t, tc.expectedTemplateName, cachedTemplateName) + assert.Equal(t, tc.expectedTemplateName, instanceTemplateNameType.Name) + assert.Equal(t, tc.expectedTemplateName, cachedInstanceTemplateNameType.Name) } }) } From e88f52c6e1b5040e216eddf8dbb954b841773670 Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Wed, 28 Feb 2024 22:03:30 +0000 Subject: [PATCH 08/14] changed InstanceTemplateNameType to InstanceTemplateName --- .../gce/autoscaling_gce_client.go | 14 ++--- cluster-autoscaler/cloudprovider/gce/cache.go | 20 +++---- .../cloudprovider/gce/gce_manager_test.go | 2 +- .../cloudprovider/gce/mig_info_provider.go | 30 +++++----- .../gce/mig_info_provider_test.go | 55 +++++++++++-------- 5 files changed, 66 insertions(+), 55 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go index 6480c58bf88e..f8919a50c971 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go @@ -97,7 +97,7 @@ type AutoscalingGceClient interface { FetchMigTargetSize(GceRef) (int64, error) FetchMigBasename(GceRef) (string, error) FetchMigInstances(GceRef) ([]GceInstance, error) - FetchMigTemplateName(migRef GceRef) (InstanceTemplateNameType, error) + FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) FetchMigsWithName(zone string, filter *regexp.Regexp) ([]string, error) FetchZones(region string) ([]string, error) @@ -550,28 +550,28 @@ func (client *autoscalingGceClientV1) FetchAvailableCpuPlatforms() (map[string][ return availableCpuPlatforms, nil } -func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (InstanceTemplateNameType, error) { +func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error) { registerRequest("instance_group_managers", "get") igm, err := client.gceService.InstanceGroupManagers.Get(migRef.Project, migRef.Zone, migRef.Name).Do() if err != nil { if err, ok := err.(*googleapi.Error); ok { if err.Code == http.StatusNotFound { - return InstanceTemplateNameType{}, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error()) + return InstanceTemplateName{}, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error()) } } - return InstanceTemplateNameType{}, err + return InstanceTemplateName{}, err } templateUrl, err := url.Parse(igm.InstanceTemplate) if err != nil { - return InstanceTemplateNameType{}, err + return InstanceTemplateName{}, err } regional, err := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) if err != nil { - return InstanceTemplateNameType{}, err + return InstanceTemplateName{}, err } _, templateName := path.Split(templateUrl.EscapedPath()) - return InstanceTemplateNameType{templateName, regional}, nil + return InstanceTemplateName{templateName, regional}, nil } func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) { diff --git a/cluster-autoscaler/cloudprovider/gce/cache.go b/cluster-autoscaler/cloudprovider/gce/cache.go index 830a813cf13c..400ec090a31e 100644 --- a/cluster-autoscaler/cloudprovider/gce/cache.go +++ b/cluster-autoscaler/cloudprovider/gce/cache.go @@ -33,9 +33,9 @@ type MachineTypeKey struct { MachineTypeName string } -// InstanceTemplateNameType is used to store the name, and +// InstanceTemplateName is used to store the name, and // whether or not the instance template is regional -type InstanceTemplateNameType struct { +type InstanceTemplateName struct { Name string Regional bool } @@ -74,7 +74,7 @@ type GceCache struct { migTargetSizeCache map[GceRef]int64 migBaseNameCache map[GceRef]string listManagedInstancesResultsCache map[GceRef]string - instanceTemplateNameCache map[GceRef]InstanceTemplateNameType + instanceTemplateNameCache map[GceRef]InstanceTemplateName instanceTemplatesCache map[GceRef]*gce.InstanceTemplate kubeEnvCache map[GceRef]KubeEnv } @@ -92,7 +92,7 @@ func NewGceCache() *GceCache { migTargetSizeCache: map[GceRef]int64{}, migBaseNameCache: map[GceRef]string{}, listManagedInstancesResultsCache: map[GceRef]string{}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, kubeEnvCache: map[GceRef]KubeEnv{}, } @@ -341,23 +341,23 @@ func (gc *GceCache) InvalidateAllMigTargetSizes() { } // GetMigInstanceTemplateName returns the cached instance template ref for a mig GceRef -func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (InstanceTemplateNameType, bool) { +func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (InstanceTemplateName, bool) { gc.cacheMutex.Lock() defer gc.cacheMutex.Unlock() - instanceTemplateNameType, found := gc.instanceTemplateNameCache[ref] + instanceTemplateName, found := gc.instanceTemplateNameCache[ref] if found { klog.V(5).Infof("Instance template names cache hit for %s", ref) } - return instanceTemplateNameType, found + return instanceTemplateName, found } // SetMigInstanceTemplateName sets instance template ref for a mig GceRef -func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, instanceTemplateNameType InstanceTemplateNameType) { +func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, instanceTemplateName InstanceTemplateName) { gc.cacheMutex.Lock() defer gc.cacheMutex.Unlock() - gc.instanceTemplateNameCache[ref] = instanceTemplateNameType + gc.instanceTemplateNameCache[ref] = instanceTemplateName } // InvalidateMigInstanceTemplateName clears the instance template ref cache for a mig GceRef @@ -377,7 +377,7 @@ func (gc *GceCache) InvalidateAllMigInstanceTemplateNames() { defer gc.cacheMutex.Unlock() klog.V(5).Infof("Instance template names cache invalidated") - gc.instanceTemplateNameCache = map[GceRef]InstanceTemplateNameType{} + gc.instanceTemplateNameCache = map[GceRef]InstanceTemplateName{} } // GetMigInstanceTemplate returns the cached gce.InstanceTemplate for a mig GceRef diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go index 341bfaea3457..6bb102e9216b 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go @@ -344,7 +344,7 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa {"us-central1-f", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1}, }, migTargetSizeCache: map[GceRef]int64{}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, kubeEnvCache: map[GceRef]KubeEnv{}, migBaseNameCache: map[GceRef]string{}, diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go index c635b7bd1b40..88f42f83b020 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go @@ -45,7 +45,7 @@ type MigInfoProvider interface { // GetMigBasename returns basename for given MIG ref GetMigBasename(migRef GceRef) (string, error) // GetMigInstanceTemplateName returns instance template name for given MIG ref - GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateNameType, error) + GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateName, error) // GetMigInstanceTemplate returns instance template for given MIG ref GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error) // GetMigMachineType returns machine type used by a MIG. @@ -241,44 +241,44 @@ func (c *cachingMigInfoProvider) GetMigBasename(migRef GceRef) (string, error) { return basename, nil } -func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateNameType, error) { +func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateName, error) { c.migInfoMutex.Lock() defer c.migInfoMutex.Unlock() - instanceTemplateNameType, found := c.cache.GetMigInstanceTemplateName(migRef) + instanceTemplateName, found := c.cache.GetMigInstanceTemplateName(migRef) if found { - return instanceTemplateNameType, nil + return instanceTemplateName, nil } err := c.fillMigInfoCache() - instanceTemplateNameType, found = c.cache.GetMigInstanceTemplateName(migRef) + instanceTemplateName, found = c.cache.GetMigInstanceTemplateName(migRef) if err == nil && found { - return instanceTemplateNameType, nil + return instanceTemplateName, nil } // fallback to querying for single mig - instanceTemplateNameType, err = c.gceClient.FetchMigTemplateName(migRef) + instanceTemplateName, err = c.gceClient.FetchMigTemplateName(migRef) if err != nil { c.migLister.HandleMigIssue(migRef, err) - return InstanceTemplateNameType{}, err + return InstanceTemplateName{}, err } - c.cache.SetMigInstanceTemplateName(migRef, instanceTemplateNameType) - return instanceTemplateNameType, nil + c.cache.SetMigInstanceTemplateName(migRef, instanceTemplateName) + return instanceTemplateName, nil } func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error) { - instanceTemplateNameType, err := c.GetMigInstanceTemplateName(migRef) + instanceTemplateName, err := c.GetMigInstanceTemplateName(migRef) if err != nil { return nil, err } template, found := c.cache.GetMigInstanceTemplate(migRef) - if found && template.Name == instanceTemplateNameType.Name { + if found && template.Name == instanceTemplateName.Name { return template, nil } - klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, instanceTemplateNameType.Name) - template, err = c.gceClient.FetchMigTemplate(migRef, instanceTemplateNameType.Name, instanceTemplateNameType.Regional) + klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, instanceTemplateName.Name) + template, err = c.gceClient.FetchMigTemplate(migRef, instanceTemplateName.Name, instanceTemplateName.Regional) if err != nil { return nil, err } @@ -338,7 +338,7 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error { if err == nil { _, templateName := path.Split(templateUrl.EscapedPath()) regional, _ := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) - c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateNameType{templateName, regional}) + c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateName{templateName, regional}) } } } diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index fa72c88d895a..2fbde6ef62d4 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -52,7 +52,7 @@ type mockAutoscalingGceClient struct { fetchMigTargetSize func(GceRef) (int64, error) fetchMigBasename func(GceRef) (string, error) fetchMigInstances func(GceRef) ([]GceInstance, error) - fetchMigTemplateName func(GceRef) (InstanceTemplateNameType, error) + fetchMigTemplate func(GceRef, string, bool) (*gce.InstanceTemplate, error) fetchMigTemplate func(GceRef, string) (*gce.InstanceTemplate, error) fetchMachineType func(string, string) (*gce.MachineType, error) fetchListManagedInstancesResults func(GceRef) (string, error) @@ -82,7 +82,7 @@ func (client *mockAutoscalingGceClient) FetchMigInstances(migRef GceRef) ([]clou return client.fetchMigInstances(migRef) } -func (client *mockAutoscalingGceClient) FetchMigTemplateName(migRef GceRef) (InstanceTemplateNameType, error) { +func (client *mockAutoscalingGceClient) FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error) { return client.fetchMigTemplateName(migRef) } @@ -772,7 +772,7 @@ func TestGetMigInstanceTemplateName(t *testing.T) { name string cache *GceCache fetchMigs func(string) ([]*gce.InstanceGroupManager, error) - fetchMigTemplateName func(GceRef) (InstanceTemplateNameType, error) + fetchMigTemplateName func(GceRef) (InstanceTemplateName, error) expectedTemplateName string expectedErr error }{ @@ -780,7 +780,7 @@ func TestGetMigInstanceTemplateName(t *testing.T) { name: "template name in cache", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{mig.GceRef(): {templateName, false}}, }, expectedTemplateName: templateName, }, @@ -794,14 +794,14 @@ func TestGetMigInstanceTemplateName(t *testing.T) { name: "cache fill without mig, fallback success", cache: emptyCache(), fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{}), - fetchMigTemplateName: fetchMigTemplateNameConst(InstanceTemplateNameType{templateName, false}), + fetchMigTemplateName: fetchMigTemplateNameConst(InstanceTemplateName{templateName, false}), expectedTemplateName: templateName, }, { name: "cache fill failure, fallback success", cache: emptyCache(), fetchMigs: fetchMigsFail, - fetchMigTemplateName: fetchMigTemplateNameConst(InstanceTemplateNameType{templateName, false}), + fetchMigTemplateName: fetchMigTemplateNameConst(InstanceTemplateName{templateName, false}), expectedTemplateName: templateName, }, { @@ -829,14 +829,14 @@ func TestGetMigInstanceTemplateName(t *testing.T) { migLister := NewMigLister(tc.cache) provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second) - instanceTemplateNameType, err := provider.GetMigInstanceTemplateName(mig.GceRef()) - cachedInstanceTemplateNameType, found := tc.cache.GetMigInstanceTemplateName(mig.GceRef()) + instanceTemplateName, err := provider.GetMigInstanceTemplateName(mig.GceRef()) + cachedInstanceTemplateName, found := tc.cache.GetMigInstanceTemplateName(mig.GceRef()) assert.Equal(t, tc.expectedErr, err) assert.Equal(t, tc.expectedErr == nil, found) if tc.expectedErr == nil { - assert.Equal(t, tc.expectedTemplateName, instanceTemplateNameType.Name) - assert.Equal(t, tc.expectedTemplateName, cachedInstanceTemplateNameType.Name) + assert.Equal(t, tc.expectedTemplateName, instanceTemplateName.Name) + assert.Equal(t, tc.expectedTemplateName, cachedInstanceTemplateName.Name) } }) } @@ -857,7 +857,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name string cache *GceCache fetchMigs func(string) ([]*gce.InstanceGroupManager, error) - fetchMigTemplateName func(GceRef) (InstanceTemplateNameType, error) + fetchMigTemplateName func(GceRef) (InstanceTemplateName, error) fetchMigTemplate func(GceRef, string, bool) (*gce.InstanceTemplate, error) expectedTemplate *gce.InstanceTemplate expectedCachedTemplate *gce.InstanceTemplate @@ -867,7 +867,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "template in cache", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): template}, }, expectedTemplate: template, @@ -877,7 +877,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache without template, fetch success", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), }, fetchMigTemplate: fetchMigTemplateConst(template), @@ -888,7 +888,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache with old template, fetch success", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, }, fetchMigTemplate: fetchMigTemplateConst(template), @@ -899,7 +899,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache without template, fetch failure", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), }, fetchMigTemplate: fetchMigTemplateFail, @@ -909,7 +909,7 @@ func TestGetMigInstanceTemplate(t *testing.T) { name: "cache with old template, fetch failure", cache: &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {templateName, false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{mig.GceRef(): {templateName, false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, }, fetchMigTemplate: fetchMigTemplateFail, @@ -1007,7 +1007,7 @@ func TestGetMigMachineType(t *testing.T) { }, } cache := &GceCache{ - instanceTemplateNameCache: map[GceRef]InstanceTemplateNameType{mig.GceRef(): {"template", false}}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{mig.GceRef(): {"template", false}}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{ mig.GceRef(): { Name: "template", @@ -1127,6 +1127,7 @@ func (f *fakeTime) Now() time.Time { func emptyCache() *GceCache { return &GceCache{ +<<<<<<< HEAD migs: map[GceRef]Mig{mig.GceRef(): mig}, instances: make(map[GceRef][]GceInstance), instancesUpdateTime: make(map[GceRef]time.Time), @@ -1136,6 +1137,16 @@ func emptyCache() *GceCache { instanceTemplateNameCache: make(map[GceRef]InstanceTemplateNameType), instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), instancesFromUnknownMig: make(map[GceRef]bool), +======= + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instances: make(map[GceRef][]GceInstance), + instancesUpdateTime: make(map[GceRef]time.Time), + migTargetSizeCache: make(map[GceRef]int64), + migBaseNameCache: make(map[GceRef]string), + instanceTemplateNameCache: make(map[GceRef]InstanceTemplateName), + instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), + instancesFromUnknownMig: make(map[GceRef]bool), +>>>>>>> 2aeec4097 (changed InstanceTemplateNameType to InstanceTemplateName) } } @@ -1192,13 +1203,13 @@ func fetchMigBasenameConst(basename string) func(GceRef) (string, error) { } } -func fetchMigTemplateNameFail(_ GceRef) (InstanceTemplateNameType, error) { - return InstanceTemplateNameType{}, errFetchMigTemplateName +func fetchMigTemplateNameFail(_ GceRef) (InstanceTemplateName, error) { + return InstanceTemplateName{}, errFetchMigTemplateName } -func fetchMigTemplateNameConst(instanceTemplateNameType InstanceTemplateNameType) func(GceRef) (InstanceTemplateNameType, error) { - return func(GceRef) (InstanceTemplateNameType, error) { - return instanceTemplateNameType, nil +func fetchMigTemplateNameConst(instanceTemplateName InstanceTemplateName) func(GceRef) (InstanceTemplateName, error) { + return func(GceRef) (InstanceTemplateName, error) { + return instanceTemplateName, nil } } From 69f12dc64a3f9edc89fc41258a4081edd81da54f Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Thu, 7 Mar 2024 19:35:06 +0000 Subject: [PATCH 09/14] separated url parser to its own function, created unit test for the function --- .../gce/autoscaling_gce_client.go | 2 +- .../cloudprovider/gce/gce_url.go | 5 +++ .../cloudprovider/gce/gce_url_test.go | 31 +++++++++++++++++++ .../cloudprovider/gce/mig_info_provider.go | 9 ++++-- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go index f8919a50c971..bfdc1081b2dd 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go @@ -565,7 +565,7 @@ func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (Insta if err != nil { return InstanceTemplateName{}, err } - regional, err := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) + regional, err := IsInstanceTemplateRegional(templateUrl.String()) if err != nil { return InstanceTemplateName{}, err } diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url.go b/cluster-autoscaler/cloudprovider/gce/gce_url.go index 7b0ab135a46c..863f996002a1 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_url.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_url.go @@ -78,6 +78,11 @@ func GenerateMigUrl(domainUrl string, ref GceRef) string { return fmt.Sprintf(migUrlTemplate, ref.Project, ref.Zone, ref.Name) } +// IsInstanceTemplateRegional verifies if the instance template is regional or global +func IsInstanceTemplateRegional(templateUrl string) (bool, error) { + return regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl) +} + func parseGceUrl(url, expectedResource string) (project string, zone string, name string, err error) { reg := regexp.MustCompile(fmt.Sprintf("https://.*/projects/.*/zones/.*/%s/.*", expectedResource)) errMsg := fmt.Errorf("wrong url: expected format /projects//zones//%s/, got %s", expectedResource, url) diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url_test.go b/cluster-autoscaler/cloudprovider/gce/gce_url_test.go index 1317ef76e605..feee3b2432ae 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_url_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_url_test.go @@ -263,3 +263,34 @@ func TestParseMigUrl(t *testing.T) { }) } } + +func TestIsInstanceTemplateRegional(t *testing.T) { + tests := []struct { + name string + templateUrl string + expectRegional bool + wantErr error + }{ + { + name: "Has regional instance url", + templateUrl: "https://www.googleapis.com/compute/v1/projects/test-project/regions/us-central1/instanceTemplates/instance-template", + expectRegional: true, + }, + { + name: "Has global instance url", + templateUrl: "https://www.googleapis.com/compute/v1/projects/test-project/global/instanceTemplates/instance-template", + expectRegional: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + regional, err := IsInstanceTemplateRegional(tt.templateUrl) + assert.Equal(t, tt.wantErr, err) + if tt.wantErr != nil { + return + } + assert.Equal(t, tt.expectRegional, regional) + }) + } +} diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go index 88f42f83b020..0757d1e68bd3 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go @@ -21,7 +21,6 @@ import ( "fmt" "net/url" "path" - "regexp" "strings" "sync" "time" @@ -337,8 +336,12 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error { templateUrl, err := url.Parse(zoneMig.InstanceTemplate) if err == nil { _, templateName := path.Split(templateUrl.EscapedPath()) - regional, _ := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String()) - c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateName{templateName, regional}) + regional, err := IsInstanceTemplateRegional(templateUrl.String()) + if err != nil { + klog.Errorf("Error parsing instance template url: %v; err=%v ", templateUrl.String(), err) + } else { + c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateName{templateName, regional}) + } } } } From fd07fd08749f15e961bd0a5df95a38c333b4dbba Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Thu, 7 Mar 2024 19:35:06 +0000 Subject: [PATCH 10/14] separated url parser to its own function, created unit test for the function --- cluster-autoscaler/cloudprovider/gce/gce_url.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url.go b/cluster-autoscaler/cloudprovider/gce/gce_url.go index 863f996002a1..72fc3e774550 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_url.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_url.go @@ -78,7 +78,6 @@ func GenerateMigUrl(domainUrl string, ref GceRef) string { return fmt.Sprintf(migUrlTemplate, ref.Project, ref.Zone, ref.Name) } -// IsInstanceTemplateRegional verifies if the instance template is regional or global func IsInstanceTemplateRegional(templateUrl string) (bool, error) { return regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl) } From e937e170001c4b217c8aaf9ff9a1dd4277c7c04f Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Thu, 7 Mar 2024 20:09:00 +0000 Subject: [PATCH 11/14] added unit test with regional MIG --- .../cloudprovider/gce/gce_url.go | 1 + .../gce/mig_info_provider_test.go | 30 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url.go b/cluster-autoscaler/cloudprovider/gce/gce_url.go index 72fc3e774550..80ef91207ff0 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_url.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_url.go @@ -78,6 +78,7 @@ func GenerateMigUrl(domainUrl string, ref GceRef) string { return fmt.Sprintf(migUrlTemplate, ref.Project, ref.Zone, ref.Name) } +// IsInstanceTemplateRegional determines whether or not an instance template is regional based on the url func IsInstanceTemplateRegional(templateUrl string) (bool, error) { return regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl) } diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index 2fbde6ef62d4..c5f87cdb09e3 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -52,8 +52,8 @@ type mockAutoscalingGceClient struct { fetchMigTargetSize func(GceRef) (int64, error) fetchMigBasename func(GceRef) (string, error) fetchMigInstances func(GceRef) ([]GceInstance, error) + fetchMigTemplateName func(GceRef) (InstanceTemplateName, error) fetchMigTemplate func(GceRef, string, bool) (*gce.InstanceTemplate, error) - fetchMigTemplate func(GceRef, string) (*gce.InstanceTemplate, error) fetchMachineType func(string, string) (*gce.MachineType, error) fetchListManagedInstancesResults func(GceRef) (string, error) } @@ -765,7 +765,13 @@ func TestGetMigInstanceTemplateName(t *testing.T) { instanceGroupManager := &gce.InstanceGroupManager{ Zone: mig.GceRef().Zone, Name: mig.GceRef().Name, - InstanceTemplate: templateName, + InstanceTemplate: "https://www.googleapis.com/compute/v1/projects/test-project/global/instanceTemplates/template-name", + } + + instanceGroupManagerRegional := &gce.InstanceGroupManager{ + Zone: mig.GceRef().Zone, + Name: mig.GceRef().Name, + InstanceTemplate: "https://www.googleapis.com/compute/v1/projects/test-project/regions/us-central1/instanceTemplates/template-name", } testCases := []struct { @@ -774,6 +780,7 @@ func TestGetMigInstanceTemplateName(t *testing.T) { fetchMigs func(string) ([]*gce.InstanceGroupManager, error) fetchMigTemplateName func(GceRef) (InstanceTemplateName, error) expectedTemplateName string + expectedRegion bool expectedErr error }{ { @@ -790,6 +797,12 @@ func TestGetMigInstanceTemplateName(t *testing.T) { fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{instanceGroupManager}), expectedTemplateName: templateName, }, + { + name: "target size from cache fill, regional", + cache: emptyCache(), + fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{instanceGroupManagerRegional}), + expectedTemplateName: templateName, + }, { name: "cache fill without mig, fallback success", cache: emptyCache(), @@ -1127,26 +1140,15 @@ func (f *fakeTime) Now() time.Time { func emptyCache() *GceCache { return &GceCache{ -<<<<<<< HEAD migs: map[GceRef]Mig{mig.GceRef(): mig}, instances: make(map[GceRef][]GceInstance), instancesUpdateTime: make(map[GceRef]time.Time), migTargetSizeCache: make(map[GceRef]int64), migBaseNameCache: make(map[GceRef]string), listManagedInstancesResultsCache: make(map[GceRef]string), - instanceTemplateNameCache: make(map[GceRef]InstanceTemplateNameType), + instanceTemplateNameCache: make(map[GceRef]InstanceTemplateName), instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), instancesFromUnknownMig: make(map[GceRef]bool), -======= - migs: map[GceRef]Mig{mig.GceRef(): mig}, - instances: make(map[GceRef][]GceInstance), - instancesUpdateTime: make(map[GceRef]time.Time), - migTargetSizeCache: make(map[GceRef]int64), - migBaseNameCache: make(map[GceRef]string), - instanceTemplateNameCache: make(map[GceRef]InstanceTemplateName), - instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), - instancesFromUnknownMig: make(map[GceRef]bool), ->>>>>>> 2aeec4097 (changed InstanceTemplateNameType to InstanceTemplateName) } } From f7d2b929de5537ef39be876b0a514fe795cd725c Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Mon, 2 Sep 2024 23:03:50 +0000 Subject: [PATCH 12/14] cleaned up --- cherry_pick_pull.sh | 244 ------------------ .../gce/autoscaling_gce_client.go | 2 +- cluster-autoscaler/cloudprovider/gce/cache.go | 6 +- .../cloudprovider/gce/gce_manager_test.go | 1 - .../cloudprovider/gce/kube_env.go | 0 .../gce/mig_info_provider_test.go | 4 +- 6 files changed, 5 insertions(+), 252 deletions(-) delete mode 100755 cherry_pick_pull.sh delete mode 100644 cluster-autoscaler/cloudprovider/gce/kube_env.go diff --git a/cherry_pick_pull.sh b/cherry_pick_pull.sh deleted file mode 100755 index 92585bedfdf6..000000000000 --- a/cherry_pick_pull.sh +++ /dev/null @@ -1,244 +0,0 @@ -#!/usr/bin/env bash - -# Copyright 2015 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. - -# Usage Instructions: https://karmada.io/docs/contributor/cherry-picks - -# Checkout a PR from GitHub. (Yes, this is sitting in a Git tree. How -# meta.) Assumes you care about pulls from remote "upstream" and -# checks them out to a branch named: -# automated-cherry-pick-of--- - -set -o errexit -set -o nounset -set -o pipefail - -REPO_ROOT="$(git rev-parse --show-toplevel)" -declare -r REPO_ROOT -cd "${REPO_ROOT}" - -STARTINGBRANCH=$(git symbolic-ref --short HEAD) -declare -r STARTINGBRANCH -declare -r REBASEMAGIC="${REPO_ROOT}/.git/rebase-apply" -DRY_RUN=${DRY_RUN:-""} -REGENERATE_DOCS=${REGENERATE_DOCS:-""} -UPSTREAM_REMOTE=${UPSTREAM_REMOTE:-upstream} -FORK_REMOTE=${FORK_REMOTE:-origin} -MAIN_REPO_ORG=${MAIN_REPO_ORG:-$(git remote get-url "$UPSTREAM_REMOTE" | awk '{gsub(/http[s]:\/\/|git@/,"")}1' | awk -F'[@:./]' 'NR==1{print $3}')} -MAIN_REPO_NAME=${MAIN_REPO_NAME:-$(git remote get-url "$UPSTREAM_REMOTE" | awk '{gsub(/http[s]:\/\/|git@/,"")}1' | awk -F'[@:./]' 'NR==1{print $4}')} - -if [[ -z ${GITHUB_USER:-} ]]; then - echo "Please export GITHUB_USER= (or GH organization, if that's where your fork lives)" - exit 1 -fi - -if ! command -v gh >/dev/null; then - echo "Can't find 'gh' tool in PATH, please install from https://github.com/cli/cli" - exit 1 -fi - -if [[ "$#" -lt 2 ]]; then - echo "${0} ...: cherry pick one or more onto and leave instructions for proposing pull request" - echo - echo " Checks out and handles the cherry-pick of (possibly multiple) for you." - echo " Examples:" - echo " $0 upstream/release-3.14 12345 # Cherry-picks PR 12345 onto upstream/release-3.14 and proposes that as a PR." - echo " $0 upstream/release-3.14 12345 56789 # Cherry-picks PR 12345, then 56789 and proposes the combination as a single PR." - echo - echo " Set the DRY_RUN environment var to skip git push and creating PR." - echo " This is useful for creating patches to a release branch without making a PR." - echo " When DRY_RUN is set the script will leave you in a branch containing the commits you cherry-picked." - echo - echo " Set UPSTREAM_REMOTE (default: upstream) and FORK_REMOTE (default: origin)" - echo " to override the default remote names to what you have locally." - echo - echo " For merge process info, see https://karmada.io/docs/contributor/cherry-picks" - exit 2 -fi - -# Checks if you are logged in. Will error/bail if you are not. -gh auth status - -if git_status=$(git status --porcelain --untracked=no 2>/dev/null) && [[ -n "${git_status}" ]]; then - echo "!!! Dirty tree. Clean up and try again." - exit 1 -fi - -if [[ -e "${REBASEMAGIC}" ]]; then - echo "!!! 'git rebase' or 'git am' in progress. Clean up and try again." - exit 1 -fi - -declare -r BRANCH="$1" -shift 1 -declare -r PULLS=("$@") - -function join { - local IFS="$1" - shift - echo "$*" -} -PULLDASH=$(join - "${PULLS[@]/#/#}") # Generates something like "#12345-#56789" -declare -r PULLDASH -PULLSUBJ=$(join " " "${PULLS[@]/#/#}") # Generates something like "#12345 #56789" -declare -r PULLSUBJ - -echo "+++ Updating remotes..." -git remote update "${UPSTREAM_REMOTE}" "${FORK_REMOTE}" - -if ! git log -n1 --format=%H "${BRANCH}" >/dev/null 2>&1; then - echo "!!! '${BRANCH}' not found. The second argument should be something like ${UPSTREAM_REMOTE}/release-1.0." - echo " (In particular, it needs to be a valid, existing remote branch that I can 'git checkout'.)" - exit 1 -fi - -NEWBRANCHREQ="automated-cherry-pick-of-${PULLDASH}" # "Required" portion for tools. -declare -r NEWBRANCHREQ -NEWBRANCH="$(echo "${NEWBRANCHREQ}-${BRANCH}" | sed 's/\//-/g')" -declare -r NEWBRANCH -NEWBRANCHUNIQ="${NEWBRANCH}-$(date +%s)" -declare -r NEWBRANCHUNIQ -echo "+++ Creating local branch ${NEWBRANCHUNIQ}" - -cleanbranch="" -gitamcleanup=false -function return_to_kansas { - if [[ "${gitamcleanup}" == "true" ]]; then - echo - echo "+++ Aborting in-progress git am." - git am --abort >/dev/null 2>&1 || true - fi - - # return to the starting branch and delete the PR text file - if [[ -z "${DRY_RUN}" ]]; then - echo - echo "+++ Returning you to the ${STARTINGBRANCH} branch and cleaning up." - git checkout -f "${STARTINGBRANCH}" >/dev/null 2>&1 || true - if [[ -n "${cleanbranch}" ]]; then - git branch -D "${cleanbranch}" >/dev/null 2>&1 || true - fi - fi -} -trap return_to_kansas EXIT - -SUBJECTS=() -function make-a-pr() { - local rel - rel="$(basename "${BRANCH}")" - echo - echo "+++ Creating a pull request on GitHub at ${GITHUB_USER}:${NEWBRANCH}" - - local numandtitle - numandtitle=$(printf '%s\n' "${SUBJECTS[@]}") - prtext=$( - cat <&2 - exit 1 - fi - done - - if [[ "${conflicts}" != "true" ]]; then - echo "!!! git am failed, likely because of an in-progress 'git am' or 'git rebase'" - exit 1 - fi - } - - # set the subject - subject=$(grep -m 1 "^Subject" "/tmp/${pull}.patch" | sed -e 's/Subject: \[PATCH//g' | sed 's/.*] //') - SUBJECTS+=("#${pull}: ${subject}") - - # remove the patch file from /tmp - rm -f "/tmp/${pull}.patch" -done -gitamcleanup=false - -if [[ -n "${DRY_RUN}" ]]; then - echo "!!! Skipping git push and PR creation because you set DRY_RUN." - echo "To return to the branch you were in when you invoked this script:" - echo - echo " git checkout ${STARTINGBRANCH}" - echo - echo "To delete this branch:" - echo - echo " git branch -D ${NEWBRANCHUNIQ}" - exit 0 -fi - -if git remote -v | grep ^"${FORK_REMOTE}" | grep "${MAIN_REPO_ORG}/${MAIN_REPO_NAME}.git"; then - echo "!!! You have ${FORK_REMOTE} configured as your ${MAIN_REPO_ORG}/${MAIN_REPO_NAME}.git" - echo "This isn't normal. Leaving you with push instructions:" - echo - echo "+++ First manually push the branch this script created:" - echo - echo " git push REMOTE ${NEWBRANCHUNIQ}:${NEWBRANCH}" - echo - echo "where REMOTE is your personal fork (maybe ${UPSTREAM_REMOTE}? Consider swapping those.)." - echo "OR consider setting UPSTREAM_REMOTE and FORK_REMOTE to different values." - echo - make-a-pr - cleanbranch="" - exit 0 -fi - -echo -echo "+++ I'm about to do the following to push to GitHub (and I'm assuming ${FORK_REMOTE} is your personal fork):" -echo -echo " git push ${FORK_REMOTE} ${NEWBRANCHUNIQ}:${NEWBRANCH}" -echo -read -p "+++ Proceed (anything but 'y' aborts the cherry-pick)? [y/n] " -r -if ! [[ "${REPLY}" =~ ^[yY]$ ]]; then - echo "Aborting." >&2 - exit 1 -fi - -git push "${FORK_REMOTE}" -f "${NEWBRANCHUNIQ}:${NEWBRANCH}" -make-a-pr \ No newline at end of file diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go index bfdc1081b2dd..c2500205d532 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go @@ -96,7 +96,7 @@ type AutoscalingGceClient interface { FetchAllMigs(zone string) ([]*gce.InstanceGroupManager, error) FetchMigTargetSize(GceRef) (int64, error) FetchMigBasename(GceRef) (string, error) - FetchMigInstances(GceRef) ([]GceInstance, error) + FetchMigInstances(GceRef) ([]cloudprovider.Instance, error) FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) FetchMigsWithName(zone string, filter *regexp.Regexp) ([]string, error) diff --git a/cluster-autoscaler/cloudprovider/gce/cache.go b/cluster-autoscaler/cloudprovider/gce/cache.go index 400ec090a31e..40a99fdf0e39 100644 --- a/cluster-autoscaler/cloudprovider/gce/cache.go +++ b/cluster-autoscaler/cloudprovider/gce/cache.go @@ -64,7 +64,7 @@ type GceCache struct { // Cache content. migs map[GceRef]Mig - instances map[GceRef][]GceInstance + instances map[GceRef][]cloudprovider.Instance instancesUpdateTime map[GceRef]time.Time instancesToMig map[GceRef]GceRef instancesFromUnknownMig map[GceRef]bool @@ -76,14 +76,13 @@ type GceCache struct { listManagedInstancesResultsCache map[GceRef]string instanceTemplateNameCache map[GceRef]InstanceTemplateName instanceTemplatesCache map[GceRef]*gce.InstanceTemplate - kubeEnvCache map[GceRef]KubeEnv } // NewGceCache creates empty GceCache. func NewGceCache() *GceCache { return &GceCache{ migs: map[GceRef]Mig{}, - instances: map[GceRef][]GceInstance{}, + instances: map[GceRef][]cloudprovider.Instance{}, instancesUpdateTime: map[GceRef]time.Time{}, instancesToMig: map[GceRef]GceRef{}, instancesFromUnknownMig: map[GceRef]bool{}, @@ -94,7 +93,6 @@ func NewGceCache() *GceCache { listManagedInstancesResultsCache: map[GceRef]string{}, instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, - kubeEnvCache: map[GceRef]KubeEnv{}, } } diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go index 6bb102e9216b..65533d02924d 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go @@ -346,7 +346,6 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa migTargetSizeCache: map[GceRef]int64{}, instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, - kubeEnvCache: map[GceRef]KubeEnv{}, migBaseNameCache: map[GceRef]string{}, listManagedInstancesResultsCache: map[GceRef]string{}, } diff --git a/cluster-autoscaler/cloudprovider/gce/kube_env.go b/cluster-autoscaler/cloudprovider/gce/kube_env.go deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index c5f87cdb09e3..849237e3d09d 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -51,7 +51,7 @@ type mockAutoscalingGceClient struct { fetchMigs func(string) ([]*gce.InstanceGroupManager, error) fetchMigTargetSize func(GceRef) (int64, error) fetchMigBasename func(GceRef) (string, error) - fetchMigInstances func(GceRef) ([]GceInstance, error) + fetchMigInstances func(GceRef) ([]cloudprovider.Instance, error) fetchMigTemplateName func(GceRef) (InstanceTemplateName, error) fetchMigTemplate func(GceRef, string, bool) (*gce.InstanceTemplate, error) fetchMachineType func(string, string) (*gce.MachineType, error) @@ -1141,7 +1141,7 @@ func (f *fakeTime) Now() time.Time { func emptyCache() *GceCache { return &GceCache{ migs: map[GceRef]Mig{mig.GceRef(): mig}, - instances: make(map[GceRef][]GceInstance), + instances: make(map[GceRef][]cloudprovider.Instance), instancesUpdateTime: make(map[GceRef]time.Time), migTargetSizeCache: make(map[GceRef]int64), migBaseNameCache: make(map[GceRef]string), From f222733d74dcf6a09e663bc589a24d07b5818c71 Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Fri, 20 Sep 2024 17:15:35 +0000 Subject: [PATCH 13/14] further cleanup --- cluster-autoscaler/cloudprovider/gce/cache.go | 48 +++++++++---------- .../cloudprovider/gce/gce_manager_test.go | 9 ++-- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/cache.go b/cluster-autoscaler/cloudprovider/gce/cache.go index 40a99fdf0e39..63bee6018625 100644 --- a/cluster-autoscaler/cloudprovider/gce/cache.go +++ b/cluster-autoscaler/cloudprovider/gce/cache.go @@ -63,36 +63,34 @@ type GceCache struct { cacheMutex sync.Mutex // Cache content. - migs map[GceRef]Mig - instances map[GceRef][]cloudprovider.Instance - instancesUpdateTime map[GceRef]time.Time - instancesToMig map[GceRef]GceRef - instancesFromUnknownMig map[GceRef]bool - resourceLimiter *cloudprovider.ResourceLimiter - autoscalingOptionsCache map[GceRef]map[string]string - machinesCache map[MachineTypeKey]MachineType - migTargetSizeCache map[GceRef]int64 - migBaseNameCache map[GceRef]string - listManagedInstancesResultsCache map[GceRef]string - instanceTemplateNameCache map[GceRef]InstanceTemplateName - instanceTemplatesCache map[GceRef]*gce.InstanceTemplate + migs map[GceRef]Mig + instances map[GceRef][]cloudprovider.Instance + instancesUpdateTime map[GceRef]time.Time + instancesToMig map[GceRef]GceRef + instancesFromUnknownMig map[GceRef]bool + resourceLimiter *cloudprovider.ResourceLimiter + autoscalingOptionsCache map[GceRef]map[string]string + machinesCache map[MachineTypeKey]MachineType + migTargetSizeCache map[GceRef]int64 + migBaseNameCache map[GceRef]string + instanceTemplateNameCache map[GceRef]InstanceTemplateName + instanceTemplatesCache map[GceRef]*gce.InstanceTemplate } // NewGceCache creates empty GceCache. func NewGceCache() *GceCache { return &GceCache{ - migs: map[GceRef]Mig{}, - instances: map[GceRef][]cloudprovider.Instance{}, - instancesUpdateTime: map[GceRef]time.Time{}, - instancesToMig: map[GceRef]GceRef{}, - instancesFromUnknownMig: map[GceRef]bool{}, - autoscalingOptionsCache: map[GceRef]map[string]string{}, - machinesCache: map[MachineTypeKey]MachineType{}, - migTargetSizeCache: map[GceRef]int64{}, - migBaseNameCache: map[GceRef]string{}, - listManagedInstancesResultsCache: map[GceRef]string{}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, - instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, + migs: map[GceRef]Mig{}, + instances: map[GceRef][]cloudprovider.Instance{}, + instancesUpdateTime: map[GceRef]time.Time{}, + instancesToMig: map[GceRef]GceRef{}, + instancesFromUnknownMig: map[GceRef]bool{}, + autoscalingOptionsCache: map[GceRef]map[string]string{}, + machinesCache: map[MachineTypeKey]MachineType{}, + migTargetSizeCache: map[GceRef]int64{}, + migBaseNameCache: map[GceRef]string{}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, } } diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go index 65533d02924d..d7c70fca2f6b 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go @@ -343,11 +343,10 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa {"us-central1-c", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1}, {"us-central1-f", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1}, }, - migTargetSizeCache: map[GceRef]int64{}, - instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, - instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, - migBaseNameCache: map[GceRef]string{}, - listManagedInstancesResultsCache: map[GceRef]string{}, + migTargetSizeCache: map[GceRef]int64{}, + instanceTemplateNameCache: map[GceRef]InstanceTemplateName{}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, + migBaseNameCache: map[GceRef]string{}, } migLister := NewMigLister(cache) manager := &gceManagerImpl{ From bb3148bdffbd6424c4c6525e4a23aa86e4c443c6 Mon Sep 17 00:00:00 2001 From: Edwinhr716 Date: Fri, 20 Sep 2024 17:22:17 +0000 Subject: [PATCH 14/14] even further cleanup --- .../cloudprovider/gce/mig_info_provider_test.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index 849237e3d09d..c2c0f6f35981 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -1140,15 +1140,14 @@ func (f *fakeTime) Now() time.Time { func emptyCache() *GceCache { return &GceCache{ - migs: map[GceRef]Mig{mig.GceRef(): mig}, - instances: make(map[GceRef][]cloudprovider.Instance), - instancesUpdateTime: make(map[GceRef]time.Time), - migTargetSizeCache: make(map[GceRef]int64), - migBaseNameCache: make(map[GceRef]string), - listManagedInstancesResultsCache: make(map[GceRef]string), - instanceTemplateNameCache: make(map[GceRef]InstanceTemplateName), - instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), - instancesFromUnknownMig: make(map[GceRef]bool), + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instances: make(map[GceRef][]cloudprovider.Instance), + instancesUpdateTime: make(map[GceRef]time.Time), + migTargetSizeCache: make(map[GceRef]int64), + migBaseNameCache: make(map[GceRef]string), + instanceTemplateNameCache: make(map[GceRef]InstanceTemplateName), + instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), + instancesFromUnknownMig: make(map[GceRef]bool), } }