Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Commit

Permalink
update identity in vmss only once (#379)
Browse files Browse the repository at this point in the history 
* wip

* group vmss nodes to perform single call

* update log in struct

* remove core node from cloudprovider

* move to method

* update deployment

* Review feedback

* Review feedback 2
  • Loading branch information
@aramase @kkmsft
aramase authored and kkmsft committed Sep 17, 2019
1 parent 2d09e61 commit f2d68be
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 131 deletions.
93 changes: 39 additions & 54 deletions pkg/cloudprovider/cloudprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"github.com/Azure/go-autorest/autorest/azure"
"github.com/golang/glog"
yaml "gopkg.in/yaml.v2"
corev1 "k8s.io/api/core/v1"
)

// Client is a cloud provider client
Expand All @@ -31,10 +30,10 @@ type Client struct {

// ClientInt client interface
type ClientInt interface {
RemoveUserMSI(userAssignedMSIID string, node *corev1.Node) error
AssignUserMSI(userAssignedMSIID string, node *corev1.Node) error
UpdateUserMSI(addUserAssignedMSIIDs []string, removeUserAssignedMSIIDs []string, node *corev1.Node) error
GetUserMSIs(node *corev1.Node) ([]string, error)
RemoveUserMSI(userAssignedMSIID, name string, isvmss bool) error
AssignUserMSI(userAssignedMSIID, name string, isvmss bool) error
UpdateUserMSI(addUserAssignedMSIIDs, removeUserAssignedMSIIDs []string, name string, isvmss bool) error
GetUserMSIs(name string, isvmss bool) ([]string, error)
}

// NewCloudProvider returns a azure cloud provider client
Expand Down Expand Up @@ -152,9 +151,9 @@ func withInspection() autorest.PrepareDecorator {
}
}

// GetUserMSIs will return a list of all identities on the node
func (c *Client) GetUserMSIs(node *corev1.Node) ([]string, error) {
idH, _, err := c.getIdentityResource(node)
// GetUserMSIs will return a list of all identities on the node or vmss based on value of isvmss
func (c *Client) GetUserMSIs(name string, isvmss bool) ([]string, error) {
idH, _, err := c.getIdentityResource(name, isvmss)
if err != nil {
glog.Errorf("GetUserMSIs: get identity resource failed with error %v", err)
return nil, err
Expand All @@ -168,8 +167,8 @@ func (c *Client) GetUserMSIs(node *corev1.Node) ([]string, error) {
}

// UpdateUserMSI will batch process the removal and addition of ids
func (c *Client) UpdateUserMSI(addUserAssignedMSIIDs []string, removeUserAssignedMSIIDs []string, node *corev1.Node) error {
idH, updateFunc, err := c.getIdentityResource(node)
func (c *Client) UpdateUserMSI(addUserAssignedMSIIDs, removeUserAssignedMSIIDs []string, name string, isvmss bool) error {
idH, updateFunc, err := c.getIdentityResource(name, isvmss)
if err != nil {
return err
}
Expand All @@ -184,43 +183,43 @@ func (c *Client) UpdateUserMSI(addUserAssignedMSIIDs []string, removeUserAssigne
for _, userAssignedMSIID := range removeUserAssignedMSIIDs {
requiresUpdate = true
if err := info.RemoveUserIdentity(userAssignedMSIID); err != nil {
return fmt.Errorf("could not remove identity from node %s: %v", node.Name, err)
return fmt.Errorf("could not remove identity from node %s: %v", name, err)
}
}
// add new ids to the list
for _, userAssignedMSIID := range addUserAssignedMSIIDs {
addedToList := info.AppendUserIdentity(userAssignedMSIID)
if !addedToList {
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, node.Name)
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, name)
}
requiresUpdate = requiresUpdate || addedToList
}
if requiresUpdate {
glog.Infof("Updating user assigned MSIs on %s", node.Name)
glog.Infof("Updating user assigned MSIs on %s", name)
timeStarted := time.Now()
if err := updateFunc(); err != nil {
return err
}
glog.V(6).Infof("UpdateUserMSI of %s completed in %s", node.Name, time.Since(timeStarted))
glog.V(6).Infof("UpdateUserMSI of %s completed in %s", name, time.Since(timeStarted))
}
return nil
}

//RemoveUserMSI - Use the underlying cloud api calls and remove the given user assigned MSI from the vm.
func (c *Client) RemoveUserMSI(userAssignedMSIID string, node *corev1.Node) error {
idH, updateFunc, err := c.getIdentityResource(node)
func (c *Client) RemoveUserMSI(userAssignedMSIID, name string, isvmss bool) error {
idH, updateFunc, err := c.getIdentityResource(name, isvmss)
if err != nil {
return err
}

info := idH.IdentityInfo()
if info == nil {
glog.Errorf("Identity null for vm: %s ", node.Name)
return fmt.Errorf("identity null for vm: %s ", node.Name)
glog.Errorf("Identity null for vm: %s ", name)
return fmt.Errorf("identity null for vm: %s ", name)
}

if err := info.RemoveUserIdentity(userAssignedMSIID); err != nil {
return fmt.Errorf("could not remove identity from node %s: %v", node.Name, err)
return fmt.Errorf("could not remove identity from node %s: %v", name, err)
}

if err := updateFunc(); err != nil {
Expand All @@ -232,18 +231,18 @@ func (c *Client) RemoveUserMSI(userAssignedMSIID string, node *corev1.Node) erro
}

// AssignUserMSI - Use the underlying cloud api call and add the given user assigned MSI to the vm
func (c *Client) AssignUserMSI(userAssignedMSIID string, node *corev1.Node) error {
func (c *Client) AssignUserMSI(userAssignedMSIID, name string, isvmss bool) error {
// Get the vm using the VmClient
// Update the assigned identity into the VM using the CreateOrUpdate

glog.Infof("Find %s in resource group: %s", node.Name, c.Config.ResourceGroupName)
glog.Infof("Find %s in resource group: %s", name, c.Config.ResourceGroupName)
timeStarted := time.Now()

idH, updateFunc, err := c.getIdentityResource(node)
idH, updateFunc, err := c.getIdentityResource(name, isvmss)
if err != nil {
return err
}
glog.V(6).Infof("Get of %s completed in %s", node.Name, time.Since(timeStarted))
glog.V(6).Infof("Get of %s completed in %s", name, time.Since(timeStarted))

info := idH.IdentityInfo()
if info == nil {
Expand All @@ -255,34 +254,17 @@ func (c *Client) AssignUserMSI(userAssignedMSIID string, node *corev1.Node) erro
if err := updateFunc(); err != nil {
return err
}
glog.V(6).Infof("CreateOrUpdate of %s completed in %s", node.Name, time.Since(timeStarted))
glog.V(6).Infof("CreateOrUpdate of %s completed in %s", name, time.Since(timeStarted))
} else {
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, node.Name)
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, name)
}
return nil
}

func (c *Client) getIdentityResource(node *corev1.Node) (idH IdentityHolder, update func() error, retErr error) {
name := node.Name // fallback in case parsing the provider spec fails
func (c *Client) getIdentityResource(name string, isvmss bool) (idH IdentityHolder, update func() error, retErr error) {
rg := c.Config.ResourceGroupName
r, err := ParseResourceID(node.Spec.ProviderID)
if err != nil {
glog.Warningf("Could not parse Azure node resource ID: %v", err)
}

rt := vmTypeOrDefault(&r, c.Config.VMType)
glog.V(6).Infof("Using resource type %s for node %s", rt, name)

if r.ResourceGroup != "" {
rg = r.ResourceGroup
}

if r.ResourceName != "" {
name = r.ResourceName
}

switch rt {
case "vmss":
if isvmss {
vmss, err := c.VMSSClient.Get(rg, name)
if err != nil {
return nil, nil, err
Expand All @@ -292,16 +274,17 @@ func (c *Client) getIdentityResource(node *corev1.Node) (idH IdentityHolder, upd
return c.VMSSClient.CreateOrUpdate(rg, name, vmss)
}
idH = &vmssIdentityHolder{&vmss}
default:
vm, err := c.VMClient.Get(rg, name)
if err != nil {
return nil, nil, err
}
update = func() error {
return c.VMClient.CreateOrUpdate(rg, name, vm)
}
idH = &vmIdentityHolder{&vm}
return idH, update, nil
}

vm, err := c.VMClient.Get(rg, name)
if err != nil {
return nil, nil, err
}
update = func() error {
return c.VMClient.CreateOrUpdate(rg, name, vm)
}
idH = &vmIdentityHolder{&vm}

return idH, update, nil
}
Expand All @@ -315,7 +298,9 @@ var (
)

const (
VMResourceType = "virtualMachines"
// VMResourceType virtual machine resource type
VMResourceType = "virtualMachines"
// VMSSResourceType virtual machine scale sets resource type
VMSSResourceType = "virtualMachineScaleSets"
)

Expand Down
50 changes: 22 additions & 28 deletions pkg/cloudprovider/cloudprovider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,63 +80,63 @@ func TestSimple(t *testing.T) {
node3 := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node3-0"}, Spec: corev1.NodeSpec{ProviderID: vmProvider}}
node4 := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node4-vmss0000000"}, Spec: corev1.NodeSpec{ProviderID: vmssProvider}}

cloudClient.AssignUserMSI("ID0", node0)
cloudClient.AssignUserMSI("ID0", node0)
cloudClient.AssignUserMSI("ID0again", node0)
cloudClient.AssignUserMSI("ID1", node1)
cloudClient.AssignUserMSI("ID2", node2)
cloudClient.AssignUserMSI("ID3", node3)
cloudClient.AssignUserMSI("ID4", node4)
cloudClient.AssignUserMSI("ID0", node0.Name, false)
cloudClient.AssignUserMSI("ID0", node0.Name, false)
cloudClient.AssignUserMSI("ID0again", node0.Name, false)
cloudClient.AssignUserMSI("ID1", node1.Name, false)
cloudClient.AssignUserMSI("ID2", node2.Name, false)
cloudClient.AssignUserMSI("ID3", node3.Name, false)
cloudClient.AssignUserMSI("ID4", node4.Name, true)

testMSI := []string{"ID0", "ID0again"}
if !cloudClient.CompareMSI(node0, testMSI) {
if !cloudClient.CompareMSI(node0.Name, false, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}

cloudClient.RemoveUserMSI("ID0", node0)
cloudClient.RemoveUserMSI("ID2", node2)
cloudClient.RemoveUserMSI("ID0", node0.Name, false)
cloudClient.RemoveUserMSI("ID2", node2.Name, false)
testMSI = []string{"ID0again"}
if !cloudClient.CompareMSI(node0, testMSI) {
if !cloudClient.CompareMSI(node0.Name, false, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}
testMSI = []string{}
if !cloudClient.CompareMSI(node2, testMSI) {
if !cloudClient.CompareMSI(node2.Name, false, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}

testMSI = []string{"ID3"}
if !cloudClient.CompareMSI(node3, testMSI) {
if !cloudClient.CompareMSI(node3.Name, false, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}

testMSI = []string{"ID4"}
if !cloudClient.CompareMSI(node4, testMSI) {
if !cloudClient.CompareMSI(node4.Name, true, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}

// test the UpdateUserMSI interface
cloudClient.UpdateUserMSI([]string{"ID1", "ID2", "ID3"}, []string{"ID0again"}, node0)
cloudClient.UpdateUserMSI([]string{"ID1", "ID2", "ID3"}, []string{"ID0again"}, node0.Name, false)
testMSI = []string{"ID1", "ID2", "ID3"}
if !cloudClient.CompareMSI(node0, testMSI) {
if !cloudClient.CompareMSI(node0.Name, false, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}

cloudClient.UpdateUserMSI(nil, []string{"ID3"}, node3)
cloudClient.UpdateUserMSI(nil, []string{"ID3"}, node3.Name, false)
testMSI = []string{}
if !cloudClient.CompareMSI(node3, testMSI) {
if !cloudClient.CompareMSI(node3.Name, false, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}

cloudClient.UpdateUserMSI([]string{"ID3"}, nil, node4)
cloudClient.UpdateUserMSI([]string{"ID3"}, nil, node4.Name, true)
testMSI = []string{"ID4", "ID3"}
if !cloudClient.CompareMSI(node4, testMSI) {
if !cloudClient.CompareMSI(node4.Name, true, testMSI) {
cloudClient.PrintMSI(t)
t.Error("MSI mismatch")
}
Expand Down Expand Up @@ -295,14 +295,8 @@ func (c *TestCloudClient) ListMSI() (ret map[string]*[]string) {
return ret
}

func (c *TestCloudClient) CompareMSI(node *corev1.Node, userIDs []string) bool {
r, _ := ParseResourceID(node.Spec.ProviderID)
vmType := vmTypeOrDefault(&r, c.Client.Config.VMType)
name := node.Name
if r.ResourceName != "" {
name = r.ResourceName
}
if vmType == "vmss" {
func (c *TestCloudClient) CompareMSI(name string, isvmss bool, userIDs []string) bool {
if isvmss {
return c.testVMSSClient.CompareMSI(name, userIDs)
}
return c.testVMClient.CompareMSI(name, userIDs)
Expand Down
Loading

0 comments on commit f2d68be

Please sign in to comment.