Skip to content

Conversation

@huntergregory
Copy link
Contributor

@huntergregory huntergregory commented Nov 22, 2021

Reason for Change:
After running conformance test, fix finding issues.

  1. Had a bug where we were jumping to AZURE-NPM-EGRESS chain instead. We need to jump to this other chain since the ingress allow mark will prevent us from returning to ingress chain again after a no-op pass through egress chain.

  2. Observed multiple restarting NPM. Checked log and there is nil pointer errors in NPMSimpleError.

  • For the second error, we need to UT to catch this nil error.
  • Need to resolve lint warning.
  1. Fix case-sensitivity on protocol value
  • need to remove dependency later
  • need better management with unspecified and Any
  1. Fix problem of trying to destroy sets in use by a kernel component
  • previously when running ipset restore, we would try to destroy sets before deleting any references to them in lists
  • fix by destroying sets last in the ipset restore file
  1. Fix duplicated namedPort prefix.

  2. Fix translating egress (previously included only the default drop rule)

  • Add UTs for egress
  1. Fix string parsing issue in except information of ipblock
  • Need more UTs (in dataplane_test.go)
  • Better data structures to indicate nomatch information in ipblock
  • Need to resolve lint warning.
  1. Fix using correct, unique and consistent key in policy cache. (now deleting network policy works)
  • Need more UTs
  • Do not walk through windows part. Please revise windows if needed.
  1. Fix Linux policy manager to use PodSelectorList instead of PodSelectorIPSets (this caused an impossible ipset match condition for nested pod labels e.g. pod-a-b AND pod:a AND pod:b).
  • Will update the NPMNetworkPolicy struct to just have one of these fields eventually instead of the two.

  • uses conventional commit messages

  • includes documentation

  • adds unit tests

Notes:

@huntergregory huntergregory added the npm Related to NPM. label Nov 22, 2021
vakalapa
vakalapa previously approved these changes Nov 22, 2021
JungukCho
JungukCho previously approved these changes Nov 22, 2021
@huntergregory huntergregory dismissed stale reviews from JungukCho and vakalapa via 4514a1e November 22, 2021 20:58
@JungukCho JungukCho changed the title fix: [NPM] on ingress allow rules, jump to AZURE-NPM-INGRESS-ALLOW-MARK chain fix: [NPM] V2 NPM (with only linux data plane) bug fix after first run Nov 23, 2021
Copy link
Contributor

@vakalapa vakalapa left a comment

Choose a reason for hiding this comment

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

LGTM, great effort finding all the issues!!!!

@huntergregory huntergregory merged commit 92b89c7 into master Dec 1, 2021
@rbtr rbtr deleted the fix-ingress-jumps branch December 4, 2021 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

npm Related to NPM.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants