diff --git a/pkg/apiserver/registry/virtualmachinepolicy/rest.go b/pkg/apiserver/registry/virtualmachinepolicy/rest.go index 6e6de941..812db77b 100644 --- a/pkg/apiserver/registry/virtualmachinepolicy/rest.go +++ b/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 diff --git a/pkg/controllers/networkpolicy/tracker.go b/pkg/controllers/networkpolicy/tracker.go index 34803db3..69af1673 100644 --- a/pkg/controllers/networkpolicy/tracker.go +++ b/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.