-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Add helm flags to control the app probe rewrite beahvior. #10344
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
Changes from all commits
cc0c84a
6ea3a5a
68a1bb0
8264e41
6a0a4fe
1adcd35
55ef661
7120b70
71a6a88
a2a0df5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ gateways: | |
| # | ||
| sidecarInjectorWebhook: | ||
| enabled: true | ||
| rewriteAppHTTPProbe: false | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lots of comments please.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once again - after reverting please add comments explaining what it does. Nobody can understand - rewrite how, when to set, why, etc. Don't assume all users are familiar with the your implementation details and goals. |
||
|
|
||
| # | ||
| # galley configuration, refer to charts/galley/values.yaml | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,7 @@ var ( | |
| verbosity int | ||
| versionStr string // override build version | ||
| enableCoreDump bool | ||
| rewriteAppHTTPProbe bool | ||
| imagePullPolicy string | ||
| statusPort int | ||
| readinessInitialDelaySeconds uint32 | ||
|
|
@@ -274,6 +275,7 @@ istioctl kube-inject -f deployment.yaml -o deployment-injected.yaml --injectConf | |
| if sidecarTemplate, err = inject.GenerateTemplateFromParams(&inject.Params{ | ||
| InitImage: inject.InitImageName(hub, tag, debugMode), | ||
| ProxyImage: inject.ProxyImageName(hub, tag, debugMode), | ||
| RewriteAppHTTPProbe: rewriteAppHTTPProbe, | ||
| Verbosity: verbosity, | ||
| SidecarProxyUID: sidecarProxyUID, | ||
| Version: versionStr, | ||
|
|
@@ -372,6 +374,8 @@ func init() { | |
| injectCmd.PersistentFlags().BoolVar(&enableCoreDump, "coreDump", | ||
| true, "Enable/Disable core dumps in injected Envoy sidecar (--coreDump=true affects "+ | ||
| "all pods in a node and should only be used the cluster admin)") | ||
| injectCmd.PersistentFlags().BoolVar(&rewriteAppHTTPProbe, "rewriteAppProbe", false, "Whether injector "+ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's a flag - why do you need it on the config map ? |
||
| "rewrites the liveness health check to let kubelet health check the app when mtls is on.") | ||
| injectCmd.PersistentFlags().StringVar(&imagePullPolicy, "imagePullPolicy", inject.DefaultImagePullPolicy, | ||
| "Sets the container image pull policy. Valid options are Always,IfNotPresent,Never."+ | ||
| "The default policy is IfNotPresent.") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ func TestRewriteAppHTTPProbe(t *testing.T) { | |
| { | ||
| name: "one-container", | ||
| sidecar: &SidecarInjectionSpec{ | ||
| RewriteAppHTTPProbe: true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't make any sense - why would this be part of the spec, next to containers ? |
||
| Containers: []corev1.Container{ | ||
| { | ||
| Name: "istio-proxy", | ||
|
|
@@ -89,6 +90,7 @@ func TestRewriteAppHTTPProbe(t *testing.T) { | |
| { | ||
| name: "both-readiness-liveness-rewrite", | ||
| sidecar: &SidecarInjectionSpec{ | ||
| RewriteAppHTTPProbe: true, | ||
| Containers: []corev1.Container{ | ||
| { | ||
| Name: "istio-proxy", | ||
|
|
@@ -160,6 +162,7 @@ func TestRewriteAppHTTPProbe(t *testing.T) { | |
| { | ||
| name: "no-statusPort-find", | ||
| sidecar: &SidecarInjectionSpec{ | ||
| RewriteAppHTTPProbe: true, | ||
| Containers: []corev1.Container{ | ||
| { | ||
| Name: "istio-proxy", | ||
|
|
@@ -209,6 +212,7 @@ func TestRewriteAppHTTPProbe(t *testing.T) { | |
| { | ||
| name: "-statusPort=15020-parsing", | ||
| sidecar: &SidecarInjectionSpec{ | ||
| RewriteAppHTTPProbe: true, | ||
| Containers: []corev1.Container{ | ||
| { | ||
| Name: "istio-proxy", | ||
|
|
@@ -261,6 +265,7 @@ func TestRewriteAppHTTPProbe(t *testing.T) { | |
| { | ||
| name: "--statusPort=15020-parsing", | ||
| sidecar: &SidecarInjectionSpec{ | ||
| RewriteAppHTTPProbe: true, | ||
| Containers: []corev1.Container{ | ||
| { | ||
| Name: "istio-proxy", | ||
|
|
@@ -313,6 +318,7 @@ func TestRewriteAppHTTPProbe(t *testing.T) { | |
| { | ||
| name: "two-container-rewrite", | ||
| sidecar: &SidecarInjectionSpec{ | ||
| RewriteAppHTTPProbe: true, | ||
| Containers: []corev1.Container{ | ||
| { | ||
| Name: "istio-proxy", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| apiVersion: extensions/v1beta1 | ||
| kind: Deployment | ||
| metadata: | ||
| creationTimestamp: null | ||
| name: hello | ||
| spec: | ||
| replicas: 7 | ||
| strategy: {} | ||
| template: | ||
| metadata: | ||
| annotations: | ||
| sidecar.istio.io/status: '{"version":"","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}' | ||
| creationTimestamp: null | ||
| labels: | ||
| app: hello | ||
| tier: backend | ||
| track: stable | ||
| spec: | ||
| containers: | ||
| - image: fake.docker.io/google-samples/hello-go-gke:1.0 | ||
| livenessProbe: | ||
| httpGet: | ||
| port: http | ||
| name: hello | ||
| ports: | ||
| - containerPort: 80 | ||
| name: http | ||
| readinessProbe: | ||
| httpGet: | ||
| port: 3333 | ||
| resources: {} | ||
| - image: fake.docker.io/google-samples/hello-go-gke:1.0 | ||
| livenessProbe: | ||
| httpGet: | ||
| port: http | ||
| name: world | ||
| ports: | ||
| - containerPort: 90 | ||
| name: http | ||
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - cat | ||
| - /tmp/healthy | ||
| resources: {} | ||
| - args: | ||
| - proxy | ||
| - sidecar | ||
| - --configPath | ||
| - /etc/istio/proxy | ||
| - --binaryPath | ||
| - /usr/local/bin/envoy | ||
| - --serviceCluster | ||
| - hello.default | ||
| - --drainDuration | ||
| - 2s | ||
| - --parentShutdownDuration | ||
| - 3s | ||
| - --discoveryAddress | ||
| - istio-pilot:15007 | ||
| - --connectTimeout | ||
| - 1s | ||
| - --statsdUdpAddress | ||
| - "" | ||
| - --proxyAdminPort | ||
| - "15000" | ||
| - --controlPlaneAuthPolicy | ||
| - NONE | ||
| - --statusPort | ||
| - "15020" | ||
| - --applicationPorts | ||
| - 80,90 | ||
| env: | ||
| - name: POD_NAME | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: metadata.name | ||
| - name: POD_NAMESPACE | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: metadata.namespace | ||
| - name: INSTANCE_IP | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: status.podIP | ||
| - name: ISTIO_META_POD_NAME | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: metadata.name | ||
| - name: ISTIO_META_INTERCEPTION_MODE | ||
| value: REDIRECT | ||
| image: docker.io/istio/proxyv2:unittest | ||
| imagePullPolicy: IfNotPresent | ||
| name: istio-proxy | ||
| ports: | ||
| - containerPort: 15090 | ||
| name: http-envoy-prom | ||
| protocol: TCP | ||
| readinessProbe: | ||
| failureThreshold: 30 | ||
| httpGet: | ||
| path: /healthz/ready | ||
| port: 15020 | ||
| initialDelaySeconds: 1 | ||
| periodSeconds: 2 | ||
| resources: | ||
| requests: | ||
| cpu: 10m | ||
| securityContext: | ||
| readOnlyRootFilesystem: true | ||
| volumeMounts: | ||
| - mountPath: /etc/istio/proxy | ||
| name: istio-envoy | ||
| - mountPath: /etc/certs/ | ||
| name: istio-certs | ||
| readOnly: true | ||
| initContainers: | ||
| - args: | ||
| - -p | ||
| - "15001" | ||
| - -u | ||
| - "1337" | ||
| - -m | ||
| - REDIRECT | ||
| - -i | ||
| - '*' | ||
| - -x | ||
| - "" | ||
| - -b | ||
| - 80,90 | ||
| - -d | ||
| - "15020" | ||
| image: docker.io/istio/proxy_init:unittest | ||
| imagePullPolicy: IfNotPresent | ||
| name: istio-init | ||
| resources: {} | ||
| securityContext: | ||
| capabilities: | ||
| add: | ||
| - NET_ADMIN | ||
| volumes: | ||
| - emptyDir: | ||
| medium: Memory | ||
| name: istio-envoy | ||
| - name: istio-certs | ||
| secret: | ||
| optional: true | ||
| secretName: istio.default | ||
| status: {} | ||
| --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confused - why ?
Typically we use pod annotations to allow user to opt in/out of a specific feature.