Use deployment instead of raw pod for storage perf tests #687

mucahitkurt · 2019-07-24T21:38:54Z

Raw pod object templates are changed with deployment templates and the same pods definitions are used inside deployments. Deployment-Pod relation is 1-1.

Also test config files are changed according to these new deployment templates.

/sig storage
/assign @wojtek-t
cc @msau42

k8s-ci-robot · 2019-07-24T21:39:02Z

Hi @mucahitkurt. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

msau42 · 2019-07-24T21:50:00Z

/ok-to-test

msau42 · 2019-07-24T21:52:34Z

@wojtek-t are scale tests run on regional clusters? I'm thinking of when we want to reuse these for PVC tests too and each PVC is restricted to a single zone. To support multiple PVCs in a Pod case, we would need to either:

Use delayed volume binding to ensure all pvcs within a pod are provisioned in the same zone as the pod. This will add provisioning into pod startup latency though.
Use StatefulSet, which hashes all PVCs within a StatefulSet to provision from the same zone.

hantaowang · 2019-07-25T01:02:25Z

How does this work for config and secret where the name of the config/secret is different for every pod?

mucahitkurt · 2019-07-25T05:51:05Z

How does this work for config and secret where the name of the config/secret is different for every pod?

@hantaowang I use N deployments for N pods, so the relation is 1-1, and volume matching shouldn't be different for the raw pod case, I create N volumes and match these volumes to the N pods inside the N deployments.

wojtek-t · 2019-07-25T07:21:48Z

clusterloader2/testing/experimental/storage/pod-startup/ephemeral-volumes/1_node/config.yaml

@@ -3,7 +3,7 @@

 #Constants
 {{$TOTAL_PODS := DefaultParam .TOTAL_PODS 1}}
-{{$POD_TEMPLATE_PATH := DefaultParam .POD_TEMPLATE_PATH "../volume-types/emptydir/pod_with_emptydir.yaml"}}
+{{$DEPLOYMENT_TEMPLATE_PATH := DefaultParam .DEPLOYMENT_TEMPLATE_PATH "../volume-types/emptydir/deployment_with_emptydir.yaml"}}


I'm sorry - I was blocking @hantaowang PRs for quite some time and your stuff, so this time I will do it the other way (unfortunately you're both touching the same stuff).
#688 is super useful, so I would like that one to merge first and then I will ask you to rebase this one on top.

hantaowang · 2019-07-25T07:53:00Z

Doesn't this then also stress the deployment controller? We may inadvertently become bottle necked by the deployment and replicaset controller despite the fact that so many deployments are rare. Deployments are definitely a dimension to stress on, but imo for storage that should be eliminated as a variable.

wojtek-t · 2019-07-25T08:21:26Z

Doesn't this then also stress the deployment controller? We may inadvertently become bottle necked by the deployment and replicaset controller despite the fact that so many deployments are rare. Deployments are definitely a dimension to stress on, but imo for storage that should be eliminated as a variable.

Disagree - this number of deployments actually isn't very high (we're testing higher numbers). And we should be using what users are using not something artificial.

hantaowang · 2019-07-25T08:46:40Z

Is it that realistic to use 1 deployment per pod?

I'm thinking more for when the tests become larger scale (say 5000 nodes at 10 pods per node). I don't see anyone using 50000 deployments to manage this as that would definitely overload the controllers.

wojtek-t · 2019-07-25T09:01:49Z

Is it that realistic to use 1 deployment per pod?

I've seen a bunch of large user clusters with a huge number of those - yes it is realistic.

I'm thinking more for when the tests become larger scale (say 5000 nodes at 10 pods per node). I don't see anyone using 50000 deployments to manage this as that would definitely overload the controllers.

I wouldn't be that convinced that this will for sure overload controllers.
We will see when we get to this scale - I think we will hit many other limitations before that.

mucahitkurt · 2019-07-30T05:12:11Z

waiting for the PVC tests PR's merge to continue

wojtek-t · 2019-07-30T07:32:07Z

That PR has just merged - can you please rebase?

mucahitkurt · 2019-07-30T20:43:16Z

@wojtek-t, @hantaowang pod usages are converted to deployment usage

wojtek-t · 2019-07-31T07:03:54Z

...sting/experimental/storage/pod-startup/volume-types/configmap/deployment_with_configmap.yaml

+metadata:
+  name: {{.Name}}
+  labels:
+    app: {{.Name}}


What do we need "app" label for?

I think we don't need it

wojtek-t · 2019-07-31T07:04:52Z

...sting/experimental/storage/pod-startup/volume-types/configmap/deployment_with_configmap.yaml

+        configMap:
+          name: vol-{{AddInt $volumeIndex (MultiplyInt $index $volumesPerPod)}}
+      {{ end }}
+      dnsPolicy: Default


We don't need to set it - it will be defaulted anyway.

wojtek-t · 2019-07-31T07:05:09Z

...sting/experimental/storage/pod-startup/volume-types/configmap/deployment_with_configmap.yaml

+          name: vol-{{AddInt $volumeIndex (MultiplyInt $index $volumesPerPod)}}
+      {{ end }}
+      dnsPolicy: Default
+      terminationGracePeriodSeconds: 1


Please remove - I don't think we need it for anything.

wojtek-t · 2019-07-31T07:05:31Z

...sting/experimental/storage/pod-startup/volume-types/configmap/deployment_with_configmap.yaml

+      - key: "node.kubernetes.io/unreachable"
+        operator: "Exists"
+        effect: "NoExecute"
+        tolerationSeconds: 900


All the comments in this file apply to all other files too.

Signed-off-by: Mucahit Kurt <[email protected]>

wojtek-t · 2019-08-01T10:58:49Z

/lgtm
/approve

thanks!

k8s-ci-robot · 2019-08-01T10:59:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mucahitkurt, wojtek-t

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~~ [wojtek-t]

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

k8s-ci-robot assigned wojtek-t Jul 24, 2019

k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 24, 2019

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 24, 2019

k8s-ci-robot requested review from krzysied and mborsz July 24, 2019 21:39

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 24, 2019

wojtek-t reviewed Jul 25, 2019

View reviewed changes

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

mucahitkurt force-pushed the storage/perf-tests/use-deployment-instead-of-raw-pod branch from dc55398 to 3271c43 Compare July 30, 2019 20:40

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

wojtek-t reviewed Jul 31, 2019

View reviewed changes

use deployment instead of raw pod

f9843f0

Signed-off-by: Mucahit Kurt <[email protected]>

mucahitkurt force-pushed the storage/perf-tests/use-deployment-instead-of-raw-pod branch from 3271c43 to f9843f0 Compare July 31, 2019 19:06

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2019

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

k8s-ci-robot merged commit 70affa1 into kubernetes:master Aug 1, 2019

mucahitkurt mentioned this pull request Sep 7, 2019

REQUEST: New membership for mucahitkurt kubernetes/org#1166

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use deployment instead of raw pod for storage perf tests #687

Use deployment instead of raw pod for storage perf tests #687

mucahitkurt commented Jul 24, 2019

k8s-ci-robot commented Jul 24, 2019

msau42 commented Jul 24, 2019

msau42 commented Jul 24, 2019

hantaowang commented Jul 25, 2019

mucahitkurt commented Jul 25, 2019 •

edited

Loading

wojtek-t Jul 25, 2019

hantaowang commented Jul 25, 2019

wojtek-t commented Jul 25, 2019

hantaowang commented Jul 25, 2019

wojtek-t commented Jul 25, 2019

mucahitkurt commented Jul 30, 2019

wojtek-t commented Jul 30, 2019

mucahitkurt commented Jul 30, 2019

wojtek-t Jul 31, 2019

mucahitkurt Jul 31, 2019

wojtek-t Jul 31, 2019

wojtek-t Jul 31, 2019

wojtek-t Jul 31, 2019

wojtek-t commented Aug 1, 2019

k8s-ci-robot commented Aug 1, 2019

Use deployment instead of raw pod for storage perf tests #687

Use deployment instead of raw pod for storage perf tests #687

Conversation

mucahitkurt commented Jul 24, 2019

k8s-ci-robot commented Jul 24, 2019

msau42 commented Jul 24, 2019

msau42 commented Jul 24, 2019

hantaowang commented Jul 25, 2019

mucahitkurt commented Jul 25, 2019 • edited Loading

wojtek-t Jul 25, 2019

Choose a reason for hiding this comment

hantaowang commented Jul 25, 2019

wojtek-t commented Jul 25, 2019

hantaowang commented Jul 25, 2019

wojtek-t commented Jul 25, 2019

mucahitkurt commented Jul 30, 2019

wojtek-t commented Jul 30, 2019

mucahitkurt commented Jul 30, 2019

wojtek-t Jul 31, 2019

Choose a reason for hiding this comment

mucahitkurt Jul 31, 2019

Choose a reason for hiding this comment

wojtek-t Jul 31, 2019

Choose a reason for hiding this comment

wojtek-t Jul 31, 2019

Choose a reason for hiding this comment

wojtek-t Jul 31, 2019

Choose a reason for hiding this comment

wojtek-t commented Aug 1, 2019

k8s-ci-robot commented Aug 1, 2019

mucahitkurt commented Jul 25, 2019 •

edited

Loading