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

Commit f2d68be

Browse files
aramasekkmsft
aramase
authored and
kkmsft
committed
update identity in vmss only once (#379)
* 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
1 parent 2d09e61 commit f2d68be

File tree

5 files changed

+154
-131
lines changed

5 files changed

+154
-131
lines changed

‎pkg/cloudprovider/cloudprovider.go

+39-54
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"github.com/Azure/go-autorest/autorest/azure"
1919
"github.com/golang/glog"
2020
yaml "gopkg.in/yaml.v2"
21-
corev1 "k8s.io/api/core/v1"
2221
)
2322

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

3231
// ClientInt client interface
3332
type ClientInt interface {
34-
RemoveUserMSI(userAssignedMSIID string, node *corev1.Node) error
35-
AssignUserMSI(userAssignedMSIID string, node *corev1.Node) error
36-
UpdateUserMSI(addUserAssignedMSIIDs []string, removeUserAssignedMSIIDs []string, node *corev1.Node) error
37-
GetUserMSIs(node *corev1.Node) ([]string, error)
33+
RemoveUserMSI(userAssignedMSIID, name string, isvmss bool) error
34+
AssignUserMSI(userAssignedMSIID, name string, isvmss bool) error
35+
UpdateUserMSI(addUserAssignedMSIIDs, removeUserAssignedMSIIDs []string, name string, isvmss bool) error
36+
GetUserMSIs(name string, isvmss bool) ([]string, error)
3837
}
3938

4039
// NewCloudProvider returns a azure cloud provider client
@@ -152,9 +151,9 @@ func withInspection() autorest.PrepareDecorator {
152151
}
153152
}
154153

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

170169
// UpdateUserMSI will batch process the removal and addition of ids
171-
func (c *Client) UpdateUserMSI(addUserAssignedMSIIDs []string, removeUserAssignedMSIIDs []string, node *corev1.Node) error {
172-
idH, updateFunc, err := c.getIdentityResource(node)
170+
func (c *Client) UpdateUserMSI(addUserAssignedMSIIDs, removeUserAssignedMSIIDs []string, name string, isvmss bool) error {
171+
idH, updateFunc, err := c.getIdentityResource(name, isvmss)
173172
if err != nil {
174173
return err
175174
}
@@ -184,43 +183,43 @@ func (c *Client) UpdateUserMSI(addUserAssignedMSIIDs []string, removeUserAssigne
184183
for _, userAssignedMSIID := range removeUserAssignedMSIIDs {
185184
requiresUpdate = true
186185
if err := info.RemoveUserIdentity(userAssignedMSIID); err != nil {
187-
return fmt.Errorf("could not remove identity from node %s: %v", node.Name, err)
186+
return fmt.Errorf("could not remove identity from node %s: %v", name, err)
188187
}
189188
}
190189
// add new ids to the list
191190
for _, userAssignedMSIID := range addUserAssignedMSIIDs {
192191
addedToList := info.AppendUserIdentity(userAssignedMSIID)
193192
if !addedToList {
194-
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, node.Name)
193+
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, name)
195194
}
196195
requiresUpdate = requiresUpdate || addedToList
197196
}
198197
if requiresUpdate {
199-
glog.Infof("Updating user assigned MSIs on %s", node.Name)
198+
glog.Infof("Updating user assigned MSIs on %s", name)
200199
timeStarted := time.Now()
201200
if err := updateFunc(); err != nil {
202201
return err
203202
}
204-
glog.V(6).Infof("UpdateUserMSI of %s completed in %s", node.Name, time.Since(timeStarted))
203+
glog.V(6).Infof("UpdateUserMSI of %s completed in %s", name, time.Since(timeStarted))
205204
}
206205
return nil
207206
}
208207

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

216215
info := idH.IdentityInfo()
217216
if info == nil {
218-
glog.Errorf("Identity null for vm: %s ", node.Name)
219-
return fmt.Errorf("identity null for vm: %s ", node.Name)
217+
glog.Errorf("Identity null for vm: %s ", name)
218+
return fmt.Errorf("identity null for vm: %s ", name)
220219
}
221220

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

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

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

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

242-
idH, updateFunc, err := c.getIdentityResource(node)
241+
idH, updateFunc, err := c.getIdentityResource(name, isvmss)
243242
if err != nil {
244243
return err
245244
}
246-
glog.V(6).Infof("Get of %s completed in %s", node.Name, time.Since(timeStarted))
245+
glog.V(6).Infof("Get of %s completed in %s", name, time.Since(timeStarted))
247246

248247
info := idH.IdentityInfo()
249248
if info == nil {
@@ -255,34 +254,17 @@ func (c *Client) AssignUserMSI(userAssignedMSIID string, node *corev1.Node) erro
255254
if err := updateFunc(); err != nil {
256255
return err
257256
}
258-
glog.V(6).Infof("CreateOrUpdate of %s completed in %s", node.Name, time.Since(timeStarted))
257+
glog.V(6).Infof("CreateOrUpdate of %s completed in %s", name, time.Since(timeStarted))
259258
} else {
260-
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, node.Name)
259+
glog.V(6).Infof("Identity %s already assigned to node %s. Skipping assignment.", userAssignedMSIID, name)
261260
}
262261
return nil
263262
}
264263

265-
func (c *Client) getIdentityResource(node *corev1.Node) (idH IdentityHolder, update func() error, retErr error) {
266-
name := node.Name // fallback in case parsing the provider spec fails
264+
func (c *Client) getIdentityResource(name string, isvmss bool) (idH IdentityHolder, update func() error, retErr error) {
267265
rg := c.Config.ResourceGroupName
268-
r, err := ParseResourceID(node.Spec.ProviderID)
269-
if err != nil {
270-
glog.Warningf("Could not parse Azure node resource ID: %v", err)
271-
}
272-
273-
rt := vmTypeOrDefault(&r, c.Config.VMType)
274-
glog.V(6).Infof("Using resource type %s for node %s", rt, name)
275-
276-
if r.ResourceGroup != "" {
277-
rg = r.ResourceGroup
278-
}
279-
280-
if r.ResourceName != "" {
281-
name = r.ResourceName
282-
}
283266

284-
switch rt {
285-
case "vmss":
267+
if isvmss {
286268
vmss, err := c.VMSSClient.Get(rg, name)
287269
if err != nil {
288270
return nil, nil, err
@@ -292,16 +274,17 @@ func (c *Client) getIdentityResource(node *corev1.Node) (idH IdentityHolder, upd
292274
return c.VMSSClient.CreateOrUpdate(rg, name, vmss)
293275
}
294276
idH = &vmssIdentityHolder{&vmss}
295-
default:
296-
vm, err := c.VMClient.Get(rg, name)
297-
if err != nil {
298-
return nil, nil, err
299-
}
300-
update = func() error {
301-
return c.VMClient.CreateOrUpdate(rg, name, vm)
302-
}
303-
idH = &vmIdentityHolder{&vm}
277+
return idH, update, nil
278+
}
279+
280+
vm, err := c.VMClient.Get(rg, name)
281+
if err != nil {
282+
return nil, nil, err
283+
}
284+
update = func() error {
285+
return c.VMClient.CreateOrUpdate(rg, name, vm)
304286
}
287+
idH = &vmIdentityHolder{&vm}
305288

306289
return idH, update, nil
307290
}
@@ -315,7 +298,9 @@ var (
315298
)
316299

317300
const (
318-
VMResourceType = "virtualMachines"
301+
// VMResourceType virtual machine resource type
302+
VMResourceType = "virtualMachines"
303+
// VMSSResourceType virtual machine scale sets resource type
319304
VMSSResourceType = "virtualMachineScaleSets"
320305
)
321306

‎pkg/cloudprovider/cloudprovider_test.go

+22-28
Original file line numberDiff line numberDiff line change
@@ -80,63 +80,63 @@ func TestSimple(t *testing.T) {
8080
node3 := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node3-0"}, Spec: corev1.NodeSpec{ProviderID: vmProvider}}
8181
node4 := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node4-vmss0000000"}, Spec: corev1.NodeSpec{ProviderID: vmssProvider}}
8282

83-
cloudClient.AssignUserMSI("ID0", node0)
84-
cloudClient.AssignUserMSI("ID0", node0)
85-
cloudClient.AssignUserMSI("ID0again", node0)
86-
cloudClient.AssignUserMSI("ID1", node1)
87-
cloudClient.AssignUserMSI("ID2", node2)
88-
cloudClient.AssignUserMSI("ID3", node3)
89-
cloudClient.AssignUserMSI("ID4", node4)
83+
cloudClient.AssignUserMSI("ID0", node0.Name, false)
84+
cloudClient.AssignUserMSI("ID0", node0.Name, false)
85+
cloudClient.AssignUserMSI("ID0again", node0.Name, false)
86+
cloudClient.AssignUserMSI("ID1", node1.Name, false)
87+
cloudClient.AssignUserMSI("ID2", node2.Name, false)
88+
cloudClient.AssignUserMSI("ID3", node3.Name, false)
89+
cloudClient.AssignUserMSI("ID4", node4.Name, true)
9090

9191
testMSI := []string{"ID0", "ID0again"}
92-
if !cloudClient.CompareMSI(node0, testMSI) {
92+
if !cloudClient.CompareMSI(node0.Name, false, testMSI) {
9393
cloudClient.PrintMSI(t)
9494
t.Error("MSI mismatch")
9595
}
9696

97-
cloudClient.RemoveUserMSI("ID0", node0)
98-
cloudClient.RemoveUserMSI("ID2", node2)
97+
cloudClient.RemoveUserMSI("ID0", node0.Name, false)
98+
cloudClient.RemoveUserMSI("ID2", node2.Name, false)
9999
testMSI = []string{"ID0again"}
100-
if !cloudClient.CompareMSI(node0, testMSI) {
100+
if !cloudClient.CompareMSI(node0.Name, false, testMSI) {
101101
cloudClient.PrintMSI(t)
102102
t.Error("MSI mismatch")
103103
}
104104
testMSI = []string{}
105-
if !cloudClient.CompareMSI(node2, testMSI) {
105+
if !cloudClient.CompareMSI(node2.Name, false, testMSI) {
106106
cloudClient.PrintMSI(t)
107107
t.Error("MSI mismatch")
108108
}
109109

110110
testMSI = []string{"ID3"}
111-
if !cloudClient.CompareMSI(node3, testMSI) {
111+
if !cloudClient.CompareMSI(node3.Name, false, testMSI) {
112112
cloudClient.PrintMSI(t)
113113
t.Error("MSI mismatch")
114114
}
115115

116116
testMSI = []string{"ID4"}
117-
if !cloudClient.CompareMSI(node4, testMSI) {
117+
if !cloudClient.CompareMSI(node4.Name, true, testMSI) {
118118
cloudClient.PrintMSI(t)
119119
t.Error("MSI mismatch")
120120
}
121121

122122
// test the UpdateUserMSI interface
123-
cloudClient.UpdateUserMSI([]string{"ID1", "ID2", "ID3"}, []string{"ID0again"}, node0)
123+
cloudClient.UpdateUserMSI([]string{"ID1", "ID2", "ID3"}, []string{"ID0again"}, node0.Name, false)
124124
testMSI = []string{"ID1", "ID2", "ID3"}
125-
if !cloudClient.CompareMSI(node0, testMSI) {
125+
if !cloudClient.CompareMSI(node0.Name, false, testMSI) {
126126
cloudClient.PrintMSI(t)
127127
t.Error("MSI mismatch")
128128
}
129129

130-
cloudClient.UpdateUserMSI(nil, []string{"ID3"}, node3)
130+
cloudClient.UpdateUserMSI(nil, []string{"ID3"}, node3.Name, false)
131131
testMSI = []string{}
132-
if !cloudClient.CompareMSI(node3, testMSI) {
132+
if !cloudClient.CompareMSI(node3.Name, false, testMSI) {
133133
cloudClient.PrintMSI(t)
134134
t.Error("MSI mismatch")
135135
}
136136

137-
cloudClient.UpdateUserMSI([]string{"ID3"}, nil, node4)
137+
cloudClient.UpdateUserMSI([]string{"ID3"}, nil, node4.Name, true)
138138
testMSI = []string{"ID4", "ID3"}
139-
if !cloudClient.CompareMSI(node4, testMSI) {
139+
if !cloudClient.CompareMSI(node4.Name, true, testMSI) {
140140
cloudClient.PrintMSI(t)
141141
t.Error("MSI mismatch")
142142
}
@@ -295,14 +295,8 @@ func (c *TestCloudClient) ListMSI() (ret map[string]*[]string) {
295295
return ret
296296
}
297297

298-
func (c *TestCloudClient) CompareMSI(node *corev1.Node, userIDs []string) bool {
299-
r, _ := ParseResourceID(node.Spec.ProviderID)
300-
vmType := vmTypeOrDefault(&r, c.Client.Config.VMType)
301-
name := node.Name
302-
if r.ResourceName != "" {
303-
name = r.ResourceName
304-
}
305-
if vmType == "vmss" {
298+
func (c *TestCloudClient) CompareMSI(name string, isvmss bool, userIDs []string) bool {
299+
if isvmss {
306300
return c.testVMSSClient.CompareMSI(name, userIDs)
307301
}
308302
return c.testVMClient.CompareMSI(name, userIDs)

0 commit comments

Comments
 (0)