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

Control skip port flag via custom annotation in resource #1997

Closed
Bedotech opened this issue Dec 19, 2018 · 6 comments
Closed

Control skip port flag via custom annotation in resource #1997

Bedotech opened this issue Dec 19, 2018 · 6 comments
Assignees
Labels

Comments

@Bedotech
Copy link

Bedotech commented Dec 19, 2018

Feature Request

For disable protocol auto detection on specific port we have this option on linkerd inject cli command --skip-outbound-ports and --skip-outbound-ports but when auto injection is in use we cannot specify this option for each resource.

What problem are you trying to solve?

Auto injection sidecar proxy is really awesome because with these feature we cover very large of deploy case without make difficult for the end user setup linkerd2, but we need configure what port haven't to be proxied so at this time we cannot use MySql over non standard port with auto inject feature.

How should the problem be solved?

The problem can be solved to add a custom annotation on kubernetes resources for specify what port must pass at the deployment level, the same things we already do for specify when the resource must not be injected with the sidecar proxy.

We can add two custom resource annotation:

  1. linkerd.io/skip-inbound-ports: <uintSlice>
  2. linkerd.io/skip-outbound-ports: <uintSlice>

We use the flag for the annotation and cli so is more easy for the user understand how this work and how to use it.

If the value aren't specified take the default.

This feature is allowed only in conjunction with auto injection, so we haven't to define who have the precedence in the configuration injection between the linkerd inject or annotation over the resource.

The main drawback is the are more configuration, but this feature can be enabled only in some specific case, and is already configured per resource as other configuration for linkerd.

For this use case i suggest to use annotations over labels both are metadata associated to a resource but labels are preferred for query purpose, we don't need this feature so annotation are ok.

Any alternatives you've considered?

Maybe take this info on the control plane, but is really an hard work keep the sync between deployments and new proxy subscription.

How would users interact with this feature?

For example when deploy a new service:

apiVersion: apps/v1beta2
kind: Deployment
metadata:
  name: database-{{ template "chart.fullname" . }}
  labels:
    linkerd.io/auto-inject: disabled
    app: database-{{ template "chart.name" . }}
    chart: {{ template "chart.chart" . }}
    release: {{ .Release.Name }}
    heritage: {{ .Release.Service }}
  annotations:
    linkerd.io/skip-inbound-ports: 4433
spec:
  replicas: {{ .Values.replicaCount }}
  selector:
    matchLabels:
      app: database-{{ template "chart.name" . }}
      release: {{ .Release.Name }}
  template:
    metadata:
      labels:
        linkerd.io/auto-inject: disabled
        app: database-{{ template "chart.name" . }}
        release: {{ .Release.Name }}
    spec:
      containers:
        - name: database
          image: mariadb:10.3
          imagePullPolicy: Always
          ports:
          - name: mysql
            containerPort: 4433

          livenessProbe:
            exec:
              command: ["mysqladmin", "ping"]
            initialDelaySeconds: 30
            periodSeconds: 10
            timeoutSeconds: 5
          readinessProbe:
            exec:
              # Check we can execute queries over TCP (skip-networking is off).
              command: ["mysql", "-h", "127.0.0.1", "-e", "SELECT 1"]
            initialDelaySeconds: 5
            periodSeconds: 2
            timeoutSeconds: 1

I can use this deployment and auto inject feature detect the annotation and apply the option to the injected linkerd-proxy

Sorry for my bad english, i hope the intent are clear.

@grampelberg
Copy link
Contributor

I love this! It'd be awesome to work on along with #1862 .

@Aagat
Copy link

Aagat commented Feb 15, 2019

This is a great idea!

This makes sense for my use case in conjunction with linkerd.io/auto-inject: enabled annotation to selectively enable proxy injection for certain deployments while excluding certain ports.

@klingerf
Copy link
Contributor

I agree! We're actively working on annotation-based overrides for all inject configuration options as part of #2287. That will include overrides for inbound/outbound ports.

@ihcsim
Copy link
Contributor

ihcsim commented Feb 15, 2019

EDIT: @klingerf described a better approach at #1997 (comment), by using

linkerd install --proxy-auto-inject --skip-inbound-ports 4222,6222 | kubectl apply -f -

Keep in mind that this is a global change which will also affect the proxies in the control plane.


The current workaround is to modify the the config map which the proxy-injector reads from. Inside the linkerd-proxy-injector-sidecar-config config map (within the linkerd namespace), there is aproxy-init.yaml key, with the --inbound-ports-to-ignore property.

If for example, I am auto-injecting into a NATS server, I'll add port 4222 and 6222 to the --inbound-ports-to-ignore comma-separated list. After the config map is updated, if the NATS deployment already exists, it will need to be re-created (unfortunately). Simply deleting the existing pods or updating the deployment (with kubectl apply etc.) will not trigger the admission webhook controller. We have a separate issue (#2260) to update the webhook configuration to pick up update events as well.

@klingerf
Copy link
Contributor

klingerf commented Feb 15, 2019

@ihcsim I don't think there's a need to modify the ConfigMap in the linkerd install output directly. The install command accepts flags to modify the values in that ConfigMap. For instance, for the example you gave, I recommend running:

linkerd install --proxy-auto-inject --skip-inbound-ports 4222,6222

But the values in the ConfigMap are applied globally and can't be overridden per injected deployment. We're planning on fixing that by adding support for overriding the values via annotations, as described in this issue.

@ihcsim
Copy link
Contributor

ihcsim commented Mar 14, 2019

This is fixed by #2471.

@ihcsim ihcsim closed this as completed Mar 14, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants