Extend load test to immutable secrets/configmaps #1146
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
approved
wojtek-t
force-pushed
the
immutable_secrets_test
branch
from
7542b31
to
addcb72
Compare
|
/retest |
k8s-ci-robot
added
the
needs-rebase
wojtek-t
force-pushed
the
immutable_secrets_test
branch
from
addcb72
to
9bd93ce
Compare
k8s-ci-robot
removed
the
needs-rebase
wojtek-t
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
| @@ -2,6 +2,9 @@ apiVersion: v1 | |||
| kind: ConfigMap | |||
| metadata: | |||
| name: {{.Name}} | |||
| {{if not (eq (Mod .Index 20) 0 19) }} # .Index % 20 in {0,19} - only 10% deployments will have non-immutable ConfigMap. | |||
There was a problem hiding this comment.
The if condition seems to be: .Index % 20 not in {0,19} which translates to 90% of deployments will have immutable Config map.
It's basically what you wrote, but with less negations - easier to parse.
NIT: Consider rewriting
There was a problem hiding this comment.
Let's merge as is for now and will clean up later - I'm slightly afraid of breaking something now and I would like that to land asap to have as much soaking time as possible.
There was a problem hiding this comment.
I proposed to change only the comment, but I'm fine with doing that later. Thanks!
| @@ -28,14 +28,10 @@ spec: | |||
| cpu: {{$CpuRequest}} | |||
| memory: {{$MemoryRequest}} | |||
| volumeMounts: | |||
| {{if (eq (Mod .Index 20) 0 19) }} # .Index % 20 in {0,19} - 10% deployments will have ConfigMap | |||
There was a problem hiding this comment.
So if I understand the change correctly, now every deployment will have both configmap and secret, 90% will be immutable, 10% mutable? If so, can we make it clear in the PR description / commit message?
There was a problem hiding this comment.
Yes - that's exactly the intention. Updating.
wojtek-t
force-pushed
the
immutable_secrets_test
branch
from
9bd93ce
to
688f020
Compare
|
@mm4tt - PTAL |
| @@ -2,6 +2,9 @@ apiVersion: v1 | |||
| kind: ConfigMap | |||
| metadata: | |||
| name: {{.Name}} | |||
| {{if not (eq (Mod .Index 20) 0 19) }} # .Index % 20 in {0,19} - only 10% deployments will have non-immutable ConfigMap. | |||
There was a problem hiding this comment.
I proposed to change only the comment, but I'm fine with doing that later. Thanks!
k8s-ci-robot
added
the
lgtm
|
FYI, @jprzychodzen current oncall |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mm4tt, 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:
Approvers can indicate their approval by writing |
Each deployment in load test has now both secret and configmap mounted. 90% of them are marked as immutable, only 10% is mutable (as it was before).
Scalability test coverage is a requirement for graduating "Immutable Secrets/ConfigMaps" feature to Beta.
This has been scale-tested already and the features was enabled by default in kubernetes/kubernetes#89594
ref kubernetes/enhancements#1412