Add network policy enforcement latency measurement #2234

dlapcevic · 2023-02-15T14:11:39Z

Description

A new measurement that utilizes network policy enforcement latency test clients.
https://github.com/kubernetes/perf-tests/tree/master/network/tools/network-policy-enforcement-latency

This measurement is intended to be used in the ClusterLoader2 load test.
https://github.com/kubernetes/perf-tests/tree/master/clusterloader2/testing/load

Network policy enforcement latency is the required time for any change to pods or network policies to be reflected in the system. It's measured by simulating a real scenario with workloads that are connecting to each other, and measuring the time it takes for the network policies to be honored for new pods or network policy changes. The relevant scalability and performance dimensions are pod and policy churn rate (create, update, delete).

Usage

The measurement tests network policy enforcement latency for two cases:

Created network policies
Deploy the test clients (setup and run) with "testType" flag set to "policy-creation" after creating the target pods.
Created pods that are affected by network policies
Deploy the test clients (setup and run) with "testType" flag set to "pod-creation”, before creating the target pods. Target pods are all pods that have the specified label: { targetLabelKey: targetLabelValue }.

The test is set up by creating the required resources, including the network policy enforcement latency test client pods that measure the latencies and generate metrics for them.

/kind feature

dlapcevic · 2023-02-15T14:12:14Z

/assign @marseel

aojea · 2023-02-15T14:51:19Z

/cc

...loader2/pkg/measurement/common/network-policy/manifests/dep-test-client-policy-creation.yaml

...terloader2/pkg/measurement/common/network-policy/manifests/dep-test-client-pod-creation.yaml

...loader2/pkg/measurement/common/network-policy/manifests/dep-test-client-policy-creation.yaml

clusterloader2/pkg/measurement/common/network-policy/network-policy-enforcement-latency.go

marseel · 2023-04-25T11:35:35Z

/lgtm

As it's a non-trivial measurement I would like someone else to take a look too.

...erloader2/pkg/measurement/common/network-policy/manifests/policy-egress-allow-apiserver.yaml

marseel · 2023-04-25T11:37:33Z

/hold

dlapcevic · 2023-05-10T11:59:34Z

Hi @marseel and @aojea, could you please recommend another reviewer?

tosi3k

Nitpicks, apart from that LGTM.

tosi3k · 2023-05-22T09:51:42Z

clusterloader2/pkg/measurement/common/network-policy/network-policy-enforcement-latency.go

+	limiter := rate.NewLimiter(rate.Limit(policyLoadQPS), policyLoadQPS)
+
+	for nsIdx, ns := range nps.targetNamespaces {
+		baseCidr := fmt.Sprintf("10.0.%d.0/24", nsIdx)


The measurement is not suitable for more than 256 test namespaces.

Could we at least have some way of indicating this is not allowed by explicitly failing in Execute when the user attempts to do so?

Added octet := nsIdx % 256 to make it work for more than 256 namespaces, because the CIDR is not very important here, it's just testing the network policy load.

tosi3k · 2023-05-22T10:06:36Z

...loader2/pkg/measurement/common/network-policy/manifests/policy-egress-allow-target-pods.yaml

+        matchLabels:
+          kubernetes.io/metadata.name: {{.TargetNamespace}}
+{{else}}
+      namespaceSelector: {}


Nit: is this actually needed?

Yes, because we are allowing ingress traffic from all test namespaces and not just from the one that the pods are in and policies applied.
More info: https://github.com/ahmetb/kubernetes-network-policy-recipes/blob/master/05-allow-traffic-from-all-namespaces.md

Selects all pods in all namespaces (namespaceSelector: {}). By default, if you omit specifying a namespaceSelector it does not select any namespaces, which means it will allow traffic only from the namespace the NetworkPolicy is deployed to.

tosi3k · 2023-05-22T10:12:48Z

clusterloader2/pkg/measurement/common/network-policy/manifests/clusterrole.yaml

+  verbs: ["get", "list", "watch"]
+- apiGroups: ["networking.k8s.io"]
+  resources: ["networkpolicies"]
+  verbs: ["get", "list", "watch"]


Nitpick: "watch" for networkpolicies isn't required, is it?

Apparently neither "list" is being used. I'm removing both "list" and "watch" for network policies.
They are being used by the test client app https://github.com/kubernetes/perf-tests/tree/master/network/tools/network-policy-enforcement-latency

tosi3k · 2023-05-22T10:17:36Z

clusterloader2/pkg/measurement/common/network-policy/network-policy-enforcement-latency.go

+}
+
+func (nps *networkPolicyEnforcementMeasurement) run(config *measurement.Config) error {
+	// Should this be mandatory? Fail if not provided.


Should this get a TODO or is it an unfinished leftover?

tosi3k · 2023-05-22T10:23:14Z

clusterloader2/pkg/measurement/common/network-policy/network-policy-enforcement-latency.go

+	return nil
+}
+
+func (nps *networkPolicyEnforcementMeasurement) run(config *measurement.Config) error {


Could we add some check to run that setup was run beforehand?

Done. Used nps.framework == nil to check if setup was run before.

dlapcevic · 2023-06-06T12:16:48Z

@tosi3k @marseel can we please merge now?

marseel · 2023-06-06T12:48:12Z

/lgtm
/approve

tosi3k · 2023-06-07T09:34:53Z

/lgtm

k8s-ci-robot · 2023-06-07T09:35:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dlapcevic, marseel, tosi3k

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~clusterloader2/OWNERS~~ [marseel,tosi3k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dlapcevic · 2023-06-09T11:37:06Z

@marseel can we please merge now?

tosi3k · 2023-06-09T11:37:58Z

I think you just need to cancel the hold :).

/hold cancel

dlapcevic · 2023-06-09T11:39:44Z

Thanks @tosi3k. I didn't know that I had a permission to do it.

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 15, 2023

k8s-ci-robot requested review from mborsz and wojtek-t February 15, 2023 14:11

k8s-ci-robot assigned marseel Feb 15, 2023

dlapcevic force-pushed the netpol-enforcement-latency-test branch from 0b8d7a0 to 3671f0c Compare February 15, 2023 14:47

k8s-ci-robot requested a review from aojea February 15, 2023 14:51

dlapcevic force-pushed the netpol-enforcement-latency-test branch from 3671f0c to 7a51f1e Compare February 15, 2023 15:22

marseel reviewed Feb 16, 2023

View reviewed changes

...loader2/pkg/measurement/common/network-policy/manifests/dep-test-client-policy-creation.yaml Show resolved Hide resolved

...terloader2/pkg/measurement/common/network-policy/manifests/dep-test-client-pod-creation.yaml Show resolved Hide resolved

marseel reviewed Feb 24, 2023

View reviewed changes

dlapcevic force-pushed the netpol-enforcement-latency-test branch 5 times, most recently from 7d2dc96 to c9000be Compare March 31, 2023 08:33

dlapcevic requested a review from marseel April 3, 2023 08:41

dlapcevic mentioned this pull request Apr 14, 2023

Add network policy enforcement latency measurement to load test #2251

Merged

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2023

marseel reviewed Apr 25, 2023

View reviewed changes

...erloader2/pkg/measurement/common/network-policy/manifests/policy-egress-allow-apiserver.yaml Show resolved Hide resolved

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 25, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2023

tosi3k suggested changes May 22, 2023

View reviewed changes

dlapcevic force-pushed the netpol-enforcement-latency-test branch from c9000be to 1ca5029 Compare June 6, 2023 12:15

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2023

dlapcevic force-pushed the netpol-enforcement-latency-test branch from 1ca5029 to 02a316d Compare June 6, 2023 12:37

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2023

Add network policy enforcement latency measurement

c4cb115

dlapcevic force-pushed the netpol-enforcement-latency-test branch from 02a316d to c4cb115 Compare June 7, 2023 09:26

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2023

k8s-ci-robot assigned tosi3k Jun 7, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 9, 2023

k8s-ci-robot merged commit 9025300 into kubernetes:master Jun 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add network policy enforcement latency measurement #2234

Add network policy enforcement latency measurement #2234

dlapcevic commented Feb 15, 2023

dlapcevic commented Feb 15, 2023

aojea commented Feb 15, 2023

marseel commented Apr 25, 2023

marseel commented Apr 25, 2023

dlapcevic commented May 10, 2023

tosi3k left a comment

tosi3k May 22, 2023

dlapcevic Jun 6, 2023

tosi3k May 22, 2023

dlapcevic Jun 1, 2023 •

edited

Loading

tosi3k May 22, 2023

dlapcevic Jun 6, 2023

tosi3k May 22, 2023

dlapcevic Jun 6, 2023

tosi3k May 22, 2023

dlapcevic Jun 6, 2023

dlapcevic commented Jun 6, 2023

marseel commented Jun 6, 2023

tosi3k commented Jun 7, 2023

k8s-ci-robot commented Jun 7, 2023

dlapcevic commented Jun 9, 2023

tosi3k commented Jun 9, 2023

dlapcevic commented Jun 9, 2023

Add network policy enforcement latency measurement #2234

Add network policy enforcement latency measurement #2234

Conversation

dlapcevic commented Feb 15, 2023

dlapcevic commented Feb 15, 2023

aojea commented Feb 15, 2023

marseel commented Apr 25, 2023

marseel commented Apr 25, 2023

dlapcevic commented May 10, 2023

tosi3k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlapcevic Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlapcevic commented Jun 6, 2023

marseel commented Jun 6, 2023

tosi3k commented Jun 7, 2023

k8s-ci-robot commented Jun 7, 2023

dlapcevic commented Jun 9, 2023

tosi3k commented Jun 9, 2023

dlapcevic commented Jun 9, 2023

dlapcevic Jun 1, 2023 •

edited

Loading