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

Update proxy injector webhook to respond to UPDATE events #2332

Merged
merged 1 commit into from Feb 21, 2019

Conversation

ihcsim
Copy link
Contributor

@ihcsim ihcsim commented Feb 20, 2019

This change will cause the k8s api server to call the proxy injector webhook when deployment resources are updated. It extends the current auto-inject-on-create-only behaviour to enable user's existing workloads to be auto-injected, without the needs to re-create them.

Tested with the kubectl apply, edit and annotate commands, with the modified emojivoto manifests in this gist.

Note that:

  • Annotating a namespace doesn't trigger the webhook. An apply on the deployments is still necessary.
  • The annotate command can only annotates the deployment, not the pod template.
$ bin/linkerd version
Client version: git-11bf01a1
Server version: git-11bf01a1

$ bin/linkerd install --tls=optional --proxy-auto-inject | kubectl apply -f -

# use `apply` to add annotation to the pod template
$ kubectl apply -f emojivoto-unmeshed.yml
$ kubectl apply -f emojivoto-annotate-pods.yml
$ kubectl -n emojivoto get po
NAME                        READY   STATUS    RESTARTS   AGE
emoji-676c4b5969-dx6bd      2/2     Running   0          3m47s
vote-bot-666b9dcc77-6t88h   2/2     Running   0          3m47s
voting-b78bdbf96-pzbc7      2/2     Running   0          3m47s
web-585467d95-vhgh2         2/2     Running   0          3m47s

# use `apply` to add annotation to the namespace
$ kubectl apply -f emojivoto-unmeshed.yml
$ kubectl apply -f emojivoto-annotate-namespace.yml
$ kubectl -n emojivoto get po
NAME                        READY   STATUS    RESTARTS   AGE
emoji-856897d8fc-6s7b7      2/2     Running   0          2m23s
vote-bot-6b76c4787c-fm9lm   2/2     Running   0          2m23s
voting-749fb5c59b-v62lp     2/2     Running   0          2m23s
web-59584fcc9-dwqzj         2/2     Running   0          2m23s

# use `edit` to annotate namespace
# an `apply` on the deployments is still necessary
$ kubectl edit ns emojivoto
$ kubectl apply -f emojivoto-unmeshed.yml
$ kubectl -n emojivoto get po
NAME                        READY   STATUS    RESTARTS   AGE
emoji-856897d8fc-8jggh      2/2     Running   0          33s
vote-bot-6b76c4787c-8tkfl   2/2     Running   0          30s
voting-749fb5c59b-h4vdf     2/2     Running   0          31s
web-59584fcc9-554xk         2/2     Running   0         31s

# use `edit` to annotate deployment
$ kubectl -n emojivoto edit deploy web
$ kubectl -n emojivoto get po
NAME                        READY   STATUS    RESTARTS   AGE
emoji-86cbb5c67d-w9krh      1/1     Running   0          5m
vote-bot-77996c4698-shp4l   1/1     Running   0          4m59s
voting-6c6494c57c-j5txl     1/1     Running   0          5m
web-585467d95-6lpg2         2/2     Running   0          91s

# use `annotate` to annotate namespace
# an `apply` on the deployments is still necessary
$ kubectl annotate ns emojivoto linkerd.io/inject=enabled
$ kubectl apply -f emojivoto-unmeshed.yml
$ kubectl -n emojivoto get po
NAME                        READY   STATUS    RESTARTS   AGE
emoji-856897d8fc-8jggh      2/2     Running   0          2m31s
vote-bot-6b76c4787c-8tkfl   2/2     Running   0          2m30s
voting-749fb5c59b-h4vdf     2/2     Running   0          2m31s
web-59584fcc9-554xk         2/2     Running   0          2m31s

# make sure custom configs are preserved
$ linkerd inject --tls=optional --skip-inbound-ports=4222,6222 | kubectl apply -f -
$ kubectl -n emojivoto edit deploy emoji
..... # annotate the pod template
     template:
       metadata:
         annotations:
           linkerd.io/created-by: linkerd/cli edge-19.2.2
           linkerd.io/proxy-version: edge-19.2.2
           linkerd.io/inject: enabled
....
$ kubectl describe deploy emoji
..... # ignore ports still there
      Args:
        ...
        --inbound-ports-to-ignore
          4222,6222,4190,4191

Fixes #2260

Signed-off-by: Ivan Sim ivan@buoyant.io

Signed-off-by: Ivan Sim <ivan@buoyant.io>
@ihcsim ihcsim self-assigned this Feb 20, 2019
@ihcsim ihcsim added this to In progress in Inject/Install Configuration via automation Feb 20, 2019
@ihcsim ihcsim moved this from In progress to Needs review in Inject/Install Configuration Feb 20, 2019
@alpeb
Copy link
Member

alpeb commented Feb 20, 2019

Great! so this means it would just be a matter of properly documenting these behaviors.
BTW, I had no idea apply is actually incremental (not overwriting things done by edit or previous applys)...

Inject/Install Configuration automation moved this from Needs review to Reviewer approved Feb 20, 2019
Copy link
Member

@klingerf klingerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⭐️ Great! Am glad to see this was such a straightforward fix.

To test this I first deployed the books app to a namespace that doesn't include the linkerd.io/inject: enabled annotation, then I edited the books deployment's pod spec to include the annotation, and I verified that the books pod is redeployed with the linkerd-proxy container. 👍

I think you're already aware that this branch doesn't support the linkerd.io/inject: disabled annotation on update, but wanted to mention it anyway. For instance, if I deploy the books app to a namespace that does include the linkerd.io/inject: enabled annotation, all pods in the namespace are injected on create. If I then edit the books deployment's pod spec to include the linkerd.io/inject: disabled annotation, the books pod is not redeployed and it continues to run with the linkerd-proxy container.

I'm totally fine handling the uninject use case separately, but would you mind creating an issue so that we don't lose track of it when #2260 closes?

@ihcsim ihcsim merged commit c86b2b8 into master Feb 21, 2019
Inject/Install Configuration automation moved this from Reviewer approved to Done Feb 21, 2019
@ihcsim ihcsim deleted the isim/webhook-update-event branch February 21, 2019 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants