-
Notifications
You must be signed in to change notification settings - Fork 452
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
fix for #2366 #2385
fix for #2366 #2385
Conversation
I can review when I'm back on Monday. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix looks fine to me. Can we also set the lifecycle on the nginx Pod in the E2E test for this instrumentation? I think that would be here: https://github.com/open-telemetry/opentelemetry-operator/blob/main/tests/e2e-instrumentation/instrumentation-nginx-contnr-secctx/01-install-app.yaml.
Sure, I'm out till Monday, this was max I dared to do remote.
Dne čt 23. 11. 2023 11:50 uživatel Mikołaj Świątek ***@***.***>
napsal:
… ***@***.**** commented on this pull request.
The fix looks fine to me. Can we also set the lifecycle on the nginx Pod
in the E2E test for this instrumentation? I think that would be here:
https://github.com/open-telemetry/opentelemetry-operator/blob/main/tests/e2e-instrumentation/instrumentation-nginx-contnr-secctx/01-install-app.yaml
.
—
Reply to this email directly, view it on GitHub
<#2385 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABS4R65XT5NGIFNMVCTVLVDYF4TAXAVCNFSM6AAAAAA7WWJYCKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBWGE4DMMJYGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***
com>
|
I don't think we need a separate E2E test for this, I'd just like to have it set in one of the existing ones as a smoke detector. |
Thanks for the fix! Would it be possible to release a patch including this fix? |
It will be included in 0.90.0 release. We release the operator once OTEL collector is released. The release usually happens quite often. |
@pavolloffay would it be possible to release a new version now? |
This patch should have been released a long time ago |
Apologies, I was looking in the wrong place 🤦 Thanks guys for your work in fixing this 👍 |
* fix for open-telemetry#2366 * added cloggen * typo * typo * e2e test for lifecycle removal from init container * e2e test for lifecycle removal in init container
Description: Fix for
Resolves #2366 - removal of lifecycle on cloned initContainer
Link to tracking Issue: #2366
Testing: Added test case to verify lifecycle removal from clonedInit container
Documentation: N/A