From 1d0c237adce970bf1a291bc07cfb77b9908294b8 Mon Sep 17 00:00:00 2001 From: Krzysztof Jastrzebski Date: Thu, 14 Sep 2017 08:30:35 +0200 Subject: [PATCH] Cloudprovider/gce/gce_cloud_provider.go unit tests. --- .../builder/cloud_provider_builder.go | 2 +- .../cloudprovider/gce/gce_cloud_provider.go | 22 +- .../gce/gce_cloud_provider_test.go | 470 +++++++++++++++++- .../cloudprovider/gce/gce_manager.go | 79 ++- .../cloudprovider/gce/gce_manager_test.go | 4 +- 5 files changed, 540 insertions(+), 37 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go b/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go index fb5601c570..79f64e0ea1 100644 --- a/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go +++ b/cluster-autoscaler/cloudprovider/builder/cloud_provider_builder.go @@ -58,7 +58,7 @@ func (b CloudProviderBuilder) Build(discoveryOpts cloudprovider.NodeGroupDiscove if b.cloudProviderFlag == "gce" || b.cloudProviderFlag == "gke" { // GCE Manager - var gceManager *gce.GceManager + var gceManager gce.GceManager var gceError error mode := gce.ModeGCE if b.cloudProviderFlag == "gke" { diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go index 71ca5d657c..bbeb7064a6 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go @@ -60,12 +60,12 @@ var autoprovisionedMachineTypes = []string{ // GceCloudProvider implements CloudProvider interface. type GceCloudProvider struct { - gceManager *GceManager + gceManager GceManager } // BuildGceCloudProvider builds CloudProvider implementation for GCE. -func BuildGceCloudProvider(gceManager *GceManager, specs []string) (*GceCloudProvider, error) { - if gceManager.mode == ModeGKE && len(specs) != 0 { +func BuildGceCloudProvider(gceManager GceManager, specs []string) (*GceCloudProvider, error) { + if gceManager.getMode() == ModeGKE && len(specs) != 0 { return nil, fmt.Errorf("GKE gets nodegroup specification via API, command line specs are not allowed") } @@ -135,8 +135,8 @@ func (gce *GceCloudProvider) NewNodeGroup(machineType string, labels map[string] exist: false, nodePoolName: nodePoolName, GceRef: GceRef{ - Project: gce.gceManager.projectId, - Zone: gce.gceManager.zone, + Project: gce.gceManager.getProjectId(), + Zone: gce.gceManager.getZone(), Name: nodePoolName + "-temporary-mig", }, minSize: minAutoprovisionedSize, @@ -148,7 +148,7 @@ func (gce *GceCloudProvider) NewNodeGroup(machineType string, labels map[string] }, gceManager: gce.gceManager, } - _, err := gce.gceManager.templates.buildNodeFromAutoprovisioningSpec(mig) + _, err := gce.gceManager.getTemplates().buildNodeFromAutoprovisioningSpec(mig) if err != nil { return nil, err } @@ -189,7 +189,7 @@ type autoprovisioningSpec struct { type Mig struct { GceRef - gceManager *GceManager + gceManager GceManager minSize int maxSize int autoprovisioned bool @@ -346,17 +346,17 @@ func (mig *Mig) Autoprovisioned() bool { func (mig *Mig) TemplateNodeInfo() (*schedulercache.NodeInfo, error) { var node *apiv1.Node if mig.Exist() { - template, err := mig.gceManager.templates.getMigTemplate(mig) + template, err := mig.gceManager.getTemplates().getMigTemplate(mig) if err != nil { return nil, err } - node, err = mig.gceManager.templates.buildNodeFromTemplate(mig, template) + node, err = mig.gceManager.getTemplates().buildNodeFromTemplate(mig, template) if err != nil { return nil, err } } else if mig.Autoprovisioned() { var err error - node, err = mig.gceManager.templates.buildNodeFromAutoprovisioningSpec(mig) + node, err = mig.gceManager.getTemplates().buildNodeFromAutoprovisioningSpec(mig) if err != nil { return nil, err } @@ -368,7 +368,7 @@ func (mig *Mig) TemplateNodeInfo() (*schedulercache.NodeInfo, error) { return nodeInfo, nil } -func buildMig(value string, gceManager *GceManager) (*Mig, error) { +func buildMig(value string, gceManager GceManager) (*Mig, error) { spec, err := dynamic.SpecFromString(value, true) if err != nil { diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go index f464727950..b8a76095f4 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go @@ -17,13 +17,479 @@ limitations under the License. package gce import ( + "net/http" + "reflect" "testing" - "github.com/stretchr/testify/assert" - apiv1 "k8s.io/api/core/v1" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" + . "k8s.io/autoscaler/cluster-autoscaler/utils/test" + + apiv1 "k8s.io/api/core/v1" + + "strings" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + gcev1 "google.golang.org/api/compute/v1" ) +type gceManagerMock struct { + mock.Mock +} + +func (m *gceManagerMock) RegisterMig(mig *Mig) bool { + args := m.Called(mig) + return args.Bool(0) +} + +func (m *gceManagerMock) UnregisterMig(toBeRemoved *Mig) bool { + args := m.Called(toBeRemoved) + return args.Bool(0) +} + +func (m *gceManagerMock) GetMigSize(mig *Mig) (int64, error) { + args := m.Called(mig) + return args.Get(0).(int64), args.Error(1) +} + +func (m *gceManagerMock) SetMigSize(mig *Mig, size int64) error { + args := m.Called(mig, size) + return args.Error(0) +} + +func (m *gceManagerMock) DeleteInstances(instances []*GceRef) error { + args := m.Called(instances) + return args.Error(0) +} + +func (m *gceManagerMock) GetMigForInstance(instance *GceRef) (*Mig, error) { + args := m.Called(instance) + return args.Get(0).(*Mig), args.Error(1) +} + +func (m *gceManagerMock) GetMigNodes(mig *Mig) ([]string, error) { + args := m.Called(mig) + return args.Get(0).([]string), args.Error(1) +} + +func (m *gceManagerMock) getMigs() []*migInformation { + args := m.Called() + return args.Get(0).([]*migInformation) +} + +func (m *gceManagerMock) createNodePool(mig *Mig) error { + args := m.Called(mig) + return args.Error(0) +} + +func (m *gceManagerMock) deleteNodePool(toBeRemoved *Mig) error { + args := m.Called(toBeRemoved) + return args.Error(0) +} + +func (m *gceManagerMock) getZone() string { + args := m.Called() + return args.String(0) +} + +func (m *gceManagerMock) getProjectId() string { + args := m.Called() + return args.String(0) +} + +func (m *gceManagerMock) getMode() GcpCloudProviderMode { + args := m.Called() + return args.Get(0).(GcpCloudProviderMode) +} + +func (m *gceManagerMock) getTemplates() *templateBuilder { + args := m.Called() + return args.Get(0).(*templateBuilder) +} + +func TestBuildGceCloudProvider(t *testing.T) { + gceManagerMock := &gceManagerMock{} + + ng1Name := "https://content.googleapis.com/compute/v1/projects/project1/zones/us-central1-b/instanceGroups/ng1" + ng2Name := "https://content.googleapis.com/compute/v1/projects/project1/zones/us-central1-b/instanceGroups/ng2" + + // GCE mode. + gceManagerMock.On("getMode").Return(ModeGCE).Once() + gceManagerMock.On("RegisterMig", + mock.MatchedBy(func(mig *Mig) bool { + return mig.Name == "ng1" || mig.Name == "ng2" + })).Return(true).Times(2) + + provider, err := BuildGceCloudProvider(gceManagerMock, + []string{"0:10:" + ng1Name, "0:5:https:" + ng2Name}) + assert.NoError(t, err) + assert.NotNil(t, provider) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // GKE mode. + gceManagerMock.On("getMode").Return(ModeGKE).Once() + + provider, err = BuildGceCloudProvider(gceManagerMock, []string{}) + assert.NoError(t, err) + assert.NotNil(t, provider) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Error on GKE mode with specs. + gceManagerMock.On("getMode").Return(ModeGKE).Once() + + provider, err = BuildGceCloudProvider(gceManagerMock, + []string{"0:10:" + ng1Name, "0:5:https:" + ng2Name}) + assert.Error(t, err) + assert.Equal(t, "GKE gets nodegroup specification via API, command line specs are not allowed", err.Error()) + mock.AssertExpectationsForObjects(t, gceManagerMock) +} + +func TestNodeGroups(t *testing.T) { + gceManagerMock := &gceManagerMock{} + gce := &GceCloudProvider{ + gceManager: gceManagerMock, + } + mig := &migInformation{config: &Mig{GceRef: GceRef{Name: "ng1"}}} + gceManagerMock.On("getMigs").Return([]*migInformation{mig}).Once() + result := gce.NodeGroups() + assert.Equal(t, []cloudprovider.NodeGroup{mig.config}, result) + mock.AssertExpectationsForObjects(t, gceManagerMock) +} + +func TestNodeGroupForNode(t *testing.T) { + gceManagerMock := &gceManagerMock{} + gce := &GceCloudProvider{ + gceManager: gceManagerMock, + } + n := BuildTestNode("n1", 1000, 1000) + n.Spec.ProviderID = "gce://project1/us-central1-b/n1" + mig := Mig{GceRef: GceRef{Name: "ng1"}} + gceManagerMock.On("GetMigForInstance", mock.AnythingOfType("*gce.GceRef")).Return(&mig, nil).Once() + + nodeGroup, err := gce.NodeGroupForNode(n) + assert.NoError(t, err) + assert.Equal(t, mig, *reflect.ValueOf(nodeGroup).Interface().(*Mig)) + mock.AssertExpectationsForObjects(t, gceManagerMock) +} + +const getMachineTypeResponse = `{ + "kind": "compute#machineType", + "id": "3001", + "creationTimestamp": "2015-01-16T09:25:43.314-08:00", + "name": "n1-standard-1", + "description": "1 vCPU, 3.75 GB RAM", + "guestCpus": 1, + "memoryMb": 3840, + "maximumPersistentDisks": 32, + "maximumPersistentDisksSizeGb": "65536", + "zone": "us-central1-a", + "selfLink": "https://www.googleapis.com/compute/v1/projects/krzysztof-jastrzebski-dev/zones/us-central1-a/machineTypes/n1-standard-1", + "isSharedCpu": false +}` + +const getInstanceGroupManagerResponse = `{ + "kind": "compute#instanceGroupManager", + "id": "3213213219", + "creationTimestamp": "2017-09-15T04:47:24.687-07:00", + "name": "gke-cluster-1-default-pool", + "zone": "https://www.googleapis.com/compute/v1/projects/project1/zones/us-central1-b", + "instanceTemplate": "https://www.googleapis.com/compute/v1/projects/project1/global/instanceTemplates/gke-cluster-1-default-pool", + "instanceGroup": "https://www.googleapis.com/compute/v1/projects/project1/zones/us-central1-b/instanceGroups/gke-cluster-1-default-pool", + "baseInstanceName": "gke-cluster-1-default-pool-f23aac-grp", + "fingerprint": "kfdsuH", + "currentActions": { + "none": 3, + "creating": 0, + "creatingWithoutRetries": 0, + "recreating": 0, + "deleting": 0, + "abandoning": 0, + "restarting": 0, + "refreshing": 0 + }, + "targetSize": 3, + "selfLink": "https://www.googleapis.com/compute/v1/projects/project1/zones/us-central1-b/instanceGroupManagers/gke-cluster-1-default-pool" +}` + +const getInstanceTemplateResponse = `{ + "kind": "compute#instanceTemplate", + "id": "28701103232323232", + "creationTimestamp": "2017-09-15T04:47:21.577-07:00", + "name": "gke-cluster-1-default-pool", + "description": "", + "properties": { + "tags": { + "items": [ + "gke-cluster-1-fc0afeeb-node" + ] + }, + "machineType": "n1-standard-1", + "canIpForward": true, + "networkInterfaces": [ + { + "kind": "compute#networkInterface", + "network": "https://www.googleapis.com/compute/v1/projects/project1/global/networks/default", + "subnetwork": "https://www.googleapis.com/compute/v1/projects/project1/regions/us-central1/subnetworks/default", + "accessConfigs": [ + { + "kind": "compute#accessConfig", + "type": "ONE_TO_ONE_NAT", + "name": "external-nat" + } + ] + } + ], + "disks": [ + { + "kind": "compute#attachedDisk", + "type": "PERSISTENT", + "mode": "READ_WRITE", + "boot": true, + "initializeParams": { + "sourceImage": "https://www.googleapis.com/compute/v1/projects/gke-node-images/global/images/cos-stable-60-9592-84-0", + "diskSizeGb": "100", + "diskType": "pd-standard" + }, + "autoDelete": true + } + ], + "metadata": { + "kind": "compute#metadata", + "fingerprint": "F7n_RsHD3ng=", + "items": [ + { + "key": "kube-env", + "value": "ALLOCATE_NODE_CIDRS: \"true\"\n" + }, + { + "key": "user-data", + "value": "#cloud-config\n\nwrite_files:\n - path: /etc/systemd/system/kube-node-installation.service\n " + }, + { + "key": "gci-update-strategy", + "value": "update_disabled" + }, + { + "key": "gci-ensure-gke-docker", + "value": "true" + }, + { + "key": "configure-sh", + "value": "#!/bin/bash\n\n# Copyright 2016 The Kubernetes Authors.\n#\n# Licensed under the Apache License, " + }, + { + "key": "cluster-name", + "value": "cluster-1" + } + ] + }, + "serviceAccounts": [ + { + "email": "default", + "scopes": [ + "https://www.googleapis.com/auth/compute", + "https://www.googleapis.com/auth/devstorage.read_only", + "https://www.googleapis.com/auth/logging.write", + "https://www.googleapis.com/auth/monitoring.write", + "https://www.googleapis.com/auth/servicecontrol", + "https://www.googleapis.com/auth/service.management.readonly", + "https://www.googleapis.com/auth/trace.append" + ] + } + ], + "scheduling": { + "onHostMaintenance": "MIGRATE", + "automaticRestart": true, + "preemptible": false + } + }, + "selfLink": "https://www.googleapis.com/compute/v1/projects/project1/global/instanceTemplates/gke-cluster-1-default-pool-f7607aac" +}` + +func TestMig(t *testing.T) { + server := NewHttpServerMock() + defer server.Close() + gceManagerMock := &gceManagerMock{} + client := &http.Client{} + gceService, err := gcev1.New(client) + assert.NoError(t, err) + gceService.BasePath = server.URL + templateBuilder := &templateBuilder{gceService, "us-central1-b", "project1"} + gce := &GceCloudProvider{ + gceManager: gceManagerMock, + } + + // Test NewNodeGroup. + gceManagerMock.On("getProjectId").Return("project1").Once() + gceManagerMock.On("getZone").Return("us-central1-b").Once() + gceManagerMock.On("getTemplates").Return(templateBuilder).Once() + server.On("handle", "/project1/zones/us-central1-b/machineTypes/n1-standard-1").Return(getMachineTypeResponse).Once() + nodeGroup, err := gce.NewNodeGroup("n1-standard-1", nil, nil) + assert.NoError(t, err) + assert.NotNil(t, nodeGroup) + mig1 := reflect.ValueOf(nodeGroup).Interface().(*Mig) + assert.True(t, strings.HasPrefix(mig1.Id(), "https://content.googleapis.com/compute/v1/projects/project1/zones/us-central1-b/instanceGroups/nodeautoprovisioning-n1-standard-1")) + assert.Equal(t, true, mig1.Autoprovisioned()) + assert.Equal(t, 0, mig1.MinSize()) + assert.Equal(t, 1000, mig1.MaxSize()) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test TargetSize. + gceManagerMock.On("GetMigSize", mock.AnythingOfType("*gce.Mig")).Return(int64(2), nil).Once() + targetSize, err := mig1.TargetSize() + assert.NoError(t, err) + assert.Equal(t, 2, targetSize) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test IncreaseSize. + gceManagerMock.On("GetMigSize", mock.AnythingOfType("*gce.Mig")).Return(int64(2), nil).Once() + gceManagerMock.On("SetMigSize", mock.AnythingOfType("*gce.Mig"), int64(3)).Return(nil).Once() + err = mig1.IncreaseSize(1) + assert.NoError(t, err) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test IncreaseSize - fail on wrong size. + err = mig1.IncreaseSize(0) + assert.Error(t, err) + assert.Equal(t, "size increase must be positive", err.Error()) + + // Test IncreaseSize - fail on too big delta. + gceManagerMock.On("GetMigSize", mock.AnythingOfType("*gce.Mig")).Return(int64(2), nil).Once() + err = mig1.IncreaseSize(1000) + assert.Error(t, err) + assert.Equal(t, "size increase too large - desired:1002 max:1000", err.Error()) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test DecreaseTargetSize. + gceManagerMock.On("GetMigSize", mock.AnythingOfType("*gce.Mig")).Return(int64(3), nil).Once() + gceManagerMock.On("GetMigNodes", mock.AnythingOfType("*gce.Mig")).Return( + []string{"gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-9j4g", + "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-dck1"}, nil).Once() + gceManagerMock.On("SetMigSize", mock.AnythingOfType("*gce.Mig"), int64(2)).Return(nil).Once() + err = mig1.DecreaseTargetSize(-1) + assert.NoError(t, err) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test DecreaseTargetSize - fail on positive delta. + err = mig1.DecreaseTargetSize(1) + assert.Error(t, err) + assert.Equal(t, "size decrease must be negative", err.Error()) + + // Test DecreaseTargetSize - fail on deleting existing nodes. + gceManagerMock.On("GetMigSize", mock.AnythingOfType("*gce.Mig")).Return(int64(3), nil).Once() + gceManagerMock.On("GetMigNodes", mock.AnythingOfType("*gce.Mig")).Return( + []string{"gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-9j4g", + "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-dck1"}, nil).Once() + + err = mig1.DecreaseTargetSize(-2) + assert.Error(t, err) + assert.Equal(t, "attempt to delete existing nodes targetSize:3 delta:-2 existingNodes: 2", err.Error()) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test Belongs - true. + gceManagerMock.On("GetMigForInstance", mock.AnythingOfType("*gce.GceRef")).Return(mig1, nil).Once() + node := BuildTestNode("gke-cluster-1-default-pool-f7607aac-dck1", 1000, 1000) + node.Spec.ProviderID = "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-dck1" + + belongs, err := mig1.Belongs(node) + assert.NoError(t, err) + assert.True(t, belongs) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test Belongs - false. + mig2 := &Mig{ + GceRef: GceRef{ + Project: "project1", + Zone: "us-central1-b", + Name: "default-pool", + }, + gceManager: gceManagerMock, + minSize: 0, + maxSize: 1000, + autoprovisioned: true, + exist: true, + nodePoolName: "default-pool", + spec: nil} + gceManagerMock.On("GetMigForInstance", mock.AnythingOfType("*gce.GceRef")).Return(mig2, nil).Once() + + belongs, err = mig1.Belongs(node) + assert.NoError(t, err) + assert.False(t, belongs) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test DeleteNodes. + n1 := BuildTestNode("gke-cluster-1-default-pool-f7607aac-9j4g", 1000, 1000) + n1.Spec.ProviderID = "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-9j4g" + n1ref := &GceRef{"project1", "us-central1-b", "gke-cluster-1-default-pool-f7607aac-9j4g"} + n2 := BuildTestNode("gke-cluster-1-default-pool-f7607aac-dck1", 1000, 1000) + n2.Spec.ProviderID = "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-dck1" + n2ref := &GceRef{"project1", "us-central1-b", "gke-cluster-1-default-pool-f7607aac-dck1"} + gceManagerMock.On("GetMigSize", mock.AnythingOfType("*gce.Mig")).Return(int64(2), nil).Once() + gceManagerMock.On("GetMigForInstance", n1ref).Return(mig1, nil).Once() + gceManagerMock.On("GetMigForInstance", n2ref).Return(mig1, nil).Once() + gceManagerMock.On("DeleteInstances", []*GceRef{n1ref, n2ref}).Return(nil).Once() + err = mig1.DeleteNodes([]*apiv1.Node{n1, n2}) + assert.NoError(t, err) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test DeleteNodes - fail on reaching min size. + gceManagerMock.On("GetMigSize", mock.AnythingOfType("*gce.Mig")).Return(int64(0), nil).Once() + err = mig1.DeleteNodes([]*apiv1.Node{n1, n2}) + assert.Error(t, err) + assert.Equal(t, "min size reached, nodes will not be deleted", err.Error()) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test Nodes. + gceManagerMock.On("GetMigNodes", mock.AnythingOfType("*gce.Mig")).Return( + []string{"gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-9j4g", + "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-dck1"}, nil).Once() + nodes, err := mig1.Nodes() + assert.NoError(t, err) + assert.Equal(t, "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-9j4g", nodes[0]) + assert.Equal(t, "gce://project1/us-central1-b/gke-cluster-1-default-pool-f7607aac-dck1", nodes[1]) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test Create. + gceManagerMock.On("createNodePool", mock.AnythingOfType("*gce.Mig")).Return(nil).Once() + err = mig1.Create() + assert.NoError(t, err) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test Delete. + mig1.exist = true + gceManagerMock.On("deleteNodePool", mock.AnythingOfType("*gce.Mig")).Return(nil).Once() + err = mig1.Delete() + assert.NoError(t, err) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test TemplateNodeInfo. + gceManagerMock.On("getTemplates").Return(templateBuilder).Times(2) + server.On("handle", "/project1/zones/us-central1-b/instanceGroupManagers/default-pool").Return(getInstanceGroupManagerResponse).Once() + server.On("handle", "/project1/global/instanceTemplates/gke-cluster-1-default-pool").Return(getInstanceTemplateResponse).Once() + server.On("handle", "/project1/zones/us-central1-b/machineTypes/n1-standard-1").Return(getMachineTypeResponse).Once() + templateNodeInfo, err := mig2.TemplateNodeInfo() + assert.NoError(t, err) + assert.NotNil(t, templateNodeInfo) + assert.NotNil(t, templateNodeInfo.Node()) + mock.AssertExpectationsForObjects(t, gceManagerMock) + + // Test TemplateNodeInfo for non-existing autoprovisioned Mig. + gceManagerMock.On("getTemplates").Return(templateBuilder).Once() + server.On("handle", "/project1/zones/us-central1-b/machineTypes/n1-standard-1").Return(getMachineTypeResponse).Once() + mig1.exist = false + templateNodeInfo, err = mig1.TemplateNodeInfo() + assert.NoError(t, err) + assert.NotNil(t, templateNodeInfo) + assert.NotNil(t, templateNodeInfo.Node()) + mock.AssertExpectationsForObjects(t, gceManagerMock) +} + +func TestGceRefFromProviderId(t *testing.T) { + ref, err := GceRefFromProviderId("gce://project1/us-central1-b/name1") + assert.NoError(t, err) + assert.Equal(t, GceRef{"project1", "us-central1-b", "name1"}, *ref) +} + func TestBuildMig(t *testing.T) { _, err := buildMig("a", nil) assert.Error(t, err) diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager.go b/cluster-autoscaler/cloudprovider/gce/gce_manager.go index 13278926c1..f113247449 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager.go @@ -66,8 +66,33 @@ type migInformation struct { basename string } -// GceManager is handles gce communication and data caching. -type GceManager struct { +// GceManager handles gce communication and data caching. +type GceManager interface { + // RegisterMig registers mig in Gce Manager. Returns true if the node group didn't exist before. + RegisterMig(mig *Mig) bool + // UnregisterMig unregisters mig in Gce Manager. Returns true if the node group has been removed. + UnregisterMig(toBeRemoved *Mig) bool + // GetMigSize gets MIG size. + GetMigSize(mig *Mig) (int64, error) + // SetMigSize sets MIG size. + SetMigSize(mig *Mig, size int64) error + // DeleteInstances deletes the given instances. All instances must be controlled by the same MIG. + DeleteInstances(instances []*GceRef) error + // GetMigForInstance returns MigConfig of the given Instance + GetMigForInstance(instance *GceRef) (*Mig, error) + // GetMigNodes returns mig nodes. + GetMigNodes(mig *Mig) ([]string, error) + getMigs() []*migInformation + createNodePool(mig *Mig) error + deleteNodePool(toBeRemoved *Mig) error + getZone() string + getProjectId() string + getMode() GcpCloudProviderMode + getTemplates() *templateBuilder +} + +// gceManagerImpl handles gce communication and data caching. +type gceManagerImpl struct { migs []*migInformation migCache map[GceRef]*Mig @@ -85,7 +110,7 @@ type GceManager struct { } // CreateGceManager constructs gceManager object. -func CreateGceManager(configReader io.Reader, mode GcpCloudProviderMode, clusterName string) (*GceManager, error) { +func CreateGceManager(configReader io.Reader, mode GcpCloudProviderMode, clusterName string) (GceManager, error) { // Create Google Compute Engine token. var err error tokenSource := google.ComputeTokenSource("") @@ -127,7 +152,7 @@ func CreateGceManager(configReader io.Reader, mode GcpCloudProviderMode, cluster if err != nil { return nil, err } - manager := &GceManager{ + manager := &gceManagerImpl{ migs: make([]*migInformation, 0), gceService: gceService, migCache: make(map[GceRef]*Mig), @@ -166,14 +191,14 @@ func CreateGceManager(configReader io.Reader, mode GcpCloudProviderMode, cluster return manager, nil } -func (m *GceManager) assertGKE() { +func (m *gceManagerImpl) assertGKE() { if m.mode != ModeGKE { glog.Fatalf("This should run only in GKE mode") } } // Gets all registered node pools -func (m *GceManager) fetchAllNodePools() error { +func (m *gceManagerImpl) fetchAllNodePools() error { m.assertGKE() nodePoolsResponse, err := m.gkeService.Projects.Zones.Clusters.NodePools.List(m.projectId, m.zone, m.clusterName).Do() @@ -228,7 +253,7 @@ func (m *GceManager) fetchAllNodePools() error { } // RegisterMig registers mig in Gce Manager. Returns true if the node group didn't exist before. -func (m *GceManager) RegisterMig(mig *Mig) bool { +func (m *gceManagerImpl) RegisterMig(mig *Mig) bool { m.migsMutex.Lock() defer m.migsMutex.Unlock() @@ -261,7 +286,7 @@ func (m *GceManager) RegisterMig(mig *Mig) bool { } // UnregisterMig unregisters mig in Gce Manager. Returns true if the node group has been removed. -func (m *GceManager) UnregisterMig(toBeRemoved *Mig) bool { +func (m *gceManagerImpl) UnregisterMig(toBeRemoved *Mig) bool { m.migsMutex.Lock() defer m.migsMutex.Unlock() @@ -280,7 +305,7 @@ func (m *GceManager) UnregisterMig(toBeRemoved *Mig) bool { return found } -func (m *GceManager) deleteNodePool(toBeRemoved *Mig) error { +func (m *gceManagerImpl) deleteNodePool(toBeRemoved *Mig) error { m.assertGKE() if !toBeRemoved.Autoprovisioned() { return fmt.Errorf("only autoprovisioned node pools can be deleted") @@ -298,7 +323,7 @@ func (m *GceManager) deleteNodePool(toBeRemoved *Mig) error { return m.fetchAllNodePools() } -func (m *GceManager) createNodePool(mig *Mig) error { +func (m *gceManagerImpl) createNodePool(mig *Mig) error { m.assertGKE() // TODO: handle preemptable @@ -342,7 +367,7 @@ func (m *GceManager) createNodePool(mig *Mig) error { } // GetMigSize gets MIG size. -func (m *GceManager) GetMigSize(mig *Mig) (int64, error) { +func (m *gceManagerImpl) GetMigSize(mig *Mig) (int64, error) { igm, err := m.gceService.InstanceGroupManagers.Get(mig.Project, mig.Zone, mig.Name).Do() if err != nil { return -1, err @@ -351,7 +376,7 @@ func (m *GceManager) GetMigSize(mig *Mig) (int64, error) { } // SetMigSize sets MIG size. -func (m *GceManager) SetMigSize(mig *Mig, size int64) error { +func (m *gceManagerImpl) SetMigSize(mig *Mig, size int64) error { glog.V(0).Infof("Setting mig size %s to %d", mig.Id(), size) op, err := m.gceService.InstanceGroupManagers.Resize(mig.Project, mig.Zone, mig.Name, size).Do() if err != nil { @@ -361,7 +386,7 @@ func (m *GceManager) SetMigSize(mig *Mig, size int64) error { } // GCE -func (m *GceManager) waitForOp(operation *gce.Operation, project string, zone string) error { +func (m *gceManagerImpl) waitForOp(operation *gce.Operation, project string, zone string) error { for start := time.Now(); time.Since(start) < operationWaitTimeout; time.Sleep(operationPollInterval) { glog.V(4).Infof("Waiting for operation %s %s %s", project, zone, operation.Name) if op, err := m.gceService.ZoneOperations.Get(project, zone, operation.Name).Do(); err == nil { @@ -377,7 +402,7 @@ func (m *GceManager) waitForOp(operation *gce.Operation, project string, zone st } // GKE -func (m *GceManager) waitForGkeOp(operation *gke.Operation) error { +func (m *gceManagerImpl) waitForGkeOp(operation *gke.Operation) error { for start := time.Now(); time.Since(start) < operationWaitTimeout; time.Sleep(operationPollInterval) { glog.V(4).Infof("Waiting for operation %s %s %s", m.projectId, m.zone, operation.Name) if op, err := m.gkeService.Projects.Zones.Operations.Get(m.projectId, m.zone, operation.Name).Do(); err == nil { @@ -393,7 +418,7 @@ func (m *GceManager) waitForGkeOp(operation *gke.Operation) error { } // DeleteInstances deletes the given instances. All instances must be controlled by the same MIG. -func (m *GceManager) DeleteInstances(instances []*GceRef) error { +func (m *gceManagerImpl) DeleteInstances(instances []*GceRef) error { if len(instances) == 0 { return nil } @@ -425,7 +450,7 @@ func (m *GceManager) DeleteInstances(instances []*GceRef) error { return m.waitForOp(op, commonMig.Project, commonMig.Zone) } -func (m *GceManager) getMigs() []*migInformation { +func (m *gceManagerImpl) getMigs() []*migInformation { m.migsMutex.Lock() defer m.migsMutex.Unlock() migs := make([]*migInformation, 0, len(m.migs)) @@ -437,8 +462,7 @@ func (m *GceManager) getMigs() []*migInformation { } return migs } - -func (m *GceManager) updateMigBasename(ref GceRef, basename string) { +func (m *gceManagerImpl) updateMigBasename(ref GceRef, basename string) { m.migsMutex.Lock() defer m.migsMutex.Unlock() for _, mig := range m.migs { @@ -449,7 +473,7 @@ func (m *GceManager) updateMigBasename(ref GceRef, basename string) { } // GetMigForInstance returns MigConfig of the given Instance -func (m *GceManager) GetMigForInstance(instance *GceRef) (*Mig, error) { +func (m *gceManagerImpl) GetMigForInstance(instance *GceRef) (*Mig, error) { m.cacheMutex.Lock() defer m.cacheMutex.Unlock() if mig, found := m.migCache[*instance]; found { @@ -473,7 +497,7 @@ func (m *GceManager) GetMigForInstance(instance *GceRef) (*Mig, error) { return nil, nil } -func (m *GceManager) regenerateCache() error { +func (m *gceManagerImpl) regenerateCache() error { newMigCache := make(map[GceRef]*Mig) for _, migInfo := range m.getMigs() { @@ -505,7 +529,7 @@ func (m *GceManager) regenerateCache() error { } // GetMigNodes returns mig nodes. -func (m *GceManager) GetMigNodes(mig *Mig) ([]string, error) { +func (m *gceManagerImpl) GetMigNodes(mig *Mig) ([]string, error) { instances, err := m.gceService.InstanceGroupManagers.ListManagedInstances(mig.Project, mig.Zone, mig.Name).Do() if err != nil { return []string{}, err @@ -521,6 +545,19 @@ func (m *GceManager) GetMigNodes(mig *Mig) ([]string, error) { return result, nil } +func (m *gceManagerImpl) getZone() string { + return m.zone +} +func (m *gceManagerImpl) getProjectId() string { + return m.projectId +} +func (m *gceManagerImpl) getMode() GcpCloudProviderMode { + return m.mode +} +func (m *gceManagerImpl) getTemplates() *templateBuilder { + return m.templates +} + // Code borrowed from gce cloud provider. Reuse the original as soon as it becomes public. func getProjectAndZone() (string, string, error) { result, err := metadata.Get("instance/zone") diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go index a679c047aa..0cddbf13e1 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go @@ -336,12 +336,12 @@ const managedInstancesResponse2 = `{ ] }` -func newTestGceManager(t *testing.T, testServerURL string, mode GcpCloudProviderMode) *GceManager { +func newTestGceManager(t *testing.T, testServerURL string, mode GcpCloudProviderMode) *gceManagerImpl { client := &http.Client{} gceService, err := gce.New(client) assert.NoError(t, err) gceService.BasePath = testServerURL - manager := &GceManager{ + manager := &gceManagerImpl{ migs: make([]*migInformation, 0), gceService: gceService, migCache: make(map[GceRef]*Mig),