open-telemetry / opentelemetry-helm-charts

OpenTelemetry Helm Charts
https://opentelemetry.io
Apache License 2.0
395 stars 485 forks source link

imagePullSecrets on the collector image #696

Open pseymournutanix opened 1 year ago

pseymournutanix commented 1 year ago

Hello,

The helm chart now allows imagePullSecrets for the operator image, but this doesn't fall into the collector (unless I am missing something).

Perhaps it would be easier to add them to the service accounts if set in .Values.imagePullSecrets

Allex1 commented 1 year ago

@pseymournutanix That's a good point and we are aware of it. As we don't actually deploy the cr as part of this chart we should at least update the examples. Can you help with that? Thanks

povilasv commented 1 year ago

Perhaps it would be easier to add them to the service accounts if set in .Values.imagePullSecrets

Can we actually do this?

I think Operator creates a new ServiceAccount for every Collector resource? So it's a bit outside of our control, no?

Based on this -> https://github.com/open-telemetry/opentelemetry-operator/blob/main/docs/api.md#opentelemetrycollectorspec

image

pseymournutanix commented 1 year ago

Yes it appears to be the case that a new SA is created. Rather makes the use of private registeries impossible.

povilasv commented 1 year ago

I think you could set serviceAccount on each Collector CR and then make that serviceAccount have a imagePullSecret.

So it is possible, but outside of scope of this chart, no?

sreejesh-radhakrishnan-db commented 1 year ago

I am following https://opentelemetry.io/docs/k8s-operator/automatic/#create-an-opentelemetry-collector-optional and my deployment is failing for same reason. as its private reg for image.

I can see deployment this is the service account used., are you saying we should add this serviceAccount with a imagePullSecret ?

   serviceAccount: care-optl-collector
  serviceAccountName: care-optl-collector
sreejesh-radhakrishnan-db commented 1 year ago

@pseymournutanix did you make it work? please give some pointers if the case.

TylerHelmuth commented 1 year ago

The OpenTelemetryCollector object does let you specify a image pull policy. Using https://opentelemetry.io/docs/k8s-operator/automatic/#create-an-opentelemetry-collector-optional as the starting point, I believe you can do

kubectl apply -f - <<EOF
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: demo
spec:
  imagePullPolicy: THE_VALUE_YOU_WANT
  config: |
    receivers:
      otlp:
        protocols:
          grpc:
          http:
    processors:
      memory_limiter:
        check_interval: 1s
        limit_percentage: 75
        spike_limit_percentage: 15
      batch:
        send_batch_size: 10000
        timeout: 10s

    exporters:
      logging:

    service:
      pipelines:
        traces:
          receivers: [otlp]
          processors: [memory_limiter, batch]
          exporters: [logging]
        metrics:
          receivers: [otlp]
          processors: [memory_limiter, batch]
          exporters: [logging]
        logs:
          receivers: [otlp]
          processors: [memory_limiter, batch]
          exporters: [logging]
EOF

And it will use that value in the deployment it makes for the collector.

Since this object is created outside the opentelemetry-operator chart the chart cannot be used to set this value. I believe this is another reason to look at https://github.com/open-telemetry/opentelemetry-helm-charts/issues/562 and https://github.com/open-telemetry/opentelemetry-helm-charts/issues/69

sreejesh-radhakrishnan-db commented 1 year ago

sorry the issue is with imagePullSecret not present on the deployment and not imagePullPolicy? how will having imagePullPolicy in this way help solve the issue if secret not present in deployment and hence cannot pull image from private reg?

did you mean to say imagePullSecrets?

sreejesh-radhakrishnan-db commented 1 year ago

Just to make anyone trying to do what I have been doing,

I did use imagePulllSecrets in the spec, and has to use --validate=false for k8s apply not to fail rather throw warning

Warning: unknown field "spec.imagePullSecrets"

and Deployment came up fine.

TylerHelmuth commented 1 year ago

Oh sorry, I mixed up the fields. It looks like the OpenTelemetryCollector custom resource does not accept imagePullSecrets :(

The stuff about the operator chart not being able to accomplish this is still accurate. Based on https://github.com/open-telemetry/opentelemetry-operator/issues/846 the current solution is to create and manage your own service account that has the imagePullSecrets set, and then use the name of that service account in .Spec. serviceAccount on the OpenTelemetryCollector CR.

That all kinda complicated, so I think it is worth commenting on that issue and asking that imagePullSecrets also be available on the CR directly.

seb-835 commented 6 months ago

Hi there some plan to add the imagePullSecrets on the CR directly ? or do we have to consider to always put it in the ServiceAccount ?

Saki042 commented 5 months ago

@TylerHelmuth , is this issue still open? Can I get more info about it?

TylerHelmuth commented 5 months ago

The desired state would be for the setting to be exposed on the OpenTelemetryCollector CR, so a change needs made in the otel operator repo

Saki042 commented 5 months ago

@TylerHelmuth , I am working on the Open source for the first time, might require more context. Can you confirm this is the repo link: https://github.com/open-telemetry/opentelemetry-operator