Extended load - StatefulSets and PVs. #716

mm4tt · 2019-07-31T09:48:07Z

As explained in one TODO, PVs are created by StatefulSets but are not deleted until namespaces are deleted (so after test, after all measurements are stopped). This is something we should change.

k8s-ci-robot · 2019-07-31T09:48:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mm4tt

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~~ [mm4tt]

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

mm4tt · 2019-07-31T09:58:01Z

/test pull-perf-tests-clusterloader2

mm4tt · 2019-07-31T09:59:22Z

/test pull-perf-tests-clusterloader2-kubemark

wojtek-t · 2019-08-22T11:30:04Z

clusterloader2/testing/load/experimental/extended_config.yaml

+
+# StatefulSets
+{{$STATEFUL_SET_SIZE := 5}}
+{{$STATEFUL_SETS_PER_NAMESPACE := 2}}


Apparently there is a feature for parallel-pod-management in statefulset:
https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#parallel-pod-management

So I think we should use it and have one of each size per namespace.

wojtek-t · 2019-08-22T12:02:24Z

clusterloader2/testing/load/experimental/statefulset.yaml

+  # measure api call latency for that. There are two options here:
+  # 1. Create a new measurement that will delete PVs created by StatefulSets
+  # 2. Create PVs manually, attach them here (but not via volumeClaimTemplates) and delete them
+  #    manually later


I actually think that there is a third option which is faster and simpler to do (and I would vote for proceeding this path at least for now).
PVCs connected to StatefulSet have naming schema: "-statefulset-name>-".

So we can actually create a template file for PVC and we will simply add a step to delete PVCs that would look like:

namespaceRange: min: 1 max: {{$namespaces}} replicasPerNamespace: 0 tuningSet: RandomizedSaturationTimeLimited objectBundle: - basename: pvname-ssname objectTemplatePath: pvc.yaml

If at some point we decide for more pvc-per-pod or more statefulset-per-namespace, we can opaque it in range (ugly but works - I don't we need it immediately).

The only remaining problem we have is that we enumrate from 1 to N, and StatefulSet from 0 to N-1.
But I think we can change that universally for CL, to actually use 0..N-1 scheme (I think that's reasonable on itself).

Done, but from what I managed to get from code it looks like we're enumerating from 0 in CL2 already. Please correct me if I'm wrong

I think it used to be 1..N, but apparently it's not:
https://github.com/kubernetes/perf-tests/blob/master/clusterloader2/pkg/test/simple_test_executor.go#L199

As discussed f2f, unfortunately this wouldn't work, as CL2 is not able to delete objects that it didn't create. Long story short it keeps it own state of all objects that were created/modified. When we try to delete the PVCs, CL2 doesn't know about them and assumes that none of them exist and doesn't do anything when deleting.

Leaving the TODO as it was, will tackle it in a separate PR.

wojtek-t · 2019-09-03T13:10:53Z

clusterloader2/testing/load/experimental/extended_config.yaml

+      action: start
+      apiVersion: apps/v1
+      kind: StatefulSet
+      labelSelector: group = statefulset


This one is pretty much only selecting controlling objects (so statefulsets).

So for consistency with deployments (and probably other in the near future), I vote for making that:

group = load

This would mean that they will also be measured as part of pod-startup-time and all stuff that was assuming that all pods actually have group = load label.

WDYT?

Sounds good, done.

mm4tt

/hold

Let's wait for kubernetes/test-infra#14174, #772 and #767 to get. One is required to properly roll-out this experiment, two other are baselines that will make reviewing this easier.

mm4tt · 2019-09-03T11:32:25Z

clusterloader2/testing/load/experimental/extended_config.yaml

+
+# StatefulSets
+{{$STATEFUL_SET_SIZE := 5}}
+{{$STATEFUL_SETS_PER_NAMESPACE := 2}}


mm4tt · 2019-09-03T13:23:09Z

clusterloader2/testing/load/experimental/extended_config.yaml

+      action: start
+      apiVersion: apps/v1
+      kind: StatefulSet
+      labelSelector: group = statefulset


Sounds good, done.

wojtek-t · 2019-09-03T14:43:32Z

clusterloader2/testing/load/config.yaml

@@ -8,23 +8,36 @@
 {{$NODE_MODE := DefaultParam .NODE_MODE "allnodes"}}
 {{$NODES_PER_NAMESPACE := DefaultParam .NODES_PER_NAMESPACE 100}}
 {{$PODS_PER_NODE := DefaultParam .PODS_PER_NODE 30}}
-{{$LOAD_TEST_THROUGHPUT := DefaultParam .LOAD_TEST_THROUGHPUT 10}}
+{{$LOAD_TEST_THROUGHPUT := DefaultParam .LOAD_TEST_THROUGHPUT 100}}


wojtek-t · 2019-09-03T14:46:44Z

clusterloader2/testing/load/config.yaml

 {{$BIG_GROUP_SIZE := 250}}
 {{$MEDIUM_GROUP_SIZE := 30}}
 {{$SMALL_GROUP_SIZE := 5}}
+{{$STATEFUL_SET_SIZE := 10}}


Apparently one of my comments disappeared in the meantime.
What I was suggesting that with the ability for parallel-pod-management, we may want to create 1 of each: small, medium and large StatefulSet per namespace instead of a corresponding deployment.

This would also simplify the logic below (i.e. you would leave everything as was before and do:

if ENABLE_STATEFUL_SETS { if small-deployments > 0 { small-deployments-- small-statefulsets++ } if medium-deployments > 0 { ... ...

I'm also fine with (as a first step) getting rid of stateful-sets-per-namespace and explicitly assuming 1 of each.

wojtek-t · 2019-09-03T14:46:51Z

clusterloader2/testing/load/config.yaml


+# TODO: Remove


Ref. kubernetes#704

k8s-ci-robot · 2019-09-04T10:29:39Z

@mm4tt: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-perf-tests-clusterloader2	`159b650`	link	`/test pull-perf-tests-clusterloader2`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

wojtek-t · 2019-09-04T10:54:07Z

clusterloader2/testing/load/statefulset.yaml

+            cpu: 10m
+            memory: "10M"
+  {{if $EnablePVs}}
+  # TODO(#704): We should have better control over deleting PVs.


As I mentioned offline - let's maybe first do only StatefulSets and we will extend with PVs in a separate step.

mm4tt · 2019-09-04T11:33:28Z

Closing this one, will add StatefulSets separately in #776 and then tackle PVs in a separate PR.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 31, 2019

mm4tt closed this Jul 31, 2019

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 31, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2019

mm4tt reopened this Jul 31, 2019

k8s-ci-robot requested review from mborsz and oxddr July 31, 2019 09:48

mm4tt force-pushed the extended_load_ss_pv branch from 231e460 to 4ef02b5 Compare July 31, 2019 09:48

mm4tt mentioned this pull request Jul 31, 2019

Scalability load test extended to exercise Deployments, DaemonSets, StatefulSets, Jobs, PersistentVolumes, Secrets, ConfigMaps, NetworkPolicies #704

Open

16 tasks

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2019

wojtek-t reviewed Aug 22, 2019

View reviewed changes

wojtek-t reviewed Sep 3, 2019

View reviewed changes

mm4tt force-pushed the extended_load_ss_pv branch from 4ef02b5 to b56c389 Compare September 3, 2019 14:27

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 3, 2019

mm4tt commented Sep 3, 2019

View reviewed changes

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

wojtek-t reviewed Sep 3, 2019

View reviewed changes

mm4tt force-pushed the extended_load_ss_pv branch from b56c389 to 0de2ffe Compare September 4, 2019 09:58

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 4, 2019

Extended load - StatefulSets and PVs.

159b650

Ref. kubernetes#704

mm4tt force-pushed the extended_load_ss_pv branch from 0de2ffe to 159b650 Compare September 4, 2019 10:01

wojtek-t reviewed Sep 4, 2019

View reviewed changes

mm4tt closed this Sep 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extended load - StatefulSets and PVs. #716

Extended load - StatefulSets and PVs. #716

mm4tt commented Jul 31, 2019

k8s-ci-robot commented Jul 31, 2019

mm4tt commented Jul 31, 2019

mm4tt commented Jul 31, 2019

wojtek-t Aug 22, 2019

mm4tt Sep 3, 2019

wojtek-t Aug 22, 2019

mm4tt Sep 3, 2019

wojtek-t Sep 3, 2019

mm4tt Sep 4, 2019

wojtek-t Sep 3, 2019

mm4tt Sep 3, 2019

mm4tt left a comment

mm4tt Sep 3, 2019

mm4tt Sep 3, 2019

wojtek-t Sep 3, 2019

wojtek-t Sep 3, 2019

wojtek-t Sep 3, 2019

wojtek-t Sep 3, 2019

k8s-ci-robot commented Sep 4, 2019

wojtek-t Sep 4, 2019

mm4tt commented Sep 4, 2019

Extended load - StatefulSets and PVs. #716

Extended load - StatefulSets and PVs. #716

Conversation

mm4tt commented Jul 31, 2019

k8s-ci-robot commented Jul 31, 2019

mm4tt commented Jul 31, 2019

mm4tt commented Jul 31, 2019

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

Choose a reason for hiding this comment

mm4tt 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 4, 2019

Choose a reason for hiding this comment

mm4tt commented Sep 4, 2019