CNP Tier integration #956

abhiraut · 2020-07-15T02:43:41Z

Add support for Tiered ClusterNetworkPolicies by associating a CNP with Tier name. This PR adds the following:

Add a new field tier to CNP and native NP specs
Add "Emergency, SecurityOps, NetworkOps, Platform, Application" as choices for tier names
Add Tier column to the CNP kubectl get cnp output
Update internal NetworkPolicy types to include the TierPriority associated with above tier names
Add unit tests

A CNP without any association to any tier will be created in the default lowest priority tier i.e. "Application Tier". The same
applies for all existing CNP created prior to the Tier introduction.
The tiers have the following precedence:
Emergency > SecurityOps > NetworkOps > Platform > Application
i.e. all policies associated with Emergency Tiers will be evaluated before any policy associated with SecurityOps tier and so on.
The K8s NetworkPolicies will be evaluated once all Tiers are evaluated i.e. after the Application Tier.

Related-Issue: #917

antrea-bot · 2020-07-15T02:43:53Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

Dyanngg

LGTM, minor nit

pkg/controller/networkpolicy/clusternetworkpolicy_test.go

pkg/apis/networking/types.go

Signed-off-by: abhiraut <rauta@vmware.com>

Dyanngg

LGTM

antrea-bot · 2020-07-29T11:26:17Z

Can one of the admins verify this patch?

Dyanngg

--- a/pkg/controller/networkpolicy/networkpolicy_controller.go
+++ b/pkg/controller/networkpolicy/networkpolicy_controller.go
@@ -1324,6 +1324,7 @@ func (n *NetworkPolicyController) syncInternalNetworkPolicy(key string) error {
                Rules:           internalNP.Rules,
                AppliedToGroups: internalNP.AppliedToGroups,
                Priority:        internalNP.Priority,
+               TierPriority:    internalNP.TierPriority,

Without this line, the Tier info for the Internal NetworkPolicy would be lost each time a span update happens for example.

abhiraut · 2020-07-29T22:26:09Z

/test-all

abhiraut · 2020-07-30T18:07:52Z

/test-windows-networkpolicy

Dyanngg

lgtm, this PR is also used for integration testing in #986 and result is good.

antrea-bot · 2020-08-03T23:59:58Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

abhiraut · 2020-08-04T00:10:48Z

/test-all

build/yamls/base/crds.yml

antoninbas · 2020-08-04T00:34:32Z

pkg/apis/security/v1alpha1/types.go

+	// Tier's Priority and the NetworkPolicy's own Priority. If not specified,
+	// this policy will be created in the Application Tier right above the K8s
+	// NetworkPolicy which resides at the bottom.
+	Tier string `json:"tier,omitempty"`


I am curious about why we do not do something like this in this file:

type Tier string const ( TierEmergency Tier = "Emergency" TierSecurityOps Tier = "SecurityOps" # ... )

Since my understanding based on the openAPI schema for the CRDs is that we constrain the tiers to a predefined set.
This seems to be pretty common in K8s APIs: https://godoc.org/k8s.io/api/core/v1#RestartPolicy

i left it as string because idea is to replace the static tiers to CRDs where the name of the tier may not necessarily be part of the enum..

abhiraut · 2020-08-04T19:58:59Z

/test-all

antoninbas

LGTM

tnqn

LGTM, two minor comments.

pkg/controller/networkpolicy/clusternetworkpolicy_test.go

tnqn · 2020-08-05T15:04:32Z

pkg/controller/networkpolicy/store/networkpolicy.go

+	if in.TierPriority != nil {
+		inTierPriority := uint32(*in.TierPriority)
+		out.TierPriority = &inTierPriority
+	}


I think we assume objects in store will always be overridden when it's updated, so it should be safe to just copy the pointer like other fields. AppliedToGroups is a slice, Priority is *float64, we don't make a copy too.

updated.. needed to do conversion from type TierPriority to uint32 .. moved the TierPriority type to networking types.. so this conversion is no longer needed. PTAL

abhiraut · 2020-08-05T21:03:59Z

/test-all

antoninbas

Approving again, it seems Quan's comments have been addressed

abhiraut · 2020-08-06T04:49:53Z

/test-windows-conformance

tnqn

LGTM

Add support for Tiered ClusterNetworkPolicies by associating a CNP with Tier name. This PR adds the following: Add a new field tier to CNP and native NP specs Add "Emergency, SecurityOps, NetworkOps, Platform, Application" as choices for tier names Add Tier column to the CNP kubectl get cnp output Update internal NetworkPolicy types to include the TierPriority associated with above tier names A CNP without any association to any tier will be created in the default lowest priority tier i.e. "Application Tier". The same applies for all existing CNP created prior to the Tier introduction. The tiers have the following precedence: Emergency > SecurityOps > NetworkOps > Platform > Application i.e. all policies associated with Emergency Tiers will be evaluated before any policy associated with SecurityOps tier and so on. The K8s NetworkPolicies will be evaluated once all Tiers are evaluated i.e. after the Application Tier.

vmwclabot added the cla-not-required label Jul 15, 2020

abhiraut added 2 commits July 17, 2020 14:39

Add TierPriority to internal NetworkPolicy types

a6e2c58

Add TierPriority with codegen

ddbd746

abhiraut force-pushed the cnp-tier-ctrl branch 3 times, most recently from 3e74f76 to aaf87ff Compare July 21, 2020 00:10

Remove Tier CRD references

1781955

abhiraut force-pushed the cnp-tier-ctrl branch from aaf87ff to 1781955 Compare July 21, 2020 00:32

abhiraut changed the title ~~WIP: Cnp tier integration~~ CNP Tier integration Jul 21, 2020

Add unit tests and comments

935aeb6

abhiraut requested review from Dyanngg, tnqn and jianjuns July 21, 2020 22:04

Dyanngg previously approved these changes Jul 21, 2020

View reviewed changes

pkg/controller/networkpolicy/clusternetworkpolicy_test.go Show resolved Hide resolved

Dyanngg reviewed Jul 24, 2020

View reviewed changes

pkg/apis/networking/types.go Outdated Show resolved Hide resolved

abhiraut dismissed Dyanngg’s stale review via b255784 July 24, 2020 19:08

Update comment about TierPriority in internal types

bc98bd5

Signed-off-by: abhiraut <rauta@vmware.com>

abhiraut force-pushed the cnp-tier-ctrl branch from b255784 to bc98bd5 Compare July 24, 2020 19:25

abhiraut requested a review from Dyanngg July 24, 2020 19:25

abhiraut added this to the Antrea v0.9.0 release milestone Jul 24, 2020

Dyanngg mentioned this pull request Jul 27, 2020

[CNP] Agent CNP Tier Support #986

Merged

Dyanngg approved these changes Jul 29, 2020

View reviewed changes

Dyanngg requested changes Jul 29, 2020

View reviewed changes

Insert TierPriority on sync of internal NetworkPolicy

ea03f72

abhiraut requested a review from Dyanngg July 29, 2020 19:29

Dyanngg previously approved these changes Jul 30, 2020

View reviewed changes

Rename InterTenant to Platform tier

aae7976

abhiraut dismissed Dyanngg’s stale review via aae7976 August 3, 2020 23:59

jianjuns previously approved these changes Aug 4, 2020

View reviewed changes

antoninbas reviewed Aug 4, 2020

View reviewed changes

Use enum for cnp tier type string

41de108

abhiraut dismissed jianjuns’s stale review via 41de108 August 4, 2020 19:48

abhiraut requested review from antoninbas, jianjuns and Dyanngg August 4, 2020 20:18

Dyanngg previously approved these changes Aug 4, 2020

View reviewed changes

antoninbas previously approved these changes Aug 5, 2020

View reviewed changes

tnqn reviewed Aug 5, 2020

View reviewed changes

Update TierPriority types

ce0ef2e

abhiraut dismissed stale reviews from antoninbas and Dyanngg via ce0ef2e August 5, 2020 19:13

Fix imports

a02b12b

abhiraut requested review from Dyanngg, antoninbas and tnqn August 5, 2020 21:37

antoninbas approved these changes Aug 5, 2020

View reviewed changes

Dyanngg approved these changes Aug 5, 2020

View reviewed changes

tnqn approved these changes Aug 6, 2020

View reviewed changes

abhiraut merged commit 1ff54b5 into antrea-io:master Aug 6, 2020

abhiraut deleted the cnp-tier-ctrl branch August 6, 2020 06:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CNP Tier integration #956

CNP Tier integration #956

abhiraut commented Jul 15, 2020 •

edited

Loading

antrea-bot commented Jul 15, 2020

Dyanngg left a comment

Dyanngg left a comment

antrea-bot commented Jul 29, 2020

Dyanngg left a comment

abhiraut commented Jul 29, 2020

abhiraut commented Jul 30, 2020

Dyanngg left a comment

antrea-bot commented Aug 3, 2020

abhiraut commented Aug 4, 2020

antoninbas Aug 4, 2020

abhiraut Aug 4, 2020

abhiraut commented Aug 4, 2020

antoninbas left a comment

tnqn left a comment

tnqn Aug 5, 2020

abhiraut Aug 5, 2020

abhiraut commented Aug 5, 2020

antoninbas left a comment

abhiraut commented Aug 6, 2020

tnqn left a comment

CNP Tier integration #956

CNP Tier integration #956

Conversation

abhiraut commented Jul 15, 2020 • edited Loading

antrea-bot commented Jul 15, 2020

Dyanngg left a comment

Choose a reason for hiding this comment

Dyanngg left a comment

Choose a reason for hiding this comment

antrea-bot commented Jul 29, 2020

Dyanngg left a comment

Choose a reason for hiding this comment

abhiraut commented Jul 29, 2020

abhiraut commented Jul 30, 2020

Dyanngg left a comment

Choose a reason for hiding this comment

antrea-bot commented Aug 3, 2020

abhiraut commented Aug 4, 2020

antoninbas Aug 4, 2020

Choose a reason for hiding this comment

abhiraut Aug 4, 2020

Choose a reason for hiding this comment

abhiraut commented Aug 4, 2020

antoninbas left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn Aug 5, 2020

Choose a reason for hiding this comment

abhiraut Aug 5, 2020

Choose a reason for hiding this comment

abhiraut commented Aug 5, 2020

antoninbas left a comment

Choose a reason for hiding this comment

abhiraut commented Aug 6, 2020

tnqn left a comment

Choose a reason for hiding this comment

abhiraut commented Jul 15, 2020 •

edited

Loading