Skip to content
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

Istio injector annotations don't work as described in doc #6476

Closed
tcnghia opened this issue Jun 21, 2018 · 15 comments

Comments

@tcnghia
Copy link

commented Jun 21, 2018

Describe the bug
Istio injector annotations don't work as describe in at https://istio.io/docs/setup/kubernetes/sidecar-injection/#policy,

  • policy=disabled -> The sidecar injector will not inject the sidecar into pods by default. Add the sidecar.istio.io/inject annotation with value true to the pod template spec to enable injection.
  • policy=enabled -> The sidecar injector will inject the sidecar into pods by default. Add the sidecar.istio.io/inject annotation with value false to the pod template spec to disable injection

Expected behavior
When policy=disabled the sidecar injector will not inject the sidecar into pods by default. Add the sidecar.istio.io/inject annotation with value true to the pod template spec to enable injection.

Steps to reproduce the bug
Create a pod in a namespace where policy=disabled or policy unset. Injection will not happen no matter what we set pod annotation to.

Version
0.8

Is Istio Auth enabled or not?
No, auth not eabled.

helm template --namespace=istio-system \
    --set sidecarInjectorWebhook.enabled=true \
    --set global.proxy.image=proxyv2 \
    --set prometheus.enabled=false \
    install/kubernetes/helm/istio > ../istio.yaml

Environment
GKE 1.10

@tcnghia

This comment has been minimized.

Copy link
Author

commented Jun 21, 2018

inject_test.yaml

apiVersion: v1
kind: Namespace
metadata:
  labels:
    istio-injection: enabled
  name: policy-enabled
---
apiVersion: v1
kind: Namespace
metadata:
  labels:
    istio-injection: disabled
  name: policy-disabled
---
apiVersion: v1
kind: Namespace
metadata:
  name: policy-default
---
apiVersion: v1
kind: Pod
metadata:
  name: nolabel
  namespace: policy-enabled
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: nolabel
  namespace: policy-disabled
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: nolabel
  namespace: policy-default
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: labeled-true
  namespace: policy-enabled
  annotations:
    "sidecar.istio.io/inject": "true"
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: labeled-true
  namespace: policy-disabled
  annotations:
    "sidecar.istio.io/inject": "true"
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: labeled-true
  namespace: policy-default
  annotations:
    "sidecar.istio.io/inject": "true"
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: labeled-false
  namespace: policy-enabled
  annotations:
    "sidecar.istio.io/inject": "false"
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: labeled-false
  namespace: policy-disabled
  annotations:
    "sidecar.istio.io/inject": "false"
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
---
apiVersion: v1
kind: Pod
metadata:
  name: labeled-false
  namespace: policy-default
  annotations:
    "sidecar.istio.io/inject": "false"
spec:
  containers:
  - image: docker.io/citizenstig/httpbin
    imagePullPolicy: IfNotPresent
    name: httpbin
$ kubectl get pods --all-namespaces=true | grep "label\|NAME"
NAMESPACE                NAME                                                READY     STATUS              RESTARTS   AGE
policy-default           labeled-false                                       1/1       Running             0          13s
policy-default           labeled-true                                        1/1       Running             0          13s
policy-default           nolabel                                             1/1       Running             0          13s
policy-disabled          labeled-false                                       1/1       Running             0          13s
policy-disabled          labeled-true                                        1/1       Running             0          13s
policy-disabled          nolabel                                             1/1       Running             0          13s
policy-enabled           labeled-false                                       1/1       Running             0          13s
policy-enabled           labeled-true                                        2/2       Running             0          13s
policy-enabled           nolabel                                             2/2       Running             0          13s
@prune998

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

I'm not sure I understang your issue, but :

  • do a kubectl get namespaces -o jsonpath='{.items[*].metadata.labels}' to list namespaces and labels associated. You should see something like map[istio-injection:enabled name:test] if your namespace have the injectin enabled
  • check the content of the configmap istio-sidecar-injector in your istio-system namespace. You can use something like kubectl -n istio-system get configmaps istio-sidecar-injector -o yaml. Your config should start with policy: disabled`
  • check that your deployment includes the annotation "sidecar.istio.io/inject": "false" under the spec->template->metadata->annotations

Note that the Configmap is installed with policy: enabled by default so you have to use --set global.proxy.policy=disabled when using Helm

@ymesika

This comment has been minimized.

Copy link
Member

commented Jun 21, 2018

@prune998, @tcnghia tested all combinations of having ns sidecar injector label vs having annotation in the pod.
His results are a bit confusing because he used the term policy for the NS label and labeled for the annotation.
The bottom line is that the label has high priority than the annotation. For instance, if you have a pod with an annotation to inject deployed into a ns with no label the pod won't get injected.

@tcnghia Did I understand the results correctly?

I asked him to open this issue following his post here because I think we need to clarify what's the expected behaviour for all combinations and fix/document it.

@tcnghia

This comment has been minimized.

Copy link
Author

commented Jun 21, 2018

@ymesika the label doesn't always have higher priority than the annotation. For example, when namespace policy label is enabled and pod annotation is false we see sidecar injection turned off. So the annotation takes precedence in that case. However, when namespace policy label is disabled, namespace policy has higher priority than pod annotation.

The documentation https://istio.io/docs/setup/kubernetes/sidecar-injection/#policy explicitly states that

policy=disabled - The sidecar injector will not inject the sidecar into pods by default. Add the sidecar.istio.io/inject annotation with value true to the pod template spec to enable injection.

but this isn't the case as shown in the pod policy-disabled/labeled-true in my example.

@prune998 does this explain the issue? thanks

I think with the current behavior it is hard to turn on sidecar injection for one particular pod. We will need to opt-in the whole namespace, then opt-out all the other N-1 pods. The behavior stated by the documentation at https://istio.io/docs/setup/kubernetes/sidecar-injection/#policy is better.

@tcnghia

This comment has been minimized.

Copy link
Author

commented Jun 21, 2018

BTW, I saw that the documented policy is consistent with the code in

switch strings.ToLower(annotations[sidecarAnnotationPolicyKey]) {

However, in my webhook logs the AdmissionReview didn't happen for Pods in a namespace with policy disabled.

I hope this information helps.

@ymesika

This comment has been minimized.

Copy link
Member

commented Jun 21, 2018

@tcnghia I think you are confusing the policy from the doc with the namespace label. The policy in the doc refers to the Policy flag within the istio-sidecar-injector ConfigMap.

@tcnghia

This comment has been minimized.

Copy link
Author

commented Jun 21, 2018

@ymesika I see now. Can you please recommend how I use the opt-in policy described in the doc? Do I set policy=disabled on ConfigMap and set sidecar.istio.io/inject=true on the Pod?

Thank

@prune998

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

@tcnghia that's what I'm doing... No sidecar is injected except in the pods I want.
I think you still need to have istio-injection:enabled label on the namespace for it to work

@tcnghia

This comment has been minimized.

Copy link
Author

commented Jun 21, 2018

@ymesika It looks like setting up the ConfigMap policy works as you described.

I summarized the behavior as followed. Please correct me if I am wrong.

Namespace label istio-injection ConfigMap key policy Pod annotation sidecar.istio.io/inject Injection
enabled enabled true yes
enabled enabled false no
enabled enabled yes
enabled disabled true yes
enabled disabled false no
enabled disabled no
disabled enabled true no
disabled enabled false no
disabled enabled no
disabled disabled true no
disabled disabled false no
disabled disabled no

tl;dr: if namespace policy=disabled then no injection. Otherwise, if Pod annotation is true => injection; false => no injection; empty => fallback on the ConfigMap key policy.

In my case I have one namespace where I want to disable sidecar injection by default, except for one Pod (opt-in) (this namespace contains my controllers and webhooks). I also have K namespaces where I want to enable sidecar injection by default (this namespace contains my users' containers). Do you have recommendation of how I can achieve that?

Thanks a lot for the help so far.

@tcnghia

This comment has been minimized.

Copy link
Author

commented Jun 21, 2018

If I understand correctly ConfigMap is a cluster-wide injection policy, namespace label istio-injection is a namespace level injection policy, and Pod annotation sidecar.istio.io/inject is a Pod level injection policy.

Since namespace level policy are not yet documented, may I request that we adopt a hierarchical model? In such a hierarchical model, the policy with the least scope wins. So sidecar.istio.io/inject has highest priority, followed by namespace label istio-injection, then injector config ConfigMap key policy. Currently I think the namespace label istio-injection already trumps the injector ConfigMap key policy, so this proposal won't be too different from previously. The only two cases that will change are

Namespace label istio-injection ConfigMap key policy Pod annotation sidecar.istio.io/inject Injection
disabled enabled true no -> yes
disabled disabled true no -> yes

That would simplify things and allow application frameworks to ensure sidecar injection on Pods that they created on behalf of their users no matter what namespace they are in.

Some more background: in our current use case we create Pods on behalf of our users in their namespaces. Our Pods should always get sidecar injection -- but currently we can't ensure that without asking the user to add the label to their namespace first. Our users may not want to do so, since adding the label istio-injection=true to their namespace will all other Pods to be injected as well unless they also add sidecar.istio.io/inject=false in all their other Pods.

@ymesika @prune998 what do you think?

@ayj

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

tl;dr: if namespace policy=disabled then no injection.

We don't have a namespace policy. The mutatingwebhookconfiguration's NamespaceSelector determines whether or not the webhook is invoked at all by the api-server. It can't be overridden by the per-pod annotation.

As an enhancement we could replace the boolean policy option in the istio-inject ConfigMap with namespace policy selector.

  1. mutatingwebhookconfiguration's NamespaceSelector => selects when webhook is invoked
  2. namespace policy selector => selects default opt-in / opt-out policy per namespace using label selector
  3. annotation => allows pod-level overrides of namespace policy selector (#2).

@sakshigoel12 sakshigoel12 added this to the 1.0 milestone Jun 22, 2018

@ostromart

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

This is a documentation issue, dropping to P1.

@stale

This comment has been minimized.

Copy link

commented Jul 22, 2018

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 2 weeks unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale

This comment has been minimized.

Copy link

commented Nov 18, 2018

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 18, 2018

@stale

This comment has been minimized.

Copy link

commented Dec 19, 2018

This issue has been automatically closed because it has not had activity in the last month and a half. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted". Thank you for your contributions.

@stale stale bot closed this Dec 19, 2018

@rlenglet rlenglet removed this from the 1.4 milestone Jul 9, 2019

@rlenglet rlenglet modified the milestones: 1.3, 1.2 Jul 9, 2019

@fpesce fpesce modified the milestones: 1.2, 1.1 Jul 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
9 participants
You can’t perform that action at this time.