IBM / core-dump-handler

Save core dumps from a Kubernetes Service or RedHat OpenShift to an S3 protocol compatible object store
https://ibm.github.io/core-dump-handler/
MIT License
136 stars 40 forks source link

optional pod selector labels #98

Closed tanmaykm closed 2 years ago

tanmaykm commented 2 years ago

This allows enabling core-dump-handler only on pods that match a configured selector pod-label. Useful if we need core-dump-handler only for a subset of our workload. The composer checks for the pod label and only prepares the upload only if the pod matches the selector configured. The default (empty selector label) is to have no selectors and enable collection for all. Only the existence of the label is checked for.

The selector can be configured via daemonSet.podSelectorLabel in values.yaml, which translates to the POD_SELECTOR_LABEL environment variable in the daemonset.

No9 commented 2 years ago

Thanks for this as well. In principle this is a great feature but I need to map the specifics to the cloudevent work going on in this branch. https://github.com/IBM/core-dump-handler/tree/cloud-event I want to keep the labeling for the different features consistent. It might be able to land 'as-is' or it may make sense to define a specific label. I'm busy today but I'll provide guidance before the weekend. If you could apply the rustfmt fix I'd appreciate it.

tanmaykm commented 2 years ago

Thanks for looking into it! Added https://github.com/IBM/core-dump-handler/pull/98/commits/89010c64a384ad02bad178247b9ce493595ee625 to fix ci and rustfmt

No9 commented 2 years ago

The good news is that this doesn't interfere with the cloud events.

The bad news is that the pod selector config mapping isn't quite in the right place.

All config that is passed to the composer should be in the composer section https://github.com/IBM/core-dump-handler/blob/main/charts/core-dump-handler/values.yaml#L23 The values should map to an environment variable prefixed with COMP_ in the agent.

Could you add an example of a label value in the Environment config https://github.com/IBM/core-dump-handler/pull/98/files#diff-35330df803a4bbfce6b8b734490d5f0b77bb117755aeac1958500a4934a86bb8R217

The values section in the docs also needs to be updated along with the environment section. https://github.com/IBM/core-dump-handler/pull/98/files#diff-35330df803a4bbfce6b8b734490d5f0b77bb117755aeac1958500a4934a86bb8R248

I've gone through the PR to also highlight where I think these changes need to be done. Hope it isn't too noisy.

(Edit) Once the mapping is aligned I'm happy to merge and push a release.

tanmaykm commented 2 years ago

I have now added https://github.com/IBM/core-dump-handler/pull/98/commits/1186de20508697288992a674d161fec711966995 which hopefully addresses these. Thanks for the guidance!

No9 commented 2 years ago

Looks good - I have some dep updates to land then I'll put together a release