create assigned identity before assignment

pkg/cloudprovider/cloudprovider.go

@@ -31,7 +31,7 @@ type Client struct {
 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
+	UpdateUserMSI(addUserAssignedMSIIDs []string, removeUserAssignedMSIIDs []string, node *corev1.Node) error
 	GetUserMSIs(node *corev1.Node) ([]string, error)
 }
 
@@ -130,8 +130,8 @@ func (c *Client) GetUserMSIs(node *corev1.Node) ([]string, error) {
 	return idList, nil
 }
 
-// AppendOrRemoveUserMSI will batch process the removal and addition of ids
-func (c *Client) AppendOrRemoveUserMSI(addUserAssignedMSIIDs []string, removeUserAssignedMSIIDs []string, node *corev1.Node) 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)
 	if err != nil {
 		return err
@@ -163,7 +163,7 @@ func (c *Client) AppendOrRemoveUserMSI(addUserAssignedMSIIDs []string, removeUse
 		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("UpdateUserMSI of %s completed in %s", node.Name, time.Since(timeStarted))
 	}
 	return nil
 }

pkg/mic/mic.go

@@ -238,7 +238,7 @@ func (c *Client) Sync(exit <-chan struct{}) {
 			workDone = true
 			for _, delID := range *deleteList {
 				glog.V(5).Infof("Deletion of id: %s", delID.Name)
-				// The inUse here checks if there are pods which are using the MSI in the newAssignedIDs.
+				// The inUse here checks if there are pods which are using the identity in the newAssignedIDs.
 				inUse := c.checkIfInUse(delID, newAssignedIDs)
 				id := delID.Spec.AzureIdentityRef
 				removedBinding := delID.Spec.AzureBindingRef
@@ -274,6 +274,16 @@ func (c *Client) Sync(exit <-chan struct{}) {
 				}
 
 				if isUserAssignedMSI {
+					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 {
+						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
+					}
+
 					c.appendToAddListForNode(nodeMap, id.Spec.ResourceID, node)
 				}
 
@@ -292,12 +302,12 @@ func (c *Client) Sync(exit <-chan struct{}) {
 			n.addUserAssignedMSIIDs = c.getUniqueIDs(n.addUserAssignedMSIIDs)
 			n.removeUserAssignedMSIIDs = c.getUniqueIDs(n.removeUserAssignedMSIIDs)
 
-			err = c.CloudClient.AppendOrRemoveUserMSI(n.addUserAssignedMSIIDs, n.removeUserAssignedMSIIDs, n.node)
+			err = c.CloudClient.UpdateUserMSI(n.addUserAssignedMSIIDs, n.removeUserAssignedMSIIDs, n.node)
 			if err != nil {
 				// check which all identity assignment failed
 				// remove those assigned identities
 				// TODO check with identity team if CreateOrUpdate results in error, is it all or some failed
-				glog.Errorf("AppendOrRemoveUserMSI failed for node %s with error %v", n.node.Name, err)
+				glog.Errorf("UpdateUserMSI failed for node %s with error %v", n.node.Name, err)
 				nodeWithErrors = append(nodeWithErrors, n.node.Name)
 			}
 			stats.Put(stats.TotalCreateOrUpdate, time.Since(beginAdding))
@@ -444,7 +454,6 @@ func (c *Client) makeAssignedIDs(azID *aadpodid.AzureIdentity, azBinding *aadpod
 func (c *Client) createAssignedIdentity(assignedID *aadpodid.AzureAssignedIdentity) error {
 	err := c.CRDClient.CreateAssignedIdentity(assignedID)
 	if err != nil {
-		glog.Error(err)
 		return err
 	}
 	return nil
@@ -453,7 +462,7 @@ func (c *Client) createAssignedIdentity(assignedID *aadpodid.AzureAssignedIdenti
 func (c *Client) removeAssignedIdentity(assignedID *aadpodid.AzureAssignedIdentity) error {
 	err := c.CRDClient.RemoveAssignedIdentity(assignedID)
 	if err != nil {
-		glog.Error(err)
+		return err
 	}
 	return nil
 }
@@ -492,7 +501,7 @@ func (c *Client) handleNodeErrors(nodesWithError []string, addList *[]aadpodid.A
 
 	nodeIdentityList := make(map[string][]string)
 	for _, n := range nodesWithError {
-		// get the list of msi's currently on node
+		// get the list of user assigned identities currently on node
 		node, err := c.NodeClient.Get(n)
 		if err != nil {
 			glog.Errorf("Lookup of node %s resulted in error %v", n, err)
@@ -501,6 +510,7 @@ func (c *Client) handleNodeErrors(nodesWithError []string, addList *[]aadpodid.A
 		idList, err := c.getUserMSIListForNode(node)
 		if err != nil {
 			glog.Errorf("Getting list of msi's from node %s resulted in error %v", n, err)
+			// if err here, we continue because the idList returned will be nil for error
 		}
 		nodeIdentityList[n] = idList
 	}
@@ -518,7 +528,6 @@ func (c *Client) handleNodeErrors(nodesWithError []string, addList *[]aadpodid.A
 		for _, createID := range *addList {
 			id := createID.Spec.AzureIdentityRef
 			binding := createID.Spec.AzureBindingRef
-			isUserAssignedMSI := c.checkIfUserAssignedMSI(id)
 
 			idExistsOnNode := c.checkIfMSIExistsOnNode(id, createID.Spec.NodeName, nodeIdentityList)
 			nodeHasErrors := isNodeErrored(createID.Spec.NodeName)
@@ -527,18 +536,6 @@ func (c *Client) handleNodeErrors(nodesWithError []string, addList *[]aadpodid.A
 				// 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))
-
-				glog.V(5).Infof("Initiating assigned id creation for pod - %s, binding - %s", createID.Spec.Pod, binding.Name)
-
-				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)
-					}
-				}
 			}
 		}
 	}

test/e2e/aadpodidentity_test.go

@@ -626,6 +626,8 @@ func setUpIdentityAndDeployment(azureIdentityName, suffix, replicas string) {
 	ok, err = daemonset.WaitOnReady(nmiDaemonSet)
 	Expect(err).NotTo(HaveOccurred())
 	Expect(ok).To(Equal(true))
+
+	time.Sleep(30 * time.Second)
 }
 
 // validateAzureAssignedIdentity will make sure a given AzureAssignedIdentity has the correct properties