Skip to content

Added network policy test setup for soak, scale and load and supporting tools #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sanamsarath
Copy link

What type of PR is this?

/kind feature
/kind regression

What this PR does / why we need it:

The changes in the pull request include:

  • new measurement "NetworkPolicySoakMeasurement" to run soak, scale and load tests for network policies. (Please refer perf-tests/clusterloader2/testing/network-policy/network-policy-soak/Readme.txt for the test setup design, and steps to run)
  • new load gen tools to support the network policy tests (generate traffic matching L7 policies)
  • enhancements to existing network-policy-enforcement measurement to support cilium network policies along with default k8s network policies.

Copilot generated summery below:

This pull request introduces several significant enhancements and modifications to the network policy enforcement and soak tests in the clusterloader2 project. The changes include adding support for Cilium network policies, updating image references, and improving the flexibility of the network policy framework.

Enhancements to Network Policy Enforcement:

Updates to Network Policy Manifests:

Changes to Image References:

Additions to Network Policy Soak Tests:

  • Added new manifests for network policy soak tests, including network policies, deployments, cluster roles, and role bindings. These changes introduce support for Cilium network policies and improve the soak test framework.
    • clusterloader2/pkg/measurement/common/network-policy/network-policy-soak/manifests/allow_apiserver_np.yaml
    • clusterloader2/pkg/measurement/common/network-policy/network-policy-soak/manifests/client_deploy.yaml
    • clusterloader2/pkg/measurement/common/network-policy/network-policy-soak/manifests/clusterrole.yaml
    • clusterloader2/pkg/measurement/common/network-policy/network-policy-soak/manifests/clusterrolebinding.yaml
    • clusterloader2/pkg/measurement/common/network-policy/network-policy-soak/manifests/network_policy.yaml
    • clusterloader2/pkg/measurement/common/network-policy/network-policy-soak/manifests/serviceaccount.yaml
    • clusterloader2/pkg/measurement/common/network-policy/network-policy-soak/manifests/target_deploy.yaml

Minor Import and Function Updates:

@sanamsarath sanamsarath marked this pull request as ready for review February 26, 2025 20:16
@@ -34,7 +34,8 @@ spec:
name: npdelaymetrics
protocol: TCP
imagePullPolicy: Always
image: gcr.io/k8s-staging-perf-tests/network-policy-enforcement-latency/policy-creation-enforcement-latency:v0.0.1
# image: gcr.io/k8s-staging-perf-tests/network-policy-enforcement-latency/policy-creation-enforcement-latency:v0.0.1
image: docker.io/sanamsarath/policy-creation-enforcement-latency:v0.0.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we using custom image here? If that's needed to add new commands, then we should publish it to ghcr and use image from there instead of docker.io

@@ -46,6 +47,8 @@ spec:
-MaxTargets={{.MaxTargets}}
-MetricsPort={{.MetricsPort}}
-AllowPolicyName={{.AllowPolicyName}}
-np_type={{.NetworkPolicy_Type}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's follow proper format like the original arguments. For example, -NetworkPolicyType instead of np_type. Also, remove _ for consistency in naming, like NetworkPolicyType instead of NetworkPolicy_Type

Choose a reason for hiding this comment

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

Does this repo have a style guide?

- method: GET
path: /
{{end}}
{{else if .NetworkPolicy_Type "ccnp" }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't quite follow the if else here. Can you use indentation those to make the condition flow clearer?

@@ -111,6 +112,12 @@ type networkPolicyEnforcementMeasurement struct {
// testClientNodeSelectorValue is value key for the node label on which the
// test client pods should run.
testClientNodeSelectorValue string
// np type is the type of network policy to be created, default is k8s.
npType string
Copy link
Collaborator

Choose a reason for hiding this comment

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

netPolicyType instead of npType. We should avoid unclear variable names

@@ -6,6 +6,7 @@ metadata:
app.kubernetes.io/name: prometheus-operator
app.kubernetes.io/part-of: kube-prometheus
app.kubernetes.io/version: 0.46.0
k8s-app: prometheus-operator
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need this? it's already defined with app.kubernetes.io/name: prometheus-operator

@@ -0,0 +1,18 @@
# test setup details -
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename file to README.md. Add documentation for different type of network policies, like what is k8s, ccnp, cnp.

@@ -0,0 +1,17 @@
USERNAME ?= sanamsarath
Copy link
Collaborator

Choose a reason for hiding this comment

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

let use some default name here instead of your name in case we want to merge this into upstream

@@ -0,0 +1,17 @@
USERNAME ?= sanamsarath
PROJECT ?= netloader
REGISTRY ?= acnpublic.azurecr.io
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here let's use ghcr

@@ -1,4 +1,4 @@
REGISTRY ?= gcr.io/k8s-staging-perf-tests/network-policy-enforcement-latency
REGISTRY ?= docker.io/sanamsarath
Copy link
Collaborator

Choose a reason for hiding this comment

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

revert

@@ -1,56 +1,113 @@
module k8s.io/perf-tests/network/tools/network-policy-enforcement-latency

go 1.22.4
go 1.23.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

changing go version could break other tests. Let's avoid doing that if not necessary

@sanamsarath sanamsarath marked this pull request as draft March 2, 2025 03:04
@sanamsarath sanamsarath marked this pull request as ready for review March 10, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants