agent: move magic concurrency equation to injector #32928
Conversation
google-cla
bot
added
the
cla: yes
istio-testing
added
the
size/XXL
kyessenov
force-pushed
the
injector_remove_requests_limits
branch
from
ab37cb6
to
3f48f18
Compare
kyessenov
added
the
release-notes-none
| @@ -190,9 +190,9 @@ spec: | |||
| {{- if .Values.global.logAsJson }} | |||
| - --log_as_json | |||
| {{- end }} | |||
| {{- if gt .ProxyConfig.Concurrency.GetValue 0 }} | |||
| {{- if gt .EstimatedConcurrency 0 }} | |||
There was a problem hiding this comment.
Can we check it actually exists? otherwise you may end up with errors like Error creating: admission webhook "namespace.sidecar-injector.istio.io" denied the request: failed to run injection template: template: inject:193:12: executing "inject" at <.EstimatedConcurrency>: can't evaluate field EstimatedConcurrency in type *inject.SidecarTemplateData if injector configmap != istiod version.
In theory it shouldn't happen but it always gets a few users
There was a problem hiding this comment.
I don't think it's possible since we can't add another function either and structs don't have presence checks in golang. Do you have any suggestion?
There was a problem hiding this comment.
Wow that is pretty annoying, I thought you could. Maybe we can just backport the new field to 1.10.1 to make it less likely to occur. this is probably fine - in theory they should be in sync anyhow
There was a problem hiding this comment.
Not an expert, but the general suggestion is to use map instead of struct, precisely because of this issue.
There was a problem hiding this comment.
We can backport a dummy variable in a follow up.
|
/retest |
1 similar comment
|
/retest |
No description provided.