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

Support for StatefulSets and Manual Sidecar Config #944

Closed
ewohltman opened this issue Mar 3, 2020 · 7 comments
Closed

Support for StatefulSets and Manual Sidecar Config #944

ewohltman opened this issue Mar 3, 2020 · 7 comments
Labels
question Further information is requested

Comments

@ewohltman
Copy link
Contributor

Hello!

Does jaeger-operator have support for injecting sidecars into StatefulSets yet? Not sure if I am doing my annotation wrong, or if jaeger-operator still only supports deployments (I looked through other issues and documentation but it wasn't super clear to me).

If StatefulSets are not yet supported, could the documentation include an example on how to manually add the sidecar config and be clear that only Deployments are currently supported?

Thanks!

@ghost ghost added the needs-triage New issues, in need of classification label Mar 3, 2020
@jpkrohling jpkrohling added question Further information is requested and removed needs-triage New issues, in need of classification labels Mar 3, 2020
@jpkrohling
Copy link
Contributor

jpkrohling commented Mar 3, 2020

StatefulSets are not supported yet. I thought we had an example in the documentation, but looks like we don't. Here's how a sidecar in a deployment looks like:

- name: jaeger-agent
image: jaegertracing/jaeger-agent:1.16.0
env:
- name: POD_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
args:
- --reporter.grpc.host-port=dns:///jaeger-collector-headless.$(POD_NAMESPACE).svc.cluster.local:14250
ports:
- containerPort: 6831
name: jg-compact-trft
protocol: UDP

Would you be open to improving our documentation by adding this? Perhaps a new section close to the auto-inject part, called "Manually adding sidecars"?

@ewohltman
Copy link
Contributor Author

Thanks very much! I'll look to update the documentation later today.

@jpkrohling
Copy link
Contributor

You might also want to deploy the following example, and compare the deployment's state after the operator injects the sidecar:

apiVersion: apps/v1
kind: Deployment
metadata:
name: myapp
annotations:
"sidecar.jaegertracing.io/inject": "true"
spec:
selector:
matchLabels:
app: myapp
template:
metadata:
labels:
app: myapp
annotations:
sidecar.istio.io/inject: "true"
spec:
containers:
- name: myapp
image: jaegertracing/vertx-create-span:operator-e2e-tests
ports:
- containerPort: 8080
protocol: TCP

That sidecar should be the one used in the documentation, as it's more general purpose (it opens more ports). The one from my previous comment uses only one port, which is OK when you know the client that is being used.

@ewohltman
Copy link
Contributor Author

ewohltman commented Mar 4, 2020

So I gathered this sidecar definition from using the annotation with a deployment:

- name: jaeger-agent
  image: jaegertracing/jaeger-agent:1.17.0
  imagePullPolicy: IfNotPresent
  ports:
    - containerPort: 5775
      name: zk-compact-trft
      protocol: UDP
    - containerPort: 5778
      name: config-rest
      protocol: TCP
    - containerPort: 6831
      name: jg-compact-trft
      protocol: UDP
    - containerPort: 6832
      name: jg-binary-trft
      protocol: UDP
    - containerPort: 14271
      name: admin-http
      protocol: TCP
  resources: {}
  env:
    - name: POD_NAME
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: metadata.name
    - name: HOST_IP
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: status.hostIP
  args:
    - --jaeger.tags=cluster=undefined,deployment.name=[myDeployment],pod.namespace=[myNamespace],pod.name=${POD_NAME:},host.ip=${HOST_IP:},container.name=[myContainer]
    - --reporter.grpc.host-port=dns:///jaeger-collector-headless.observability:14250
    - --reporter.type=grpc

Should the documentation include everything except the - --jarger.tags argument since that refers to a deployment and specific namespace/container names?

Should I also change the image field to image: jaegertracing/jaeger-agent:latest instead of a specific version number?

Is there anything else that should be included?

Thanks!

@jpkrohling
Copy link
Contributor

I think --jaeger.tags can indeed be removed. The env vars can be removed as well, as they are used only as part of the --jaeger.tags. The empty resources node can also be removed.

Not sure about the image. @pavolloffay mentioned in a comment that it would be best to keep the right versions on the versioned doc and master for the next-release doc.

@ewohltman
Copy link
Contributor Author

PRs opened to include a full StatefulSet example in jaegertracing/jaeger-operator and another in jaegertracing/documentation to include documentation on manual definitions with a link to the full StatefulSet example.

@ewohltman
Copy link
Contributor Author

My question has been answered and documentation has been updated. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants