From 2107ea3ee14b0e74e2e4eb18b56c10f6b8b0bada Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 15 Feb 2022 11:43:08 -0800 Subject: [PATCH 1/6] [NPM] Adding support for Goal state processor Hydration event --- .../goalstateprocessor/goalstateprocessor.go | 34 +++-- npm/pkg/dataplane/dataplane.go | 8 + npm/pkg/dataplane/dpshim/dpshim.go | 14 +- npm/pkg/dataplane/ipsets/ipsetmanager.go | 12 ++ .../mocks/genericdataplane_generated.go | 28 ++++ npm/pkg/dataplane/policies/policymanager.go | 10 ++ npm/pkg/dataplane/types.go | 2 + npm/pkg/protos/transport.pb.go | 138 +++++++++++++----- npm/pkg/protos/transport.proto | 8 +- 9 files changed, 202 insertions(+), 52 deletions(-) diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go index 453b42f632..68e8008771 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go @@ -92,9 +92,6 @@ func (gsp *GoalStateProcessor) processNext(stopCh <-chan struct{}) bool { } func (gsp *GoalStateProcessor) process(inputEvent *protos.Events) { - // TODO (Vamsi) differentiate between hydration event and a normal event - // in hydration event, any thing in local cache and not in event should be deleted. - klog.Infof("Processing event") // apply dataplane after syncing defer func() { @@ -104,20 +101,35 @@ func (gsp *GoalStateProcessor) process(inputEvent *protos.Events) { } }() - // Process these individual buckkets in order - // 1. Apply IPSET - // 2. Apply POLICY - // 3. Remove POLICY - // 4. Remove IPSET - - // TODO need to handle first connect stream of all GoalStates payload := inputEvent.GetPayload() - if !validatePayload(payload) { klog.Warningf("Empty payload in event %s", inputEvent) return } + switch inputEvent.GetEventType() { + case protos.Events_Hydration: + // in hydration event, any thing in local cache and not in event should be deleted. + klog.Infof("Received hydration event") + gsp.processHydrationEvent(payload) + case protos.Events_GoalState: + klog.Infof("Received goal state event") + gsp.processGoalStateEvent(payload) + default: + klog.Errorf("Received unknown event type %s", inputEvent.GetEventType()) + } +} + +func (gsp *GoalStateProcessor) processHydrationEvent(payload map[string]*protos.GoalState) { + +} + +func (gsp *GoalStateProcessor) processGoalStateEvent(payload map[string]*protos.GoalState) { + // Process these individual buckets in order + // 1. Apply IPSET + // 2. Apply POLICY + // 3. Remove POLICY + // 4. Remove IPSET if ipsetApplyPayload, ok := payload[cp.IpsetApply]; ok { err := gsp.processIPSetsApplyEvent(ipsetApplyPayload) if err != nil { diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 15cd924907..fba6e83164 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -276,6 +276,14 @@ func (dp *DataPlane) UpdatePolicy(policy *policies.NPMNetworkPolicy) error { return nil } +func (dp *DataPlane) GetAllIPSets() []string { + return dp.ipsetMgr.GetAllIPSets() +} + +func (dp *DataPlane) GetAllPolicies() []string { + return dp.policyMgr.GetAllPolicies() +} + func (dp *DataPlane) createIPSetsAndReferences(sets []*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { // Create IPSets first along with reference updates npmErrorString := npmerrors.AddSelectorReference diff --git a/npm/pkg/dataplane/dpshim/dpshim.go b/npm/pkg/dataplane/dpshim/dpshim.go index a38e47d3c4..720e42ebac 100644 --- a/npm/pkg/dataplane/dpshim/dpshim.go +++ b/npm/pkg/dataplane/dpshim/dpshim.go @@ -81,7 +81,8 @@ func (dp *DPShim) HydrateClients() (*protos.Events, error) { } return &protos.Events{ - Payload: goalStates, + EventType: protos.Events_Hydration, + Payload: goalStates, }, nil } @@ -441,7 +442,8 @@ func (dp *DPShim) ApplyDataPlane() error { go func() { dp.OutChannel <- &protos.Events{ - Payload: goalStates, + EventType: protos.Events_GoalState, + Payload: goalStates, } }() @@ -449,6 +451,14 @@ func (dp *DPShim) ApplyDataPlane() error { return nil } +func (dp *DPShim) GetAllIPSets() []string { + return nil +} + +func (dp *DPShim) GetAllPolicies() []string { + return nil +} + func (dp *DPShim) lock() { dp.mu.Lock() } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 847999c7a5..32f904ee6e 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -459,6 +459,18 @@ func (iMgr *IPSetManager) GetSelectorReferencesBySet(setName string) (map[string return set.SelectorReference, nil } +func (iMgr *IPSetManager) GetAllIPSets() []string { + iMgr.Lock() + defer iMgr.Unlock() + setNames := make([]string, len(iMgr.setMap)) + i := 0 + for setName := range iMgr.setMap { + setNames[i] = setName + i++ + } + return setNames +} + func (iMgr *IPSetManager) exists(name string) bool { _, ok := iMgr.setMap[name] return ok diff --git a/npm/pkg/dataplane/mocks/genericdataplane_generated.go b/npm/pkg/dataplane/mocks/genericdataplane_generated.go index dc25f61aa9..3b28bbef29 100644 --- a/npm/pkg/dataplane/mocks/genericdataplane_generated.go +++ b/npm/pkg/dataplane/mocks/genericdataplane_generated.go @@ -133,6 +133,34 @@ func (mr *MockGenericDataplaneMockRecorder) DeleteIPSet(setMetadata interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).DeleteIPSet), setMetadata) } +// GetAllIPSets mocks base method. +func (m *MockGenericDataplane) GetAllIPSets() []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllIPSets") + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetAllIPSets indicates an expected call of GetAllIPSets. +func (mr *MockGenericDataplaneMockRecorder) GetAllIPSets() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllIPSets", reflect.TypeOf((*MockGenericDataplane)(nil).GetAllIPSets)) +} + +// GetAllPolicies mocks base method. +func (m *MockGenericDataplane) GetAllPolicies() []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllPolicies") + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetAllPolicies indicates an expected call of GetAllPolicies. +func (mr *MockGenericDataplaneMockRecorder) GetAllPolicies() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllPolicies", reflect.TypeOf((*MockGenericDataplane)(nil).GetAllPolicies)) +} + // GetIPSet mocks base method. func (m *MockGenericDataplane) GetIPSet(setName string) *ipsets.IPSet { m.ctrl.T.Helper() diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index eb61285eb4..b9d8dabdd3 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -98,6 +98,16 @@ func (pMgr *PolicyManager) Reconcile(stopChannel <-chan struct{}) { }() } +func (pMgr *PolicyManager) GetAllPolicies() []string { + policyKeys := make([]string, len(pMgr.policyMap.cache)) + i := 0 + for policyKey := range pMgr.policyMap.cache { + policyKeys[i] = policyKey + i++ + } + return policyKeys +} + func (pMgr *PolicyManager) PolicyExists(policyKey string) bool { _, ok := pMgr.policyMap.cache[policyKey] return ok diff --git a/npm/pkg/dataplane/types.go b/npm/pkg/dataplane/types.go index aebe6c9c51..a140ad3486 100644 --- a/npm/pkg/dataplane/types.go +++ b/npm/pkg/dataplane/types.go @@ -8,6 +8,7 @@ import ( type GenericDataplane interface { BootupDataplane() error RunPeriodicTasks() + GetAllIPSets() []string GetIPSet(setName string) *ipsets.IPSet CreateIPSets(setMetadatas []*ipsets.IPSetMetadata) DeleteIPSet(setMetadata *ipsets.IPSetMetadata) @@ -16,6 +17,7 @@ type GenericDataplane interface { AddToLists(listMetadatas []*ipsets.IPSetMetadata, setMetadatas []*ipsets.IPSetMetadata) error RemoveFromList(listMetadata *ipsets.IPSetMetadata, setMetadatas []*ipsets.IPSetMetadata) error ApplyDataPlane() error + GetAllPolicies() []string AddPolicy(policies *policies.NPMNetworkPolicy) error RemovePolicy(PolicyKey string) error UpdatePolicy(policies *policies.NPMNetworkPolicy) error diff --git a/npm/pkg/protos/transport.pb.go b/npm/pkg/protos/transport.pb.go index 8dec49beb5..370e27cea2 100644 --- a/npm/pkg/protos/transport.pb.go +++ b/npm/pkg/protos/transport.pb.go @@ -63,6 +63,52 @@ func (DatapathPodMetadata_APIVersion) EnumDescriptor() ([]byte, []int) { return file_transport_proto_rawDescGZIP(), []int{0, 0} } +type Events_EventType int32 + +const ( + Events_Hydration Events_EventType = 0 + Events_GoalState Events_EventType = 1 +) + +// Enum value maps for Events_EventType. +var ( + Events_EventType_name = map[int32]string{ + 0: "Hydration", + 1: "GoalState", + } + Events_EventType_value = map[string]int32{ + "Hydration": 0, + "GoalState": 1, + } +) + +func (x Events_EventType) Enum() *Events_EventType { + p := new(Events_EventType) + *p = x + return p +} + +func (x Events_EventType) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (Events_EventType) Descriptor() protoreflect.EnumDescriptor { + return file_transport_proto_enumTypes[1].Descriptor() +} + +func (Events_EventType) Type() protoreflect.EnumType { + return &file_transport_proto_enumTypes[1] +} + +func (x Events_EventType) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use Events_EventType.Descriptor instead. +func (Events_EventType) EnumDescriptor() ([]byte, []int) { + return file_transport_proto_rawDescGZIP(), []int{1, 0} +} + // DatapathPodMetadata is the metadata for a datapath pod type DatapathPodMetadata struct { state protoimpl.MessageState @@ -135,8 +181,9 @@ type Events struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields + EventType Events_EventType `protobuf:"varint,1,opt,name=eventType,proto3,enum=protos.Events_EventType" json:"eventType,omitempty"` // Payload can contain one or more Event objects. - Payload map[string]*GoalState `protobuf:"bytes,1,rep,name=payload,proto3" json:"payload,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` + Payload map[string]*GoalState `protobuf:"bytes,2,rep,name=payload,proto3" json:"payload,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` } func (x *Events) Reset() { @@ -171,6 +218,13 @@ func (*Events) Descriptor() ([]byte, []int) { return file_transport_proto_rawDescGZIP(), []int{1} } +func (x *Events) GetEventType() Events_EventType { + if x != nil { + return x.EventType + } + return Events_Hydration +} + func (x *Events) GetPayload() map[string]*GoalState { if x != nil { return x.Payload @@ -244,28 +298,34 @@ var file_transport_proto_rawDesc = []byte{ 0x6f, 0x64, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x2e, 0x41, 0x50, 0x49, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x0a, 0x61, 0x70, 0x69, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x22, 0x14, 0x0a, 0x0a, 0x41, 0x50, 0x49, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x12, - 0x06, 0x0a, 0x02, 0x56, 0x31, 0x10, 0x00, 0x22, 0x8e, 0x01, 0x0a, 0x06, 0x45, 0x76, 0x65, 0x6e, - 0x74, 0x73, 0x12, 0x35, 0x0a, 0x07, 0x70, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x18, 0x01, 0x20, - 0x03, 0x28, 0x0b, 0x32, 0x1b, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2e, 0x45, 0x76, 0x65, - 0x6e, 0x74, 0x73, 0x2e, 0x50, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x45, 0x6e, 0x74, 0x72, 0x79, - 0x52, 0x07, 0x70, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x1a, 0x4d, 0x0a, 0x0c, 0x50, 0x61, 0x79, - 0x6c, 0x6f, 0x61, 0x64, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, - 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x27, 0x0a, 0x05, 0x76, - 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x11, 0x2e, 0x70, 0x72, 0x6f, - 0x74, 0x6f, 0x73, 0x2e, 0x47, 0x6f, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x05, 0x76, - 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, 0x1f, 0x0a, 0x09, 0x47, 0x6f, 0x61, 0x6c, - 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x64, 0x61, 0x74, 0x61, 0x18, 0x01, 0x20, - 0x01, 0x28, 0x0c, 0x52, 0x04, 0x64, 0x61, 0x74, 0x61, 0x32, 0x4b, 0x0a, 0x0f, 0x44, 0x61, 0x74, - 0x61, 0x70, 0x6c, 0x61, 0x6e, 0x65, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x73, 0x12, 0x38, 0x0a, 0x07, - 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x12, 0x1b, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, - 0x2e, 0x44, 0x61, 0x74, 0x61, 0x70, 0x61, 0x74, 0x68, 0x50, 0x6f, 0x64, 0x4d, 0x65, 0x74, 0x61, - 0x64, 0x61, 0x74, 0x61, 0x1a, 0x0e, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2e, 0x45, 0x76, - 0x65, 0x6e, 0x74, 0x73, 0x30, 0x01, 0x42, 0x43, 0x5a, 0x41, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, - 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x41, 0x7a, 0x75, 0x72, 0x65, 0x2f, 0x61, 0x7a, 0x75, 0x72, 0x65, - 0x2d, 0x63, 0x6f, 0x6e, 0x74, 0x61, 0x69, 0x6e, 0x65, 0x72, 0x2d, 0x6e, 0x65, 0x74, 0x77, 0x6f, - 0x72, 0x6b, 0x69, 0x6e, 0x67, 0x2f, 0x6e, 0x70, 0x6d, 0x2f, 0x70, 0x6b, 0x67, 0x2f, 0x70, 0x72, - 0x6f, 0x74, 0x6f, 0x73, 0x3b, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, - 0x74, 0x6f, 0x33, + 0x06, 0x0a, 0x02, 0x56, 0x31, 0x10, 0x00, 0x22, 0xf1, 0x01, 0x0a, 0x06, 0x45, 0x76, 0x65, 0x6e, + 0x74, 0x73, 0x12, 0x36, 0x0a, 0x09, 0x65, 0x76, 0x65, 0x6e, 0x74, 0x54, 0x79, 0x70, 0x65, 0x18, + 0x01, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x18, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2e, 0x45, + 0x76, 0x65, 0x6e, 0x74, 0x73, 0x2e, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x54, 0x79, 0x70, 0x65, 0x52, + 0x09, 0x65, 0x76, 0x65, 0x6e, 0x74, 0x54, 0x79, 0x70, 0x65, 0x12, 0x35, 0x0a, 0x07, 0x70, 0x61, + 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x18, 0x02, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x1b, 0x2e, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x73, 0x2e, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x73, 0x2e, 0x50, 0x61, 0x79, 0x6c, + 0x6f, 0x61, 0x64, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x52, 0x07, 0x70, 0x61, 0x79, 0x6c, 0x6f, 0x61, + 0x64, 0x1a, 0x4d, 0x0a, 0x0c, 0x50, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x45, 0x6e, 0x74, 0x72, + 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, + 0x6b, 0x65, 0x79, 0x12, 0x27, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, + 0x28, 0x0b, 0x32, 0x11, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2e, 0x47, 0x6f, 0x61, 0x6c, + 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, + 0x22, 0x29, 0x0a, 0x09, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x54, 0x79, 0x70, 0x65, 0x12, 0x0d, 0x0a, + 0x09, 0x48, 0x79, 0x64, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x10, 0x00, 0x12, 0x0d, 0x0a, 0x09, + 0x47, 0x6f, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x10, 0x01, 0x22, 0x1f, 0x0a, 0x09, 0x47, + 0x6f, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x64, 0x61, 0x74, 0x61, + 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x04, 0x64, 0x61, 0x74, 0x61, 0x32, 0x4b, 0x0a, 0x0f, + 0x44, 0x61, 0x74, 0x61, 0x70, 0x6c, 0x61, 0x6e, 0x65, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x73, 0x12, + 0x38, 0x0a, 0x07, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x12, 0x1b, 0x2e, 0x70, 0x72, 0x6f, + 0x74, 0x6f, 0x73, 0x2e, 0x44, 0x61, 0x74, 0x61, 0x70, 0x61, 0x74, 0x68, 0x50, 0x6f, 0x64, 0x4d, + 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x1a, 0x0e, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, + 0x2e, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x73, 0x30, 0x01, 0x42, 0x43, 0x5a, 0x41, 0x67, 0x69, 0x74, + 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x41, 0x7a, 0x75, 0x72, 0x65, 0x2f, 0x61, 0x7a, + 0x75, 0x72, 0x65, 0x2d, 0x63, 0x6f, 0x6e, 0x74, 0x61, 0x69, 0x6e, 0x65, 0x72, 0x2d, 0x6e, 0x65, + 0x74, 0x77, 0x6f, 0x72, 0x6b, 0x69, 0x6e, 0x67, 0x2f, 0x6e, 0x70, 0x6d, 0x2f, 0x70, 0x6b, 0x67, + 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x3b, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x62, 0x06, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -280,26 +340,28 @@ func file_transport_proto_rawDescGZIP() []byte { return file_transport_proto_rawDescData } -var file_transport_proto_enumTypes = make([]protoimpl.EnumInfo, 1) +var file_transport_proto_enumTypes = make([]protoimpl.EnumInfo, 2) var file_transport_proto_msgTypes = make([]protoimpl.MessageInfo, 4) var file_transport_proto_goTypes = []interface{}{ (DatapathPodMetadata_APIVersion)(0), // 0: protos.DatapathPodMetadata.APIVersion - (*DatapathPodMetadata)(nil), // 1: protos.DatapathPodMetadata - (*Events)(nil), // 2: protos.Events - (*GoalState)(nil), // 3: protos.GoalState - nil, // 4: protos.Events.PayloadEntry + (Events_EventType)(0), // 1: protos.Events.EventType + (*DatapathPodMetadata)(nil), // 2: protos.DatapathPodMetadata + (*Events)(nil), // 3: protos.Events + (*GoalState)(nil), // 4: protos.GoalState + nil, // 5: protos.Events.PayloadEntry } var file_transport_proto_depIdxs = []int32{ 0, // 0: protos.DatapathPodMetadata.apiVersion:type_name -> protos.DatapathPodMetadata.APIVersion - 4, // 1: protos.Events.payload:type_name -> protos.Events.PayloadEntry - 3, // 2: protos.Events.PayloadEntry.value:type_name -> protos.GoalState - 1, // 3: protos.DataplaneEvents.Connect:input_type -> protos.DatapathPodMetadata - 2, // 4: protos.DataplaneEvents.Connect:output_type -> protos.Events - 4, // [4:5] is the sub-list for method output_type - 3, // [3:4] is the sub-list for method input_type - 3, // [3:3] is the sub-list for extension type_name - 3, // [3:3] is the sub-list for extension extendee - 0, // [0:3] is the sub-list for field type_name + 1, // 1: protos.Events.eventType:type_name -> protos.Events.EventType + 5, // 2: protos.Events.payload:type_name -> protos.Events.PayloadEntry + 4, // 3: protos.Events.PayloadEntry.value:type_name -> protos.GoalState + 2, // 4: protos.DataplaneEvents.Connect:input_type -> protos.DatapathPodMetadata + 3, // 5: protos.DataplaneEvents.Connect:output_type -> protos.Events + 5, // [5:6] is the sub-list for method output_type + 4, // [4:5] is the sub-list for method input_type + 4, // [4:4] is the sub-list for extension type_name + 4, // [4:4] is the sub-list for extension extendee + 0, // [0:4] is the sub-list for field type_name } func init() { file_transport_proto_init() } @@ -350,7 +412,7 @@ func file_transport_proto_init() { File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_transport_proto_rawDesc, - NumEnums: 1, + NumEnums: 2, NumMessages: 4, NumExtensions: 0, NumServices: 1, diff --git a/npm/pkg/protos/transport.proto b/npm/pkg/protos/transport.proto index ed420996db..44ba34447d 100644 --- a/npm/pkg/protos/transport.proto +++ b/npm/pkg/protos/transport.proto @@ -21,8 +21,14 @@ message DatapathPodMetadata { // streamed to the datapath client. A events message may carry one or // more Event objects. message Events { + enum EventType + { + Hydration = 0; + GoalState = 1; + }; + EventType eventType = 1; // Payload can contain one or more Event objects. - map payload = 1; + map payload = 2; } // Event is a generic object that can be Created, From 0d13c3284ca1e8d714470bd4e69413e981945356 Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 15 Feb 2022 13:11:06 -0800 Subject: [PATCH 2/6] Adding logic in GSP to delete old cached objects --- .../goalstateprocessor/goalstateprocessor.go | 130 ++++++++++++++---- 1 file changed, 101 insertions(+), 29 deletions(-) diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go index 68e8008771..dcbf264a57 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go @@ -69,6 +69,8 @@ func (gsp *GoalStateProcessor) run(stopCh <-chan struct{}) { func (gsp *GoalStateProcessor) processNext(stopCh <-chan struct{}) bool { select { + // TODO benchmark how many events can stay in pipeline as we work + // on a previous event case inputEvents := <-gsp.inputChannel: // TODO remove this large print later klog.Infof("Received event %s", inputEvents) @@ -121,7 +123,75 @@ func (gsp *GoalStateProcessor) process(inputEvent *protos.Events) { } func (gsp *GoalStateProcessor) processHydrationEvent(payload map[string]*protos.GoalState) { + // Hydration events are sent when the daemon first starts up, or a reconnection to controller happens. + // In this case, the controller will send a current state of the cache down to daemon. + // Daemon will need to calculate what updates and deleted have been missed and send them to the dataplane. + // Sequence of processing will be: + // Apply IPsets + // Apply Policies + // Get all existing IPSets and policies in the dataplane + // Delete cached Policies not in event + // Delete cached IPSets (without references) not in the event + + var appendedIPSets map[string]struct{} + var appendedPolicies map[string]struct{} + var err error + + if ipsetApplyPayload, ok := payload[cp.IpsetApply]; ok { + appendedIPSets, err = gsp.processIPSetsApplyEvent(ipsetApplyPayload) + if err != nil { + klog.Errorf("Error processing IPSET apply HYDRATION event %s", err) + } + } + + if policyApplyPayload, ok := payload[cp.PolicyApply]; ok { + appendedPolicies, err = gsp.processPolicyApplyEvent(policyApplyPayload) + if err != nil { + klog.Errorf("Error processing POLICY apply HYDRATION event %s", err) + } + } + + cachedPolicyKeys := gsp.dp.GetAllPolicies() + toDeletePolicies := make([]string, 0) + if appendedPolicies == nil { + toDeletePolicies = cachedPolicyKeys + } else { + for _, policy := range cachedPolicyKeys { + if _, ok := appendedPolicies[policy]; !ok { + toDeletePolicies = append(toDeletePolicies, policy) + } + } + } + + if len(toDeletePolicies) > 0 { + klog.Infof("Deleting %d policies", len(toDeletePolicies)) + err = gsp.processPolicyRemoveEvent(toDeletePolicies) + if err != nil { + klog.Errorf("Error processing POLICY remove HYDRATION event %s", err) + } + } + + cachedIPSetNames := gsp.dp.GetAllIPSets() + toDeleteIPSets := make([]string, 0) + + if appendedIPSets == nil { + toDeleteIPSets = cachedIPSetNames + } else { + for _, ipset := range cachedIPSetNames { + if _, ok := appendedIPSets[ipset]; !ok { + toDeleteIPSets = append(toDeleteIPSets, ipset) + } + } + } + + if len(toDeleteIPSets) > 0 { + klog.Infof("Deleting %d ipsets", len(toDeleteIPSets)) + err = gsp.processIPSetsRemoveEvent(toDeleteIPSets) + if err != nil { + klog.Errorf("Error processing IPSET remove HYDRATION event %s", err) + } + } } func (gsp *GoalStateProcessor) processGoalStateEvent(payload map[string]*protos.GoalState) { @@ -131,43 +201,53 @@ func (gsp *GoalStateProcessor) processGoalStateEvent(payload map[string]*protos. // 3. Remove POLICY // 4. Remove IPSET if ipsetApplyPayload, ok := payload[cp.IpsetApply]; ok { - err := gsp.processIPSetsApplyEvent(ipsetApplyPayload) + _, err := gsp.processIPSetsApplyEvent(ipsetApplyPayload) if err != nil { klog.Errorf("Error processing IPSET apply event %s", err) } } if policyApplyPayload, ok := payload[cp.PolicyApply]; ok { - err := gsp.processPolicyApplyEvent(policyApplyPayload) + _, err := gsp.processPolicyApplyEvent(policyApplyPayload) if err != nil { klog.Errorf("Error processing POLICY apply event %s", err) } } if policyRemovePayload, ok := payload[cp.PolicyRemove]; ok { - err := gsp.processPolicyRemoveEvent(policyRemovePayload) + payload := bytes.NewBuffer(policyRemovePayload.GetData()) + netpolNames, err := cp.DecodeStrings(payload) + if err != nil { + klog.Errorf("Error processing POLICY remove event, failed to decode Policy remove event %s", err) + } + err = gsp.processPolicyRemoveEvent(netpolNames) if err != nil { klog.Errorf("Error processing POLICY remove event %s", err) } } if ipsetRemovePayload, ok := payload[cp.IpsetRemove]; ok { - err := gsp.processIPSetsRemoveEvent(ipsetRemovePayload) + payload := bytes.NewBuffer(ipsetRemovePayload.GetData()) + ipsetNames, err := cp.DecodeStrings(payload) + if err != nil { + klog.Errorf("Error processing IPSET remove event, failed to decode IPSet remove event", err) + } + err = gsp.processIPSetsRemoveEvent(ipsetNames) if err != nil { klog.Errorf("Error processing IPSET remove event %s", err) } } } -func (gsp *GoalStateProcessor) processIPSetsApplyEvent(goalState *protos.GoalState) error { +func (gsp *GoalStateProcessor) processIPSetsApplyEvent(goalState *protos.GoalState) (map[string]struct{}, error) { payload := bytes.NewBuffer(goalState.GetData()) payloadIPSets, err := cp.DecodeControllerIPSets(payload) if err != nil { - return npmerrors.SimpleErrorWrapper("failed to decode IPSet apply event", err) + return nil, npmerrors.SimpleErrorWrapper("failed to decode IPSet apply event", err) } klog.Infof("Processing IPSet apply event %v", payloadIPSets) - + appendedIPSets := make(map[string]struct{}, len(payloadIPSets)) for _, ipset := range payloadIPSets { if ipset == nil { klog.Warningf("Empty IPSet apply event") @@ -188,20 +268,21 @@ func (gsp *GoalStateProcessor) processIPSetsApplyEvent(goalState *protos.GoalSta case ipsets.HashSet: err = gsp.applySets(ipset, cachedIPSet) if err != nil { - return err + return nil, err } case ipsets.ListSet: err = gsp.applyLists(ipset, cachedIPSet) if err != nil { - return err + return nil, err } case ipsets.UnknownKind: - return npmerrors.SimpleError( + return nil, npmerrors.SimpleError( fmt.Sprintf("failed to decode IPSet apply event: Unknown IPSet kind %s", cachedIPSet.Kind), ) } + appendedIPSets[ipsetName] = struct{}{} } - return nil + return appendedIPSets, nil } func (gsp *GoalStateProcessor) applySets(ipSet *cp.ControllerIPSets, cachedIPSet *ipsets.IPSet) error { @@ -267,13 +348,7 @@ func (gsp *GoalStateProcessor) applyLists(ipSet *cp.ControllerIPSets, cachedIPSe return nil } -func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(goalState *protos.GoalState) error { - payload := bytes.NewBuffer(goalState.GetData()) - ipsetNames, err := cp.DecodeStrings(payload) - if err != nil { - return npmerrors.SimpleErrorWrapper("failed to decode IPSet remove event", err) - } - +func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(ipsetNames []string) error { for _, ipsetName := range ipsetNames { if ipsetName == "" { klog.Warningf("Empty IPSet remove event") @@ -292,13 +367,14 @@ func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(goalState *protos.GoalSt return nil } -func (gsp *GoalStateProcessor) processPolicyApplyEvent(goalState *protos.GoalState) error { +func (gsp *GoalStateProcessor) processPolicyApplyEvent(goalState *protos.GoalState) (map[string]struct{}, error) { payload := bytes.NewBuffer(goalState.GetData()) netpols, err := cp.DecodeNPMNetworkPolicies(payload) if err != nil { - return npmerrors.SimpleErrorWrapper("failed to decode Policy apply event", err) + return nil, npmerrors.SimpleErrorWrapper("failed to decode Policy apply event", err) } + appendedPolicies := make(map[string]struct{}, len(netpols)) for _, netpol := range netpols { if netpol == nil { klog.Warningf("Empty Policy apply event") @@ -310,18 +386,14 @@ func (gsp *GoalStateProcessor) processPolicyApplyEvent(goalState *protos.GoalSta err = gsp.dp.UpdatePolicy(netpol) if err != nil { klog.Errorf("Error applying policy %s to dataplane with error: %s", netpol.Name, err.Error()) - return npmerrors.SimpleErrorWrapper("failed update policy event", err) + return nil, npmerrors.SimpleErrorWrapper("failed update policy event", err) } + appendedPolicies[netpol.PolicyKey] = struct{}{} } - return nil + return appendedPolicies, nil } -func (gsp *GoalStateProcessor) processPolicyRemoveEvent(goalState *protos.GoalState) error { - payload := bytes.NewBuffer(goalState.GetData()) - netpolNames, err := cp.DecodeStrings(payload) - if err != nil { - return npmerrors.SimpleErrorWrapper("failed to decode Policy remove event", err) - } +func (gsp *GoalStateProcessor) processPolicyRemoveEvent(netpolNames []string) error { for _, netpolName := range netpolNames { klog.Infof("Processing %s Policy remove event", netpolName) @@ -330,7 +402,7 @@ func (gsp *GoalStateProcessor) processPolicyRemoveEvent(goalState *protos.GoalSt continue } - err = gsp.dp.RemovePolicy(netpolName) + err := gsp.dp.RemovePolicy(netpolName) if err != nil { klog.Errorf("Error removing policy %s from dataplane with error: %s", netpolName, err.Error()) return npmerrors.SimpleErrorWrapper("failed remove policy event", err) From 4b4fd86f32612206f24b685b75626417d9d6294b Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 15 Feb 2022 13:48:40 -0800 Subject: [PATCH 3/6] Adding support for ForceDelete IpSet --- .../controllers/v2/namespaceController.go | 2 +- .../v2/namespaceController_test.go | 4 +-- .../goalstateprocessor/goalstateprocessor.go | 19 +++++--------- npm/pkg/dataplane/dataplane.go | 6 ++--- npm/pkg/dataplane/dataplane_test.go | 6 ++--- npm/pkg/dataplane/dpshim/dpshim.go | 2 +- npm/pkg/dataplane/ipsets/ipsetmanager.go | 14 +++++++--- .../ipsets/ipsetmanager_linux_test.go | 26 +++++++++---------- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 14 +++++----- .../ipsets/ipsetmanager_windows_test.go | 16 ++++++------ .../mocks/genericdataplane_generated.go | 8 +++--- npm/pkg/dataplane/types.go | 2 +- test/integration/npm/main.go | 4 +-- 13 files changed, 62 insertions(+), 61 deletions(-) diff --git a/npm/pkg/controlplane/controllers/v2/namespaceController.go b/npm/pkg/controlplane/controllers/v2/namespaceController.go index 99f3a7e746..8f94090ceb 100644 --- a/npm/pkg/controlplane/controllers/v2/namespaceController.go +++ b/npm/pkg/controlplane/controllers/v2/namespaceController.go @@ -471,7 +471,7 @@ func (nsc *NamespaceController) cleanDeletedNamespace(cachedNsKey string) error } // Delete ipset for the namespace. - nsc.dp.DeleteIPSet(ipsets.NewIPSetMetadata(cachedNsKey, ipsets.Namespace)) + nsc.dp.DeleteIPSet(ipsets.NewIPSetMetadata(cachedNsKey, ipsets.Namespace), false) delete(nsc.npmNamespaceCache.NsMap, cachedNsKey) diff --git a/npm/pkg/controlplane/controllers/v2/namespaceController_test.go b/npm/pkg/controlplane/controllers/v2/namespaceController_test.go index 6449bc91c4..2be50a6b29 100644 --- a/npm/pkg/controlplane/controllers/v2/namespaceController_test.go +++ b/npm/pkg/controlplane/controllers/v2/namespaceController_test.go @@ -623,7 +623,7 @@ func TestDeleteNamespace(t *testing.T) { for i := 1; i < len(setsToAddNamespaceTo); i++ { dp.EXPECT().RemoveFromList(setsToAddNamespaceTo[i], setsToAddNamespaceTo[:1]).Return(nil).Times(1) } - dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0]).Return().Times(1) + dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0], false).Return().Times(1) deleteNamespace(t, f, nsObj, DeletedFinalStateknownObject) @@ -702,7 +702,7 @@ func TestDeleteNamespaceWithTombstoneAfterAddingNameSpace(t *testing.T) { for i := 1; i < len(setsToAddNamespaceTo); i++ { dp.EXPECT().RemoveFromList(setsToAddNamespaceTo[i], setsToAddNamespaceTo[:1]).Return(nil).Times(1) } - dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0]).Return().Times(1) + dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0], false).Return().Times(1) deleteNamespace(t, f, nsObj, DeletedFinalStateUnknownObject) testCases := []expectedNsValues{ diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go index dcbf264a57..f5ffdcbf47 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go @@ -187,10 +187,7 @@ func (gsp *GoalStateProcessor) processHydrationEvent(payload map[string]*protos. if len(toDeleteIPSets) > 0 { klog.Infof("Deleting %d ipsets", len(toDeleteIPSets)) - err = gsp.processIPSetsRemoveEvent(toDeleteIPSets) - if err != nil { - klog.Errorf("Error processing IPSET remove HYDRATION event %s", err) - } + gsp.processIPSetsRemoveEvent(toDeleteIPSets, true) } } @@ -230,12 +227,9 @@ func (gsp *GoalStateProcessor) processGoalStateEvent(payload map[string]*protos. payload := bytes.NewBuffer(ipsetRemovePayload.GetData()) ipsetNames, err := cp.DecodeStrings(payload) if err != nil { - klog.Errorf("Error processing IPSET remove event, failed to decode IPSet remove event", err) - } - err = gsp.processIPSetsRemoveEvent(ipsetNames) - if err != nil { - klog.Errorf("Error processing IPSET remove event %s", err) + klog.Errorf("Error processing IPSET remove event, failed to decode IPSet remove event: %s", err) } + gsp.processIPSetsRemoveEvent(ipsetNames, false) } } @@ -348,7 +342,7 @@ func (gsp *GoalStateProcessor) applyLists(ipSet *cp.ControllerIPSets, cachedIPSe return nil } -func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(ipsetNames []string) error { +func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(ipsetNames []string, forceDelete bool) { for _, ipsetName := range ipsetNames { if ipsetName == "" { klog.Warningf("Empty IPSet remove event") @@ -359,12 +353,11 @@ func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(ipsetNames []string) err cachedIPSet := gsp.dp.GetIPSet(ipsetName) if cachedIPSet == nil { klog.Infof("IPSet %s not found in cache, ignoring delete call.", ipsetName) - return nil + continue } - gsp.dp.DeleteIPSet(ipsets.NewIPSetMetadata(cachedIPSet.Name, cachedIPSet.Type)) + gsp.dp.DeleteIPSet(ipsets.NewIPSetMetadata(cachedIPSet.Name, cachedIPSet.Type), forceDelete) } - return nil } func (gsp *GoalStateProcessor) processPolicyApplyEvent(goalState *protos.GoalState) (map[string]struct{}, error) { diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index fba6e83164..fa13f663ff 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -92,8 +92,8 @@ func (dp *DataPlane) CreateIPSets(setMetadata []*ipsets.IPSetMetadata) { // DeleteSet checks for members and references of the given "set" type ipset // if not used then will delete it from cache -func (dp *DataPlane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata) { - dp.ipsetMgr.DeleteIPSet(setMetadata.GetPrefixName()) +func (dp *DataPlane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, forceDelete bool) { + dp.ipsetMgr.DeleteIPSet(setMetadata.GetPrefixName(), forceDelete) } // AddToSets takes in a list of IPSet names along with IP member @@ -375,7 +375,7 @@ func (dp *DataPlane) deleteIPSetsAndReferences(sets []*ipsets.TranslatedIPSet, n } // Try to delete these IPSets - dp.ipsetMgr.DeleteIPSet(set.Metadata.GetPrefixName()) + dp.ipsetMgr.DeleteIPSet(set.Metadata.GetPrefixName(), false) } return nil } diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index b40966e5ab..b7e33aeceb 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -115,7 +115,7 @@ func TestCreateAndDeleteIpSets(t *testing.T) { } for _, v := range setsTocreate { - dp.DeleteIPSet(v) + dp.DeleteIPSet(v, false) } for _, v := range setsTocreate { @@ -163,7 +163,7 @@ func TestAddToSet(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - dp.DeleteIPSet(v) + dp.DeleteIPSet(v, false) } for _, v := range setsTocreate { @@ -179,7 +179,7 @@ func TestAddToSet(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - dp.DeleteIPSet(v) + dp.DeleteIPSet(v, false) } for _, v := range setsTocreate { diff --git a/npm/pkg/dataplane/dpshim/dpshim.go b/npm/pkg/dataplane/dpshim/dpshim.go index 720e42ebac..9bca90ce47 100644 --- a/npm/pkg/dataplane/dpshim/dpshim.go +++ b/npm/pkg/dataplane/dpshim/dpshim.go @@ -124,7 +124,7 @@ func (dp *DPShim) createIPSet(set *ipsets.IPSetMetadata) { dp.dirtyCache.modifyAddorUpdateSets(setName) } -func (dp *DPShim) DeleteIPSet(setMetadata *ipsets.IPSetMetadata) { +func (dp *DPShim) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, _ bool) { dp.lock() defer dp.unlock() dp.deleteIPSet(setMetadata) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 32f904ee6e..3b3497514f 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -86,7 +86,7 @@ func (iMgr *IPSetManager) createIPSet(setMetadata *IPSetMetadata) { } // DeleteIPSet expects the prefixed ipset name -func (iMgr *IPSetManager) DeleteIPSet(name string) { +func (iMgr *IPSetManager) DeleteIPSet(name string, force bool) { iMgr.Lock() defer iMgr.Unlock() if !iMgr.exists(name) { @@ -94,8 +94,16 @@ func (iMgr *IPSetManager) DeleteIPSet(name string) { } set := iMgr.setMap[name] - if !set.canBeDeleted() { - return + if force { + // If force delete, then check if Set is used by other set or network policy + // else delete the set even if it has members + if set.shouldBeInKernel() { + return + } + } else { + if !set.canBeDeleted() { + return + } } delete(iMgr.setMap, name) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go index 81a1dffe27..e82566076e 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go @@ -465,7 +465,7 @@ func TestApplyIPSetsSuccessWithoutSave(t *testing.T) { // delete a set so the file isn't empty (otherwise the creator won't even call the exec command) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestNSSet.PrefixName) + iMgr.DeleteIPSet(TestNSSet.PrefixName, false) err := iMgr.applyIPSets() require.NoError(t, err) } @@ -625,9 +625,9 @@ func TestDestroy(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKeyNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata, TestKeyPodSet.Metadata})) require.NoError(t, iMgr.RemoveFromList(TestKeyNSList.Metadata, []*IPSetMetadata{TestKeyPodSet.Metadata})) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestNestedLabelList.PrefixName) + iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, false) creator := iMgr.fileCreatorForApply(len(calls), nil) actualLines := testAndSortRestoreFileString(t, creator.ToString()) @@ -743,7 +743,7 @@ func TestUpdateWithRealisticSaveFile(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestKVNSList.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestCIDRSet.Metadata}, "1.2.3.4", "z")) // set not in save file iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestNestedLabelList.PrefixName) + iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, false) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) actualLines := testAndSortRestoreFileString(t, creator.ToString()) // adding NSSet and KeyPodSet (should be keeping NSSet and deleting NamedportSet) @@ -1042,7 +1042,7 @@ func TestFailureOnCreateForNewSet(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNamedportSet.Metadata}, "1.2.3.4,tcp:567", "a")) // create and add member require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNamedportSet.Metadata}, "1.2.3.5,tcp:567", "b")) // add member iMgr.CreateIPSets([]*IPSetMetadata{TestKeyNSList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKeyNSList.PrefixName) + iMgr.DeleteIPSet(TestKeyNSList.PrefixName, false) // get original creator and run it the first time creator := iMgr.fileCreatorForApply(len(calls), nil) @@ -1099,7 +1099,7 @@ func TestFailureOnCreateForSetInKernel(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "6.7.8.9", "a")) // add member to kernel require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKVPodSet.Metadata}, "6.7.8.9", "a")) // add member to kernel iMgr.CreateIPSets([]*IPSetMetadata{TestKeyNSList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKeyNSList.PrefixName) + iMgr.DeleteIPSet(TestKeyNSList.PrefixName, false) // get original creator and run it the first time creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) @@ -1160,7 +1160,7 @@ func TestFailureOnAddToListInKernel(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestKeyPodSet.Metadata})) // add member to kernel require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestNestedLabelList.Metadata}, []*IPSetMetadata{TestKeyPodSet.Metadata})) // add member to kernel iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1212,7 +1212,7 @@ func TestFailureOnAddToNewList(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata})) // add member to kernel require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestNestedLabelList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata})) // add member to kernel iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1266,9 +1266,9 @@ func TestFailureOnFlush(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) // in kernel already require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.0", "a")) // not in kernel yet iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKVPodSet.PrefixName) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1319,9 +1319,9 @@ func TestFailureOnDestroy(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) // in kernel already require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.0", "a")) // not in kernel yet iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKVPodSet.PrefixName) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1358,7 +1358,7 @@ func TestFailureOnLastLine(t *testing.T) { iMgr := NewIPSetManager(applyAlwaysCfg, ioshim) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) creator := iMgr.fileCreatorForApply(2, nil) wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 1a644d9c90..f2dd1f711a 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -267,7 +267,7 @@ func TestDeleteIPSet(t *testing.T) { iMgr := NewIPSetManager(tt.args.cfg, ioShim) iMgr.CreateIPSets(tt.args.toCreateMetadatas) require.NoError(t, iMgr.ApplyIPSets()) - iMgr.DeleteIPSet(tt.args.toDeleteName) + iMgr.DeleteIPSet(tt.args.toDeleteName, false) assertExpectedInfo(t, iMgr, &tt.expectedInfo) }) } @@ -285,8 +285,8 @@ func TestDeleteIPSetNotAllowed(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{list}, []*IPSetMetadata{namespaceSet})) require.NoError(t, iMgr.ApplyIPSets()) - iMgr.DeleteIPSet(namespaceSet.GetPrefixName()) - iMgr.DeleteIPSet(list.GetPrefixName()) + iMgr.DeleteIPSet(namespaceSet.GetPrefixName(), false) + iMgr.DeleteIPSet(list.GetPrefixName(), false) assertExpectedInfo(t, iMgr, &expectedInfo{ mainCache: []setMembers{ @@ -740,7 +740,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { require.Equal(t, 5, len(iMgr.toDeleteCache)) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName()) + iMgr.DeleteIPSet(v.GetPrefixName(), false) } // Above delete will not remove setpod3 and setpod4 @@ -751,7 +751,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName()) + iMgr.DeleteIPSet(v.GetPrefixName(), false) } for _, v := range setsTocreate { @@ -806,7 +806,7 @@ func TestAddDeleteNetPolReferences(t *testing.T) { require.Equal(t, 5, len(iMgr.toDeleteCache)) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName()) + iMgr.DeleteIPSet(v.GetPrefixName(), false) } // Above delete will not remove setpod3 and setpod4 @@ -817,7 +817,7 @@ func TestAddDeleteNetPolReferences(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName()) + iMgr.DeleteIPSet(v.GetPrefixName(), false) } for _, v := range setsTocreate { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go index 7b8776ee24..406867478b 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go @@ -132,9 +132,9 @@ func TestApplyDeletions(t *testing.T) { require.NoError(t, iMgr.RemoveFromSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.1", "b")) require.NoError(t, iMgr.RemoveFromList(TestKeyNSList.Metadata, []*IPSetMetadata{TestKeyPodSet.Metadata})) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) - iMgr.DeleteIPSet(TestNestedLabelList.PrefixName) + iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, false) toDeleteSetNames := []string{TestCIDRSet.PrefixName, TestNestedLabelList.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -176,7 +176,7 @@ func TestFailureOnCreation(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestKeyPodSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.5", "c")) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) toDeleteSetNames := []string{TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -215,7 +215,7 @@ func TestFailureOnAddToList(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestKVNSList.Metadata}) require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata})) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) toDeleteSetNames := []string{TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -261,9 +261,9 @@ func TestFailureOnFlush(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) - iMgr.DeleteIPSet(TestKVPodSet.PrefixName) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -290,9 +290,9 @@ func TestFailureOnDeletion(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) - iMgr.DeleteIPSet(TestKVPodSet.PrefixName) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ diff --git a/npm/pkg/dataplane/mocks/genericdataplane_generated.go b/npm/pkg/dataplane/mocks/genericdataplane_generated.go index 3b28bbef29..45bff6404d 100644 --- a/npm/pkg/dataplane/mocks/genericdataplane_generated.go +++ b/npm/pkg/dataplane/mocks/genericdataplane_generated.go @@ -122,15 +122,15 @@ func (mr *MockGenericDataplaneMockRecorder) CreateIPSets(setMetadatas interface{ } // DeleteIPSet mocks base method. -func (m *MockGenericDataplane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata) { +func (m *MockGenericDataplane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, force bool) { m.ctrl.T.Helper() - m.ctrl.Call(m, "DeleteIPSet", setMetadata) + m.ctrl.Call(m, "DeleteIPSet", setMetadata, force) } // DeleteIPSet indicates an expected call of DeleteIPSet. -func (mr *MockGenericDataplaneMockRecorder) DeleteIPSet(setMetadata interface{}) *gomock.Call { +func (mr *MockGenericDataplaneMockRecorder) DeleteIPSet(setMetadata, force interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).DeleteIPSet), setMetadata) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).DeleteIPSet), setMetadata, force) } // GetAllIPSets mocks base method. diff --git a/npm/pkg/dataplane/types.go b/npm/pkg/dataplane/types.go index a140ad3486..d7dac24b5d 100644 --- a/npm/pkg/dataplane/types.go +++ b/npm/pkg/dataplane/types.go @@ -11,7 +11,7 @@ type GenericDataplane interface { GetAllIPSets() []string GetIPSet(setName string) *ipsets.IPSet CreateIPSets(setMetadatas []*ipsets.IPSetMetadata) - DeleteIPSet(setMetadata *ipsets.IPSetMetadata) + DeleteIPSet(setMetadata *ipsets.IPSetMetadata, force bool) AddToSets(setMetadatas []*ipsets.IPSetMetadata, podMetadata *PodMetadata) error RemoveFromSets(setMetadatas []*ipsets.IPSetMetadata, podMetadata *PodMetadata) error AddToLists(listMetadatas []*ipsets.IPSetMetadata, setMetadatas []*ipsets.IPSetMetadata) error diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index f0eba83ef7..6e95deceff 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -119,7 +119,7 @@ func main() { NodeName: "", } panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataD)) - dp.DeleteIPSet(ipsets.TestKVPodSet.Metadata) + dp.DeleteIPSet(ipsets.TestKVPodSet.Metadata, false) panicOnError(dp.ApplyDataPlane()) if includeLists { @@ -129,7 +129,7 @@ func main() { printAndWait(true) panicOnError(dp.RemoveFromSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadata)) - dp.DeleteIPSet(ipsets.TestNSSet.Metadata) + dp.DeleteIPSet(ipsets.TestNSSet.Metadata, false) panicOnError(dp.ApplyDataPlane()) printAndWait(true) From fc815b237784763d8a2c78654146e22e97ac5229 Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 15 Feb 2022 14:27:02 -0800 Subject: [PATCH 4/6] Adding missing event type --- .../controlplane/goalstateprocessor/goalstateprocessor_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go index 6145bcf870..0d56a4963a 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go @@ -96,6 +96,7 @@ func TestPolicyApplyEvent(t *testing.T) { go func() { inputChan <- &protos.Events{ + EventType: protos.Events_GoalState, Payload: map[string]*protos.GoalState{ controlplane.PolicyApply: { Data: payload.Bytes(), From b5bb3755bec99ad1d4f629c28a83e3f34b8bf337 Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 15 Feb 2022 15:03:20 -0800 Subject: [PATCH 5/6] Switching order of event type enum --- .../goalstateprocessor_test.go | 6 ++++-- npm/pkg/protos/transport.pb.go | 18 +++++++++--------- npm/pkg/protos/transport.proto | 4 ++-- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go index 0d56a4963a..6c7945ab58 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor_test.go @@ -175,7 +175,8 @@ func TestIPSetsApplyUpdateMembers(t *testing.T) { gsp, _ := NewGoalStateProcessor(ctx, "node1", "pod1", inputChan, dp) go func() { inputChan <- &protos.Events{ - Payload: goalState, + EventType: protos.Events_GoalState, + Payload: goalState, } }() time.Sleep(sleepAfterChanSent) @@ -193,7 +194,8 @@ func TestIPSetsApplyUpdateMembers(t *testing.T) { ) go func() { inputChan <- &protos.Events{ - Payload: goalState, + EventType: protos.Events_GoalState, + Payload: goalState, } }() time.Sleep(sleepAfterChanSent) diff --git a/npm/pkg/protos/transport.pb.go b/npm/pkg/protos/transport.pb.go index 370e27cea2..baddb578ef 100644 --- a/npm/pkg/protos/transport.pb.go +++ b/npm/pkg/protos/transport.pb.go @@ -66,19 +66,19 @@ func (DatapathPodMetadata_APIVersion) EnumDescriptor() ([]byte, []int) { type Events_EventType int32 const ( - Events_Hydration Events_EventType = 0 - Events_GoalState Events_EventType = 1 + Events_GoalState Events_EventType = 0 + Events_Hydration Events_EventType = 1 ) // Enum value maps for Events_EventType. var ( Events_EventType_name = map[int32]string{ - 0: "Hydration", - 1: "GoalState", + 0: "GoalState", + 1: "Hydration", } Events_EventType_value = map[string]int32{ - "Hydration": 0, - "GoalState": 1, + "GoalState": 0, + "Hydration": 1, } ) @@ -222,7 +222,7 @@ func (x *Events) GetEventType() Events_EventType { if x != nil { return x.EventType } - return Events_Hydration + return Events_GoalState } func (x *Events) GetPayload() map[string]*GoalState { @@ -312,8 +312,8 @@ var file_transport_proto_rawDesc = []byte{ 0x28, 0x0b, 0x32, 0x11, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2e, 0x47, 0x6f, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, 0x29, 0x0a, 0x09, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x54, 0x79, 0x70, 0x65, 0x12, 0x0d, 0x0a, - 0x09, 0x48, 0x79, 0x64, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x10, 0x00, 0x12, 0x0d, 0x0a, 0x09, - 0x47, 0x6f, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x10, 0x01, 0x22, 0x1f, 0x0a, 0x09, 0x47, + 0x09, 0x47, 0x6f, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x10, 0x00, 0x12, 0x0d, 0x0a, 0x09, + 0x48, 0x79, 0x64, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x10, 0x01, 0x22, 0x1f, 0x0a, 0x09, 0x47, 0x6f, 0x61, 0x6c, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x64, 0x61, 0x74, 0x61, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x04, 0x64, 0x61, 0x74, 0x61, 0x32, 0x4b, 0x0a, 0x0f, 0x44, 0x61, 0x74, 0x61, 0x70, 0x6c, 0x61, 0x6e, 0x65, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x73, 0x12, diff --git a/npm/pkg/protos/transport.proto b/npm/pkg/protos/transport.proto index 44ba34447d..1a318a1638 100644 --- a/npm/pkg/protos/transport.proto +++ b/npm/pkg/protos/transport.proto @@ -23,8 +23,8 @@ message DatapathPodMetadata { message Events { enum EventType { - Hydration = 0; - GoalState = 1; + GoalState = 0; + Hydration = 1; }; EventType eventType = 1; // Payload can contain one or more Event objects. From 678e2f25bb34eb2b8cc2515fa6e3d24f04bba4da Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 16 Feb 2022 11:11:47 -0800 Subject: [PATCH 6/6] Addressing comments --- .../controllers/v2/namespaceController.go | 2 +- .../v2/namespaceController_test.go | 4 +-- .../goalstateprocessor/goalstateprocessor.go | 7 ++--- npm/pkg/dataplane/dataplane.go | 2 +- npm/pkg/dataplane/dataplane_test.go | 7 ++--- npm/pkg/dataplane/dpshim/dpshim.go | 3 ++- npm/pkg/dataplane/ipsets/ipset.go | 5 ++++ npm/pkg/dataplane/ipsets/ipsetmanager.go | 4 +-- .../ipsets/ipsetmanager_linux_test.go | 27 ++++++++++--------- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 14 +++++----- .../ipsets/ipsetmanager_windows_test.go | 16 +++++------ .../mocks/genericdataplane_generated.go | 9 ++++--- npm/pkg/dataplane/types.go | 3 ++- npm/util/util.go | 9 +++++++ test/integration/npm/main.go | 5 ++-- 15 files changed, 69 insertions(+), 48 deletions(-) diff --git a/npm/pkg/controlplane/controllers/v2/namespaceController.go b/npm/pkg/controlplane/controllers/v2/namespaceController.go index 8f94090ceb..0c9930ba2e 100644 --- a/npm/pkg/controlplane/controllers/v2/namespaceController.go +++ b/npm/pkg/controlplane/controllers/v2/namespaceController.go @@ -471,7 +471,7 @@ func (nsc *NamespaceController) cleanDeletedNamespace(cachedNsKey string) error } // Delete ipset for the namespace. - nsc.dp.DeleteIPSet(ipsets.NewIPSetMetadata(cachedNsKey, ipsets.Namespace), false) + nsc.dp.DeleteIPSet(ipsets.NewIPSetMetadata(cachedNsKey, ipsets.Namespace), util.SoftDelete) delete(nsc.npmNamespaceCache.NsMap, cachedNsKey) diff --git a/npm/pkg/controlplane/controllers/v2/namespaceController_test.go b/npm/pkg/controlplane/controllers/v2/namespaceController_test.go index 2be50a6b29..2fc91b0140 100644 --- a/npm/pkg/controlplane/controllers/v2/namespaceController_test.go +++ b/npm/pkg/controlplane/controllers/v2/namespaceController_test.go @@ -623,7 +623,7 @@ func TestDeleteNamespace(t *testing.T) { for i := 1; i < len(setsToAddNamespaceTo); i++ { dp.EXPECT().RemoveFromList(setsToAddNamespaceTo[i], setsToAddNamespaceTo[:1]).Return(nil).Times(1) } - dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0], false).Return().Times(1) + dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0], util.SoftDelete).Return().Times(1) deleteNamespace(t, f, nsObj, DeletedFinalStateknownObject) @@ -702,7 +702,7 @@ func TestDeleteNamespaceWithTombstoneAfterAddingNameSpace(t *testing.T) { for i := 1; i < len(setsToAddNamespaceTo); i++ { dp.EXPECT().RemoveFromList(setsToAddNamespaceTo[i], setsToAddNamespaceTo[:1]).Return(nil).Times(1) } - dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0], false).Return().Times(1) + dp.EXPECT().DeleteIPSet(setsToAddNamespaceTo[0], util.SoftDelete).Return().Times(1) deleteNamespace(t, f, nsObj, DeletedFinalStateUnknownObject) testCases := []expectedNsValues{ diff --git a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go index f5ffdcbf47..c55fb0b9eb 100644 --- a/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go +++ b/npm/pkg/controlplane/goalstateprocessor/goalstateprocessor.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-container-networking/npm/pkg/dataplane" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/protos" + "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "k8s.io/klog" ) @@ -187,7 +188,7 @@ func (gsp *GoalStateProcessor) processHydrationEvent(payload map[string]*protos. if len(toDeleteIPSets) > 0 { klog.Infof("Deleting %d ipsets", len(toDeleteIPSets)) - gsp.processIPSetsRemoveEvent(toDeleteIPSets, true) + gsp.processIPSetsRemoveEvent(toDeleteIPSets, util.ForceDelete) } } @@ -229,7 +230,7 @@ func (gsp *GoalStateProcessor) processGoalStateEvent(payload map[string]*protos. if err != nil { klog.Errorf("Error processing IPSET remove event, failed to decode IPSet remove event: %s", err) } - gsp.processIPSetsRemoveEvent(ipsetNames, false) + gsp.processIPSetsRemoveEvent(ipsetNames, util.SoftDelete) } } @@ -342,7 +343,7 @@ func (gsp *GoalStateProcessor) applyLists(ipSet *cp.ControllerIPSets, cachedIPSe return nil } -func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(ipsetNames []string, forceDelete bool) { +func (gsp *GoalStateProcessor) processIPSetsRemoveEvent(ipsetNames []string, forceDelete util.DeleteOption) { for _, ipsetName := range ipsetNames { if ipsetName == "" { klog.Warningf("Empty IPSet remove event") diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index fa13f663ff..21a3ff68df 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -92,7 +92,7 @@ func (dp *DataPlane) CreateIPSets(setMetadata []*ipsets.IPSetMetadata) { // DeleteSet checks for members and references of the given "set" type ipset // if not used then will delete it from cache -func (dp *DataPlane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, forceDelete bool) { +func (dp *DataPlane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, forceDelete util.DeleteOption) { dp.ipsetMgr.DeleteIPSet(setMetadata.GetPrefixName(), forceDelete) } diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index b7e33aeceb..f03493251b 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" + "github.com/Azure/azure-container-networking/npm/util" testutils "github.com/Azure/azure-container-networking/test/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -115,7 +116,7 @@ func TestCreateAndDeleteIpSets(t *testing.T) { } for _, v := range setsTocreate { - dp.DeleteIPSet(v, false) + dp.DeleteIPSet(v, util.SoftDelete) } for _, v := range setsTocreate { @@ -163,7 +164,7 @@ func TestAddToSet(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - dp.DeleteIPSet(v, false) + dp.DeleteIPSet(v, util.SoftDelete) } for _, v := range setsTocreate { @@ -179,7 +180,7 @@ func TestAddToSet(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - dp.DeleteIPSet(v, false) + dp.DeleteIPSet(v, util.SoftDelete) } for _, v := range setsTocreate { diff --git a/npm/pkg/dataplane/dpshim/dpshim.go b/npm/pkg/dataplane/dpshim/dpshim.go index 9bca90ce47..afb97b57f1 100644 --- a/npm/pkg/dataplane/dpshim/dpshim.go +++ b/npm/pkg/dataplane/dpshim/dpshim.go @@ -12,6 +12,7 @@ import ( "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" "github.com/Azure/azure-container-networking/npm/pkg/protos" + "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "k8s.io/klog" ) @@ -124,7 +125,7 @@ func (dp *DPShim) createIPSet(set *ipsets.IPSetMetadata) { dp.dirtyCache.modifyAddorUpdateSets(setName) } -func (dp *DPShim) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, _ bool) { +func (dp *DPShim) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, _ util.DeleteOption) { dp.lock() defer dp.unlock() dp.deleteIPSet(setMetadata) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index efe3101ade..a8000be5c6 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -332,6 +332,11 @@ func (set *IPSet) shouldBeInKernel() bool { return set.usedByNetPol() || set.referencedInKernel() } +func (set *IPSet) canBeForceDeleted() bool { + return !set.usedByNetPol() && + !set.referencedInList() +} + func (set *IPSet) canBeDeleted() bool { return !set.usedByNetPol() && !set.referencedInList() && diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 3b3497514f..f399cbbb56 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -86,7 +86,7 @@ func (iMgr *IPSetManager) createIPSet(setMetadata *IPSetMetadata) { } // DeleteIPSet expects the prefixed ipset name -func (iMgr *IPSetManager) DeleteIPSet(name string, force bool) { +func (iMgr *IPSetManager) DeleteIPSet(name string, force util.DeleteOption) { iMgr.Lock() defer iMgr.Unlock() if !iMgr.exists(name) { @@ -97,7 +97,7 @@ func (iMgr *IPSetManager) DeleteIPSet(name string, force bool) { if force { // If force delete, then check if Set is used by other set or network policy // else delete the set even if it has members - if set.shouldBeInKernel() { + if !set.canBeForceDeleted() { return } } else { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go index e82566076e..3ef7f356e3 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/metrics/promutil" dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + "github.com/Azure/azure-container-networking/npm/util" testutils "github.com/Azure/azure-container-networking/test/utils" "github.com/stretchr/testify/require" ) @@ -465,7 +466,7 @@ func TestApplyIPSetsSuccessWithoutSave(t *testing.T) { // delete a set so the file isn't empty (otherwise the creator won't even call the exec command) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestNSSet.PrefixName, false) + iMgr.DeleteIPSet(TestNSSet.PrefixName, util.SoftDelete) err := iMgr.applyIPSets() require.NoError(t, err) } @@ -625,9 +626,9 @@ func TestDestroy(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKeyNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata, TestKeyPodSet.Metadata})) require.NoError(t, iMgr.RemoveFromList(TestKeyNSList.Metadata, []*IPSetMetadata{TestKeyPodSet.Metadata})) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, false) + iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, util.SoftDelete) creator := iMgr.fileCreatorForApply(len(calls), nil) actualLines := testAndSortRestoreFileString(t, creator.ToString()) @@ -743,7 +744,7 @@ func TestUpdateWithRealisticSaveFile(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestKVNSList.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestCIDRSet.Metadata}, "1.2.3.4", "z")) // set not in save file iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, false) + iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, util.SoftDelete) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) actualLines := testAndSortRestoreFileString(t, creator.ToString()) // adding NSSet and KeyPodSet (should be keeping NSSet and deleting NamedportSet) @@ -1042,7 +1043,7 @@ func TestFailureOnCreateForNewSet(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNamedportSet.Metadata}, "1.2.3.4,tcp:567", "a")) // create and add member require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNamedportSet.Metadata}, "1.2.3.5,tcp:567", "b")) // add member iMgr.CreateIPSets([]*IPSetMetadata{TestKeyNSList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKeyNSList.PrefixName, false) + iMgr.DeleteIPSet(TestKeyNSList.PrefixName, util.SoftDelete) // get original creator and run it the first time creator := iMgr.fileCreatorForApply(len(calls), nil) @@ -1099,7 +1100,7 @@ func TestFailureOnCreateForSetInKernel(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "6.7.8.9", "a")) // add member to kernel require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKVPodSet.Metadata}, "6.7.8.9", "a")) // add member to kernel iMgr.CreateIPSets([]*IPSetMetadata{TestKeyNSList.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKeyNSList.PrefixName, false) + iMgr.DeleteIPSet(TestKeyNSList.PrefixName, util.SoftDelete) // get original creator and run it the first time creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) @@ -1160,7 +1161,7 @@ func TestFailureOnAddToListInKernel(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestKeyPodSet.Metadata})) // add member to kernel require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestNestedLabelList.Metadata}, []*IPSetMetadata{TestKeyPodSet.Metadata})) // add member to kernel iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1212,7 +1213,7 @@ func TestFailureOnAddToNewList(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata})) // add member to kernel require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestNestedLabelList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata})) // add member to kernel iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1266,9 +1267,9 @@ func TestFailureOnFlush(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) // in kernel already require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.0", "a")) // not in kernel yet iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, util.SoftDelete) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1319,9 +1320,9 @@ func TestFailureOnDestroy(t *testing.T) { require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) // in kernel already require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.0", "a")) // not in kernel yet iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, util.SoftDelete) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) creator := iMgr.fileCreatorForApply(len(calls), saveFileBytes) originalLines := strings.Split(creator.ToString(), "\n") @@ -1358,7 +1359,7 @@ func TestFailureOnLastLine(t *testing.T) { iMgr := NewIPSetManager(applyAlwaysCfg, ioshim) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) // create so we can delete - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) creator := iMgr.fileCreatorForApply(2, nil) wasFileAltered, err := creator.RunCommandOnceWithFile("ipset", "restore") diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index f2dd1f711a..b80bd7b731 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -267,7 +267,7 @@ func TestDeleteIPSet(t *testing.T) { iMgr := NewIPSetManager(tt.args.cfg, ioShim) iMgr.CreateIPSets(tt.args.toCreateMetadatas) require.NoError(t, iMgr.ApplyIPSets()) - iMgr.DeleteIPSet(tt.args.toDeleteName, false) + iMgr.DeleteIPSet(tt.args.toDeleteName, util.SoftDelete) assertExpectedInfo(t, iMgr, &tt.expectedInfo) }) } @@ -285,8 +285,8 @@ func TestDeleteIPSetNotAllowed(t *testing.T) { require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{list}, []*IPSetMetadata{namespaceSet})) require.NoError(t, iMgr.ApplyIPSets()) - iMgr.DeleteIPSet(namespaceSet.GetPrefixName(), false) - iMgr.DeleteIPSet(list.GetPrefixName(), false) + iMgr.DeleteIPSet(namespaceSet.GetPrefixName(), util.SoftDelete) + iMgr.DeleteIPSet(list.GetPrefixName(), util.SoftDelete) assertExpectedInfo(t, iMgr, &expectedInfo{ mainCache: []setMembers{ @@ -740,7 +740,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { require.Equal(t, 5, len(iMgr.toDeleteCache)) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName(), false) + iMgr.DeleteIPSet(v.GetPrefixName(), util.SoftDelete) } // Above delete will not remove setpod3 and setpod4 @@ -751,7 +751,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName(), false) + iMgr.DeleteIPSet(v.GetPrefixName(), util.SoftDelete) } for _, v := range setsTocreate { @@ -806,7 +806,7 @@ func TestAddDeleteNetPolReferences(t *testing.T) { require.Equal(t, 5, len(iMgr.toDeleteCache)) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName(), false) + iMgr.DeleteIPSet(v.GetPrefixName(), util.SoftDelete) } // Above delete will not remove setpod3 and setpod4 @@ -817,7 +817,7 @@ func TestAddDeleteNetPolReferences(t *testing.T) { require.NoError(t, err) for _, v := range setsTocreate { - iMgr.DeleteIPSet(v.GetPrefixName(), false) + iMgr.DeleteIPSet(v.GetPrefixName(), util.SoftDelete) } for _, v := range setsTocreate { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go index 406867478b..c0f2c52ca2 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go @@ -132,9 +132,9 @@ func TestApplyDeletions(t *testing.T) { require.NoError(t, iMgr.RemoveFromSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.1", "b")) require.NoError(t, iMgr.RemoveFromList(TestKeyNSList.Metadata, []*IPSetMetadata{TestKeyPodSet.Metadata})) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) - iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, false) + iMgr.DeleteIPSet(TestNestedLabelList.PrefixName, util.SoftDelete) toDeleteSetNames := []string{TestCIDRSet.PrefixName, TestNestedLabelList.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -176,7 +176,7 @@ func TestFailureOnCreation(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestKeyPodSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.5", "c")) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) toDeleteSetNames := []string{TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -215,7 +215,7 @@ func TestFailureOnAddToList(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestKVNSList.Metadata}) require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata})) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) toDeleteSetNames := []string{TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -261,9 +261,9 @@ func TestFailureOnFlush(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) - iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, util.SoftDelete) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ @@ -290,9 +290,9 @@ func TestFailureOnDeletion(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) - iMgr.DeleteIPSet(TestKVPodSet.PrefixName, false) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName, util.SoftDelete) iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) - iMgr.DeleteIPSet(TestCIDRSet.PrefixName, false) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName, util.SoftDelete) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ diff --git a/npm/pkg/dataplane/mocks/genericdataplane_generated.go b/npm/pkg/dataplane/mocks/genericdataplane_generated.go index 45bff6404d..7899453ba6 100644 --- a/npm/pkg/dataplane/mocks/genericdataplane_generated.go +++ b/npm/pkg/dataplane/mocks/genericdataplane_generated.go @@ -13,6 +13,7 @@ import ( dataplane "github.com/Azure/azure-container-networking/npm/pkg/dataplane" ipsets "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" policies "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" + util "github.com/Azure/azure-container-networking/npm/util" gomock "github.com/golang/mock/gomock" ) @@ -122,15 +123,15 @@ func (mr *MockGenericDataplaneMockRecorder) CreateIPSets(setMetadatas interface{ } // DeleteIPSet mocks base method. -func (m *MockGenericDataplane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, force bool) { +func (m *MockGenericDataplane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata, deleteOption util.DeleteOption) { m.ctrl.T.Helper() - m.ctrl.Call(m, "DeleteIPSet", setMetadata, force) + m.ctrl.Call(m, "DeleteIPSet", setMetadata, deleteOption) } // DeleteIPSet indicates an expected call of DeleteIPSet. -func (mr *MockGenericDataplaneMockRecorder) DeleteIPSet(setMetadata, force interface{}) *gomock.Call { +func (mr *MockGenericDataplaneMockRecorder) DeleteIPSet(setMetadata, deleteOption interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).DeleteIPSet), setMetadata, force) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).DeleteIPSet), setMetadata, deleteOption) } // GetAllIPSets mocks base method. diff --git a/npm/pkg/dataplane/types.go b/npm/pkg/dataplane/types.go index d7dac24b5d..3385487958 100644 --- a/npm/pkg/dataplane/types.go +++ b/npm/pkg/dataplane/types.go @@ -3,6 +3,7 @@ package dataplane import ( "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" + "github.com/Azure/azure-container-networking/npm/util" ) type GenericDataplane interface { @@ -11,7 +12,7 @@ type GenericDataplane interface { GetAllIPSets() []string GetIPSet(setName string) *ipsets.IPSet CreateIPSets(setMetadatas []*ipsets.IPSetMetadata) - DeleteIPSet(setMetadata *ipsets.IPSetMetadata, force bool) + DeleteIPSet(setMetadata *ipsets.IPSetMetadata, deleteOption util.DeleteOption) AddToSets(setMetadatas []*ipsets.IPSetMetadata, podMetadata *PodMetadata) error RemoveFromSets(setMetadatas []*ipsets.IPSetMetadata, podMetadata *PodMetadata) error AddToLists(listMetadatas []*ipsets.IPSetMetadata, setMetadatas []*ipsets.IPSetMetadata) error diff --git a/npm/util/util.go b/npm/util/util.go index a1898e29ce..ac60c7a398 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -18,6 +18,15 @@ import ( "k8s.io/client-go/tools/cache" ) +// DeleteOption is used to decide if a delete is force delete or soft delete +type DeleteOption bool + +const ( + // For DeleteIPSet + ForceDelete DeleteOption = true + SoftDelete DeleteOption = false +) + // IsNewNwPolicyVerFlag indicates if the current kubernetes version is newer than 1.11 or not var IsNewNwPolicyVerFlag = false diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index 6e95deceff..8aa3e77dc5 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-container-networking/npm/pkg/dataplane" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" + "github.com/Azure/azure-container-networking/npm/util" ) const ( @@ -119,7 +120,7 @@ func main() { NodeName: "", } panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataD)) - dp.DeleteIPSet(ipsets.TestKVPodSet.Metadata, false) + dp.DeleteIPSet(ipsets.TestKVPodSet.Metadata, util.SoftDelete) panicOnError(dp.ApplyDataPlane()) if includeLists { @@ -129,7 +130,7 @@ func main() { printAndWait(true) panicOnError(dp.RemoveFromSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadata)) - dp.DeleteIPSet(ipsets.TestNSSet.Metadata, false) + dp.DeleteIPSet(ipsets.TestNSSet.Metadata, util.SoftDelete) panicOnError(dp.ApplyDataPlane()) printAndWait(true)