update to using one get per node

pkg/cloudprovider/cloudprovider.go

@@ -32,7 +32,7 @@ type ClientInt interface {
 	RemoveUserMSI(userAssignedMSIID string, node *corev1.Node) error
 	AssignUserMSI(userAssignedMSIID string, node *corev1.Node) error
 	AppendOrRemoveUserMSI(addUserAssignedMSIIDs []string, removeUserAssignedMSIIDs []string, node *corev1.Node) error
-	CheckUserMSI(userAssignedMSIID string, node *corev1.Node) bool
+	GetUserMSIs(node *corev1.Node) ([]string, error)
 }
 
 // NewCloudProvider returns a azure cloud provider client
@@ -115,19 +115,19 @@ func withInspection() autorest.PrepareDecorator {
 	}
 }
 
-// CheckUserMSI checks if the identity has been assigned to the node
-func (c *Client) CheckUserMSI(userAssignedMSIID string, node *corev1.Node) bool {
+// 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)
 	if err != nil {
-		glog.Errorf("CheckUserMSI: get identity resource failed with error %v", err)
-		return false
+		glog.Errorf("GetUserMSIs: get identity resource failed with error %v", err)
+		return nil, err
 	}
-
 	info := idH.IdentityInfo()
 	if info == nil {
-		return false
+		return nil, fmt.Errorf("identity info is nil")
 	}
-	return info.CheckUserIdentityExists(userAssignedMSIID)
+	idList := info.GetUserIdentityList()
+	return idList, nil
 }
 
 // AppendOrRemoveUserMSI will batch process the removal and addition of ids

pkg/cloudprovider/identity.go

@@ -21,7 +21,7 @@ type IdentityHolder interface {
 type IdentityInfo interface {
 	AppendUserIdentity(id string) bool
 	RemoveUserIdentity(id string) error
-	CheckUserIdentityExists(id string) bool
+	GetUserIdentityList() []string
 }
 
 var (

pkg/cloudprovider/vm.go

@@ -108,9 +108,6 @@ func (i *vmIdentityInfo) AppendUserIdentity(id string) bool {
 	return appendUserIdentity(&i.info.Type, i.info.IdentityIds, id)
 }
 
-func (i *vmIdentityInfo) CheckUserIdentityExists(id string) bool {
-	if i.info.IdentityIds == nil || !checkIfIDInList(*i.info.IdentityIds, id) {
-		return false
-	}
-	return true
+func (i *vmIdentityInfo) GetUserIdentityList() []string {
+	return *i.info.IdentityIds
 }

pkg/cloudprovider/vmss.go

@@ -116,9 +116,6 @@ func (i *vmssIdentityInfo) AppendUserIdentity(id string) bool {
 	return appendUserIdentity(&i.info.Type, i.info.IdentityIds, id)
 }
 
-func (i *vmssIdentityInfo) CheckUserIdentityExists(id string) bool {
-	if i.info.IdentityIds == nil || !checkIfIDInList(*i.info.IdentityIds, id) {
-		return false
-	}
-	return true
+func (i *vmssIdentityInfo) GetUserIdentityList() []string {
+	return *i.info.IdentityIds
 }

pkg/mic/mic.go

@@ -273,17 +273,6 @@ func (c *Client) Sync(exit <-chan struct{}) {
 					continue
 				}
 
-				glog.V(5).Infof("Initiating assigned id creation for pod - %s, binding - %s", createID.Spec.Pod, binding.Name)
-
-				err = c.createAssignedIdentity(&createID)
-				if err != nil {
-					// Since k8s event has only Info and Warning, using Warning.
-					c.EventRecorder.Event(binding, corev1.EventTypeWarning, "binding apply error",
-						fmt.Sprintf("Creating assigned identity binding %s node %s for pod %s resulted in error %v", binding.Name, createID.Spec.NodeName, createID.Name, err))
-					glog.Error(err)
-					continue
-				}
-
 				if isUserAssignedMSI {
 					c.appendToAddListForNode(nodeMap, id.Spec.ResourceID, node)
 				}
@@ -501,6 +490,21 @@ func (c *Client) appendToAddListForNode(nodeMap map[string]trackUserAssignedMSII
 
 func (c *Client) handleNodeErrors(nodesWithError []string, addList *[]aadpodid.AzureAssignedIdentity, deleteList *[]aadpodid.AzureAssignedIdentity, newAssignedIDs []aadpodid.AzureAssignedIdentity) error {
 
+	nodeIdentityList := make(map[string][]string)
+	for _, n := range nodesWithError {
+		// get the list of msi's currently on node
+		node, err := c.NodeClient.Get(n)
+		if err != nil {
+			glog.Errorf("Lookup of node %s resulted in error %v", n, err)
+			continue
+		}
+		idList, err := c.getUserMSIListForNode(node)
+		if err != nil {
+			glog.Errorf("Getting list of msi's from node %s resulted in error %v", n, err)
+		}
+		nodeIdentityList[n] = idList
+	}
+
 	isNodeErrored := func(node string) bool {
 		for _, n := range nodesWithError {
 			if n == node {
@@ -516,30 +520,24 @@ func (c *Client) handleNodeErrors(nodesWithError []string, addList *[]aadpodid.A
 			binding := createID.Spec.AzureBindingRef
 			isUserAssignedMSI := c.checkIfUserAssignedMSI(id)
 
-			node, err := c.NodeClient.Get(createID.Spec.NodeName)
-			if err != nil {
-				c.EventRecorder.Event(binding, corev1.EventTypeWarning, "get node error",
-					fmt.Sprintf("Lookup of node %s for pod %s resulted in error %v", createID.Spec.NodeName, createID.Name, err))
-				continue
-			}
-			idExistsOnNode := c.checkIfMSIExistsOnNode(id, node)
+			idExistsOnNode := c.checkIfMSIExistsOnNode(id, createID.Spec.NodeName, nodeIdentityList)
 			nodeHasErrors := isNodeErrored(createID.Spec.NodeName)
 
 			if !nodeHasErrors || idExistsOnNode {
 				// the identity was successfully assigned to the node
 				c.EventRecorder.Event(binding, corev1.EventTypeNormal, "binding applied",
 					fmt.Sprintf("Binding %s applied on node %s for pod %s", binding.Name, createID.Spec.NodeName, createID.Name))
-				continue
-			}
 
-			if isUserAssignedMSI {
-				// Since k8s event has only Info and Warning, using Warning.
-				c.EventRecorder.Event(binding, corev1.EventTypeWarning, "binding apply error",
-					fmt.Sprintf("Applying binding %s node %s for pod %s resulted in error", binding.Name, createID.Spec.NodeName, createID.Name))
+				glog.V(5).Infof("Initiating assigned id creation for pod - %s, binding - %s", createID.Spec.Pod, binding.Name)
 
-				newErr := c.removeAssignedIdentity(&createID)
-				if newErr != nil {
-					glog.Errorf("Error when removing assigned identity in create error path err: %v", newErr)
+				if isUserAssignedMSI {
+					err := c.createAssignedIdentity(&createID)
+					if err != nil {
+						// Since k8s event has only Info and Warning, using Warning.
+						c.EventRecorder.Event(binding, corev1.EventTypeWarning, "binding apply error",
+							fmt.Sprintf("Creating assigned identity binding %s node %s for pod %s resulted in error %v", binding.Name, createID.Spec.NodeName, createID.Name, err))
+						glog.Error(err)
+					}
 				}
 			}
 		}
@@ -553,14 +551,7 @@ func (c *Client) handleNodeErrors(nodesWithError []string, addList *[]aadpodid.A
 			isUserAssignedMSI := c.checkIfUserAssignedMSI(id)
 			nodeHasErrors := isNodeErrored(delID.Spec.NodeName)
 
-			node, err := c.NodeClient.Get(delID.Spec.NodeName)
-			if err != nil {
-				c.EventRecorder.Event(removedBinding, corev1.EventTypeWarning, "get node error",
-					fmt.Sprintf("Lookup of node %s for pod %s resulted in error %v", delID.Spec.NodeName, delID.Name, err))
-				continue
-			}
-
-			idExistsOnNode := c.checkIfMSIExistsOnNode(id, node)
+			idExistsOnNode := c.checkIfMSIExistsOnNode(id, delID.Spec.NodeName, nodeIdentityList)
 
 			if !nodeHasErrors || !idExistsOnNode {
 				// the identity was successfully removed from node
@@ -595,8 +586,19 @@ func (c *Client) getAssignedIDName(podName string, podNameSpace string, idName s
 	return podName + "-" + podNameSpace + "-" + idName
 }
 
-func (c *Client) checkIfMSIExistsOnNode(id *aadpodid.AzureIdentity, node *corev1.Node) bool {
-	return c.CloudClient.CheckUserMSI(id.Spec.ResourceID, node)
+func (c *Client) checkIfMSIExistsOnNode(id *aadpodid.AzureIdentity, nodeName string, nodeMSIList map[string][]string) bool {
+	if nList, ok := nodeMSIList[nodeName]; ok {
+		for _, userAssignedMSI := range nList {
+			if userAssignedMSI == id.Spec.ResourceID {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func (c *Client) getUserMSIListForNode(node *corev1.Node) ([]string, error) {
+	return c.CloudClient.GetUserMSIs(node)
 }
 
 func (c *Client) convertIDListToMap(arr []aadpodid.AzureIdentity) (m map[string]aadpodid.AzureIdentity, err error) {