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

Sidecar injection does not create container probers #571

Closed
adriankostrubiak-tomtom opened this issue Nov 22, 2021 · 2 comments · Fixed by #574
Closed

Sidecar injection does not create container probers #571

adriankostrubiak-tomtom opened this issue Nov 22, 2021 · 2 comments · Fixed by #574

Comments

@adriankostrubiak-tomtom
Copy link
Contributor

When a OpenTelemetryCollector running in sidecar mode is configured with a spec.config containing a healthcheck extenstion, I would expect that the injected container contain at least a LivenessProbe. In the current state, this is not the case.


With a configuration like

---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: otlp-sidecar
  namespace: my-namespace
spec:
  mode: sidecar
  config: |
    receivers:
      otlp:
        protocols:
          grpc:
    processors:
      batch:
        ....
    exporters:
      otlp:
        endpoint: distributor....
    extensions:
      health_check:
    service:
      extensions: [health_check]
      pipelines:
        traces:
          receivers: [otlp]
          processors: [batch]
          exporters: [otlp]

I would expect the injected sidecar to contain a readiness and liveness probe.

The running collector agent does expose the healthcheck, as one would expect:

$ curl -v http://0.0.0.0:13133
*   Trying 0.0.0.0:13133...
....[snip]...
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Mon, 22 Nov 2021 21:08:33 GMT
< Content-Length: 102
< 
* Connection #0 to host 0.0.0.0 left intact
{"status":"Server available","upSince":"2021-11-22T14:41:53.003085969Z","uptime":"6h26m40.625191838s"}$ 

However there is no LivenessProbe nor a ReadinessProbe defined in the creation of the container:

return corev1.Container{
Name: naming.Container(),
Image: image,
ImagePullPolicy: otelcol.Spec.ImagePullPolicy,
VolumeMounts: volumeMounts,
Args: args,
Env: envVars,
EnvFrom: otelcol.Spec.EnvFrom,
Resources: otelcol.Spec.Resources,
SecurityContext: otelcol.Spec.SecurityContext,
}

And we end up with an injected container such as:

  - args:
    - --config=/conf/collector.yaml
    env:
    - name: POD_NAME
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: metadata.name
    image: otel/opentelemetry-collector:0.38.0
    imagePullPolicy: IfNotPresent
    name: otc-container
    resources:
      limits:
        memory: 96Mi
      requests:
        cpu: 20m
        memory: 96Mi
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /conf
      name: otc-internal

I'd be happy to take a look at this if this is a desired feature.

@jpkrohling
Copy link
Member

This is absolutely desirable. I think we did talk about this in the past, @VineethReddy02, perhaps you have further ideas on the topic?

adriankostrubiak-tomtom added a commit to adriankostrubiak-tomtom/opentelemetry-operator that referenced this issue Nov 23, 2021
@adriankostrubiak-tomtom
Copy link
Contributor Author

Cool. I decided to take a stab at this based on what I think makes sense, at least for a first cut. That's the above draft PR.

I welcome any feedback on the changes.

jpkrohling pushed a commit that referenced this issue Nov 26, 2021
…orts a health_check. (#574)

* Add liveness probe to created container if otelcol configuration supports a health_check.
Fixes #571

Signed-off-by: Adrian Kostrubiak <[email protected]>
shree007 pushed a commit to shree007/opentelemetry-operator that referenced this issue Dec 12, 2021
…orts a health_check. (open-telemetry#574)

* Add liveness probe to created container if otelcol configuration supports a health_check.
Fixes open-telemetry#571

Signed-off-by: Adrian Kostrubiak <[email protected]>
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this issue May 1, 2024
…orts a health_check. (open-telemetry#574)

* Add liveness probe to created container if otelcol configuration supports a health_check.
Fixes open-telemetry#571

Signed-off-by: Adrian Kostrubiak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants