Fix an issue in tracker module.

Consider a scenario where the following sequence of events are received.
An ANP with Ipaddress block and ATGroup is realized -> Secret delete ->
Sync with cloud finds diff and updates member -> AT Modify -> AT Modofy
-> NP delete -> ProcessTrackers -> AT delete.

This results in a scenario where a tracker has one appliedToSgs, there are
no nps in npIndexers, prevAppliedToSgs is nil, computing NP status will
result in an empty appliedToToNpMap and there by an error.

Fix is to add an additional check to compute Npstatus from
appliedToToNpMap and appliedToSgs.

Signed-off-by: Anand Kumar <[email protected]>

pkg/apiserver/registry/virtualmachinepolicy/rest.go

@@ -83,7 +83,10 @@ func (r *REST) Get(ctx context.Context, name string, _ *metav1.GetOptions) (runt
 		return nil, errors.NewNotFound(runtimev1alpha1.Resource("virtualmachinepolicy"), name)
 	}
 	// For a given vm namespaced name there should be only one matching tracker.
-	return r.convertToVmp(objs[0]), nil
+	if vmp := r.convertToVmp(objs[0]); vmp != nil {
+		return vmp, nil
+	}
+	return nil, nil
 }
 
 func (r *REST) List(ctx context.Context, _ *internalversion.ListOptions) (runtime.Object, error) {
@@ -97,7 +100,9 @@ func (r *REST) List(ctx context.Context, _ *internalversion.ListOptions) (runtim
 	vmpList := &runtimev1alpha1.VirtualMachinePolicyList{}
 	for _, obj := range objs {
 		vmp := r.convertToVmp(obj)
-		vmpList.Items = append(vmpList.Items, *vmp)
+		if vmp != nil {
+			vmpList.Items = append(vmpList.Items, *vmp)
+		}
 	}
 	return vmpList, nil
 }
@@ -135,6 +140,9 @@ func (r *REST) ConvertToTable(_ context.Context, obj runtime.Object, _ runtime.O
 func (r *REST) convertToVmp(obj interface{}) *runtimev1alpha1.VirtualMachinePolicy {
 	failed, inProgress := false, false
 	tracker := obj.(*networkpolicy.CloudResourceNpTracker)
+	if len(tracker.NpStatus) == 0 {
+		return nil
+	}
 	for _, status := range tracker.NpStatus {
 		if status.Realization == runtimev1alpha1.Failed {
 			failed = true

pkg/controllers/networkpolicy/tracker.go

@@ -61,6 +61,7 @@ func (r *NetworkPolicyReconciler) newCloudResourceNpTracker(rsc *cloudresource.C
 	}
 
 	// A vm is uniquely identified by its cloud assigned id and vpc id.
+	// TODO: Same vm imported in multiple Namespaces is not supported.
 	vm := vmItems[0].(*runtimev1alpha1.VirtualMachine)
 	tracker := &CloudResourceNpTracker{
 		appliedToSGs:     make(map[string]*appliedToSecurityGroup),
@@ -109,7 +110,8 @@ func (r *NetworkPolicyReconciler) processCloudResourceNpTrackers() {
 		npStatus, ok := status[tracker.NamespacedName.Namespace]
 		if !ok {
 			// Should not occur.
-			log.Error(fmt.Errorf("failed to get NetworkPolicy status for tracker"), "", "tracker", tracker)
+			log.Error(fmt.Errorf("failed to get NetworkPolicy status for tracker"), "",
+				"tracker", tracker, "appliedToSGs", tracker.appliedToSGs, "appliedToToNpMap", tracker.appliedToToNpMap)
 			tracker.unmarkDirty()
 			continue
 		}
@@ -161,6 +163,7 @@ func (c *CloudResourceNpTracker) computeNpStatus(r *NetworkPolicyReconciler) map
 
 	// retrieve all network policies related to cloud resource's applied groups
 	npMap := make(map[interface{}]string)
+	changed := false
 	appliedToToNpMap := make(map[string][]types.NamespacedName)
 	for key, asg := range c.appliedToSGs {
 		nps, err := r.networkPolicyIndexer.ByIndex(networkPolicyIndexerByAppliedToGrp, asg.id.Name)
@@ -172,6 +175,7 @@ func (c *CloudResourceNpTracker) computeNpStatus(r *NetworkPolicyReconciler) map
 		// Not considering cloud resources belongs to multiple AppliedToGroups of same NetworkPolicy.
 		for _, i := range nps {
 			namespacedName := types.NamespacedName{Namespace: i.(*networkPolicy).Namespace, Name: i.(*networkPolicy).Name}
+			changed = true
 			appliedToToNpMap[asg.id.Name] = append(appliedToToNpMap[asg.id.Name], namespacedName)
 			npMap[i] = key
 		}
@@ -231,6 +235,7 @@ func (c *CloudResourceNpTracker) computeNpStatus(r *NetworkPolicyReconciler) map
 	}
 
 	for _, asg := range newPrevSgs {
+		changed = true
 		if asg.status == nil {
 			delete(c.appliedToToNpMap, asg.id.Name)
 			delete(newPrevSgs, asg.id.CloudResourceID.String())
@@ -284,6 +289,30 @@ func (c *CloudResourceNpTracker) computeNpStatus(r *NetworkPolicyReconciler) map
 		}
 	}
 
+	// When a NP is deleted, but the appliedToGroup delete is not received yet, then NP Status is computed based on
+	// previous appliedToSgs. Suppose prevAppliedToSGs is nil, then NP status will be empty.
+	// An additional check required to compute the NP status, based on the appliedToToNpMap and appliedToSGs.
+	// If there are any entries in appliedToToNpMap with an appliedToSG, set the np status as in progress, since
+	// appliedToGroup is in a transient state.
+	if !changed && len(c.appliedToToNpMap) > 0 {
+		for _, asg := range c.appliedToSGs {
+			if namespacedNames, ok := c.appliedToToNpMap[asg.id.Name]; ok {
+				for _, namespacedName := range namespacedNames {
+					npList, ok := ret[namespacedName.Namespace]
+					if !ok {
+						npList = make(map[string]*runtimev1alpha1.NetworkPolicyStatus)
+						ret[namespacedName.Namespace] = npList
+					}
+					npList[namespacedName.Name] = &runtimev1alpha1.NetworkPolicyStatus{
+						Reason:      NoneStr,
+						Realization: runtimev1alpha1.InProgress,
+					}
+					appliedToToNpMap[asg.id.Name] = append(appliedToToNpMap[asg.id.Name], namespacedName)
+				}
+			}
+		}
+	}
+
 	// Update the map with the latest data.
 	c.appliedToToNpMap = appliedToToNpMap
 	// Update the previous appliedToSgs.