Skip to content

Conversation

@vakalapa
Copy link
Contributor

@vakalapa vakalapa commented Oct 4, 2021

This PR includes following changes:

  1. When a network policy is applied or removed in dataplane all ipsets mentioned in the network policy will be updated with relevant references
  2. When a new pod gets added to windows NPM, all the relevant calculations will be done to check if any existing network policies are to be applied or removed on its endpoint
  3. Improved UTs for ipset Mgr and dp

@vakalapa vakalapa added enhancement npm Related to NPM. labels Oct 5, 2021
Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updatePod() can be more efficient using the algorithm I designed, and I think we might have some disconnect with AddIPSetReferences()

@vakalapa vakalapa marked this pull request as ready for review October 6, 2021 23:36
err := dp.ipsetMgr.AddReference(set.Name, netpolName, referenceType)
if err != nil {
return err
npmErrorString := npmerrors.AddSelectorReference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a super strong preference, but can we consolidate these four lines into a function since it happens 4+ times?

return nil
}

func (dp *DataPlane) addIPSetReferences(sets map[string]*ipsets.IPSet, netpolName string, referenceType ipsets.ReferenceType) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename this and deleteIPSetReferences? Maybe createIPSetsAndAddReference? Name is confusing with ipsetMgr.AddReference

Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can address minor comments above in another PR

@vakalapa vakalapa changed the title [NPM] generic Policy Manager and some windows specific policy bits [NPM] generic Policy Manager and some windows specific policy updates Oct 7, 2021
@vakalapa vakalapa merged commit 779e965 into master Oct 7, 2021
@vakalapa vakalapa deleted the vakr/ipsetswindows branch October 7, 2021 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement npm Related to NPM.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants