Make Create() return newly created node group

This commit is contained in:
Aleksandra Malinowska 2018-08-06 11:57:12 +02:00
parent 4df8fe53d8
commit 90b67feff2
13 changed files with 39 additions and 36 deletions

View File

@ -179,8 +179,8 @@ func (ng *AwsNodeGroup) Exist() bool {
} }
// Create creates the node group on the cloud provider side. // Create creates the node group on the cloud provider side.
func (ng *AwsNodeGroup) Create() error { func (ng *AwsNodeGroup) Create() (cloudprovider.NodeGroup, error) {
return cloudprovider.ErrAlreadyExist return nil, cloudprovider.ErrAlreadyExist
} }
// Autoprovisioned returns true if the node group is autoprovisioned. // Autoprovisioned returns true if the node group is autoprovisioned.

View File

@ -96,8 +96,8 @@ func (as *AgentPool) Exist() bool {
} }
// Create creates the node group on the cloud provider side. // Create creates the node group on the cloud provider side.
func (as *AgentPool) Create() error { func (as *AgentPool) Create() (cloudprovider.NodeGroup, error) {
return cloudprovider.ErrAlreadyExist return nil, cloudprovider.ErrAlreadyExist
} }
// Delete deletes the node group on the cloud provider side. // Delete deletes the node group on the cloud provider side.

View File

@ -461,8 +461,8 @@ func (agentPool *ContainerServiceAgentPool) Exist() bool {
//Create is returns already exists since we don't support the //Create is returns already exists since we don't support the
//agent pool creation. //agent pool creation.
func (agentPool *ContainerServiceAgentPool) Create() error { func (agentPool *ContainerServiceAgentPool) Create() (cloudprovider.NodeGroup, error) {
return cloudprovider.ErrAlreadyExist return nil, cloudprovider.ErrAlreadyExist
} }
//Delete is not implemented since we don't support agent pool //Delete is not implemented since we don't support agent pool

View File

@ -75,8 +75,8 @@ func (scaleSet *ScaleSet) Exist() bool {
} }
// Create creates the node group on the cloud provider side. // Create creates the node group on the cloud provider side.
func (scaleSet *ScaleSet) Create() error { func (scaleSet *ScaleSet) Create() (cloudprovider.NodeGroup, error) {
return cloudprovider.ErrAlreadyExist return nil, cloudprovider.ErrAlreadyExist
} }
// Delete deletes the node group on the cloud provider side. // Delete deletes the node group on the cloud provider side.

View File

@ -132,7 +132,7 @@ type NodeGroup interface {
Exist() bool Exist() bool
// Create creates the node group on the cloud provider side. Implementation optional. // Create creates the node group on the cloud provider side. Implementation optional.
Create() error Create() (NodeGroup, error)
// Delete deletes the node group on the cloud provider side. // Delete deletes the node group on the cloud provider side.
// This will be executed only for autoprovisioned node groups, once their size drops to 0. // This will be executed only for autoprovisioned node groups, once their size drops to 0.

View File

@ -379,11 +379,11 @@ func (mig *Mig) Exist() bool {
} }
// Create creates the node group on the cloud provider side. // Create creates the node group on the cloud provider side.
func (mig *Mig) Create() error { func (mig *Mig) Create() (cloudprovider.NodeGroup, error) {
if !mig.exist && mig.autoprovisioned { if !mig.exist && mig.autoprovisioned {
return mig.gceManager.createNodePool(mig) return mig.gceManager.createNodePool(mig)
} }
return fmt.Errorf("Cannot create non-autoprovisioned node group") return nil, fmt.Errorf("Cannot create non-autoprovisioned node group")
} }
// Delete deletes the node group on the cloud provider side. // Delete deletes the node group on the cloud provider side.

View File

@ -90,9 +90,9 @@ func (m *gceManagerMock) getMigs() []*migInformation {
return args.Get(0).([]*migInformation) return args.Get(0).([]*migInformation)
} }
func (m *gceManagerMock) createNodePool(mig *Mig) error { func (m *gceManagerMock) createNodePool(mig *Mig) (*Mig, error) {
args := m.Called(mig) args := m.Called(mig)
return args.Error(0) return mig, args.Error(0)
} }
func (m *gceManagerMock) deleteNodePool(toBeRemoved *Mig) error { func (m *gceManagerMock) deleteNodePool(toBeRemoved *Mig) error {
@ -430,7 +430,7 @@ func TestMig(t *testing.T) {
// Test Create. // Test Create.
mig1.exist = false mig1.exist = false
gceManagerMock.On("createNodePool", mock.AnythingOfType("*gce.Mig")).Return(nil).Once() gceManagerMock.On("createNodePool", mock.AnythingOfType("*gce.Mig")).Return(nil).Once()
err = mig1.Create() _, err = mig1.Create()
assert.NoError(t, err) assert.NoError(t, err)
mock.AssertExpectationsForObjects(t, gceManagerMock) mock.AssertExpectationsForObjects(t, gceManagerMock)

View File

@ -117,7 +117,7 @@ type GceManager interface {
// Cleanup cleans up open resources before the cloud provider is destroyed, i.e. go routines etc. // Cleanup cleans up open resources before the cloud provider is destroyed, i.e. go routines etc.
Cleanup() error Cleanup() error
getMigs() []*migInformation getMigs() []*migInformation
createNodePool(mig *Mig) error createNodePool(mig *Mig) (*Mig, error)
deleteNodePool(toBeRemoved *Mig) error deleteNodePool(toBeRemoved *Mig) error
getLocation() string getLocation() string
getProjectId() string getProjectId() string
@ -408,24 +408,23 @@ func (m *gceManagerImpl) deleteNodePool(toBeRemoved *Mig) error {
return m.refreshNodePools() return m.refreshNodePools()
} }
func (m *gceManagerImpl) createNodePool(mig *Mig) error { func (m *gceManagerImpl) createNodePool(mig *Mig) (*Mig, error) {
m.assertGKENAP() m.assertGKENAP()
err := m.GkeService.CreateNodePool(mig) err := m.GkeService.CreateNodePool(mig)
if err != nil { if err != nil {
return err return nil, err
} }
err = m.refreshNodePools() err = m.refreshNodePools()
if err != nil { if err != nil {
return err return nil, err
} }
for _, existingMig := range m.getMigs() { for _, existingMig := range m.getMigs() {
if existingMig.config.nodePoolName == mig.nodePoolName { if existingMig.config.nodePoolName == mig.nodePoolName {
*mig = *existingMig.config return existingMig.config, nil
return nil
} }
} }
return fmt.Errorf("node pool %s not found", mig.nodePoolName) return nil, fmt.Errorf("node pool %s not found", mig.nodePoolName)
} }
func (m *gceManagerImpl) fetchMachinesCache() error { func (m *gceManagerImpl) fetchMachinesCache() error {

View File

@ -738,8 +738,9 @@ func TestCreateNodePool(t *testing.T) {
}, },
} }
err := g.createNodePool(mig) newMig, err := g.createNodePool(mig)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, newMig.Exist())
migs := g.getMigs() migs := g.getMigs()
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, 2, len(migs)) assert.Equal(t, 2, len(migs))

View File

@ -250,8 +250,8 @@ func (nodeGroup *NodeGroup) Exist() bool {
} }
// Create creates the node group on the cloud provider side. // Create creates the node group on the cloud provider side.
func (nodeGroup *NodeGroup) Create() error { func (nodeGroup *NodeGroup) Create() (cloudprovider.NodeGroup, error) {
return cloudprovider.ErrNotImplemented return nil, cloudprovider.ErrNotImplemented
} }
// Delete deletes the node group on the cloud provider side. // Delete deletes the node group on the cloud provider side.

View File

@ -169,11 +169,10 @@ func (tcp *TestCloudProvider) AddNodeGroup(id string, min int, max int, size int
} }
// AddAutoprovisionedNodeGroup adds node group to test cloud provider. // AddAutoprovisionedNodeGroup adds node group to test cloud provider.
func (tcp *TestCloudProvider) AddAutoprovisionedNodeGroup(id string, min int, max int, size int, machineType string) { func (tcp *TestCloudProvider) AddAutoprovisionedNodeGroup(id string, min int, max int, size int, machineType string) *TestNodeGroup {
tcp.Lock() tcp.Lock()
defer tcp.Unlock() defer tcp.Unlock()
nodeGroup := &TestNodeGroup{
tcp.groups[id] = &TestNodeGroup{
cloudProvider: tcp, cloudProvider: tcp,
id: id, id: id,
minSize: min, minSize: min,
@ -183,6 +182,8 @@ func (tcp *TestCloudProvider) AddAutoprovisionedNodeGroup(id string, min int, ma
autoprovisioned: true, autoprovisioned: true,
machineType: machineType, machineType: machineType,
} }
tcp.groups[id] = nodeGroup
return nodeGroup
} }
// AddNode adds the given node to the group. // AddNode adds the given node to the group.
@ -282,12 +283,12 @@ func (tng *TestNodeGroup) Exist() bool {
} }
// Create creates the node group on the cloud provider side. // Create creates the node group on the cloud provider side.
func (tng *TestNodeGroup) Create() error { func (tng *TestNodeGroup) Create() (cloudprovider.NodeGroup, error) {
if tng.Exist() { if tng.Exist() {
return fmt.Errorf("Group already exist") return nil, fmt.Errorf("Group already exist")
} }
tng.cloudProvider.AddAutoprovisionedNodeGroup(tng.id, tng.minSize, tng.maxSize, 0, tng.machineType) newNodeGroup := tng.cloudProvider.AddAutoprovisionedNodeGroup(tng.id, tng.minSize, tng.maxSize, 0, tng.machineType)
return tng.cloudProvider.onNodeGroupCreate(tng.id) return newNodeGroup, tng.cloudProvider.onNodeGroupCreate(tng.id)
} }
// Delete deletes the node group on the cloud provider side. // Delete deletes the node group on the cloud provider side.

View File

@ -46,8 +46,10 @@ func (f *FakeNodeGroup) Nodes() ([]string, error) { return []string{},
func (f *FakeNodeGroup) TemplateNodeInfo() (*schedulercache.NodeInfo, error) { func (f *FakeNodeGroup) TemplateNodeInfo() (*schedulercache.NodeInfo, error) {
return nil, cloudprovider.ErrNotImplemented return nil, cloudprovider.ErrNotImplemented
} }
func (f *FakeNodeGroup) Exist() bool { return true } func (f *FakeNodeGroup) Exist() bool { return true }
func (f *FakeNodeGroup) Create() error { return cloudprovider.ErrAlreadyExist } func (f *FakeNodeGroup) Create() (cloudprovider.NodeGroup, error) {
return nil, cloudprovider.ErrAlreadyExist
}
func (f *FakeNodeGroup) Delete() error { return cloudprovider.ErrNotImplemented } func (f *FakeNodeGroup) Delete() error { return cloudprovider.ErrNotImplemented }
func (f *FakeNodeGroup) Autoprovisioned() bool { return false } func (f *FakeNodeGroup) Autoprovisioned() bool { return false }

View File

@ -41,21 +41,21 @@ func (p *AutoprovisioningNodeGroupManager) CreateNodeGroup(context *context.Auto
} }
oldId := nodeGroup.Id() oldId := nodeGroup.Id()
err := nodeGroup.Create() newNodeGroup, err := nodeGroup.Create()
if err != nil { if err != nil {
context.LogRecorder.Eventf(apiv1.EventTypeWarning, "FailedToCreateNodeGroup", context.LogRecorder.Eventf(apiv1.EventTypeWarning, "FailedToCreateNodeGroup",
"NodeAutoprovisioning: attempt to create node group %v failed: %v", oldId, err) "NodeAutoprovisioning: attempt to create node group %v failed: %v", oldId, err)
// TODO(maciekpytel): add some metric here after figuring out failure scenarios // TODO(maciekpytel): add some metric here after figuring out failure scenarios
return nil, errors.ToAutoscalerError(errors.CloudProviderError, err) return nil, errors.ToAutoscalerError(errors.CloudProviderError, err)
} }
newId := nodeGroup.Id() newId := newNodeGroup.Id()
if newId != oldId { if newId != oldId {
glog.V(2).Infof("Created node group %s based on template node group %s, will use new node group in scale-up", newId, oldId) glog.V(2).Infof("Created node group %s based on template node group %s, will use new node group in scale-up", newId, oldId)
} }
context.LogRecorder.Eventf(apiv1.EventTypeNormal, "CreatedNodeGroup", context.LogRecorder.Eventf(apiv1.EventTypeNormal, "CreatedNodeGroup",
"NodeAutoprovisioning: created new node group %v", newId) "NodeAutoprovisioning: created new node group %v", newId)
metrics.RegisterNodeGroupCreation() metrics.RegisterNodeGroupCreation()
return nodeGroup, nil return newNodeGroup, nil
} }
// RemoveUnneededNodeGroups removes node groups that are not needed anymore. // RemoveUnneededNodeGroups removes node groups that are not needed anymore.