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

Allow running daemonset in hostNetwork mode #393

Merged
merged 2 commits into from
Aug 19, 2021

Conversation

owais
Copy link
Contributor

@owais owais commented Aug 13, 2021

No description provided.

@owais owais force-pushed the allow-running-in-host-network branch 5 times, most recently from bde5c7f to 7af3df9 Compare August 13, 2021 00:57
@owais owais marked this pull request as ready for review August 13, 2021 00:58
@owais owais requested review from a team and tigrannajaryan August 13, 2021 00:58
@owais
Copy link
Contributor Author

owais commented Aug 13, 2021

I believe this is all that is required to enable the feature so submitting for review but haven't tested locally in a real cluster yet. Will do that later today in a few hours and report back.

dmitryax
dmitryax previously approved these changes Aug 13, 2021
@owais owais force-pushed the allow-running-in-host-network branch from 7af3df9 to de358ff Compare August 13, 2021 13:54
@mergify mergify bot dismissed dmitryax’s stale review August 13, 2021 13:54

Pull request has been modified.

@owais
Copy link
Contributor Author

owais commented Aug 13, 2021

Verified on a local cluster. Good to go.

dmitryax
dmitryax previously approved these changes Aug 13, 2021
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an e2e test for this case? I know that some platforms like OpenShift restrict options like these, and an e2e test would be useful to validate that.

@mergify mergify bot dismissed dmitryax’s stale review August 17, 2021 10:55

Pull request has been modified.

@owais owais force-pushed the allow-running-in-host-network branch from 515eb03 to 0099444 Compare August 17, 2021 11:00
@owais owais force-pushed the allow-running-in-host-network branch from 0099444 to 4270cce Compare August 17, 2021 11:01
@owais owais force-pushed the allow-running-in-host-network branch from 4270cce to d8ecbfd Compare August 17, 2021 11:19
@owais
Copy link
Contributor Author

owais commented Aug 17, 2021

@jpkrohling added e2e test for daemonset+hostNetwork

@owais owais requested a review from jpkrohling August 17, 2021 11:20
@VineethReddy02
Copy link
Contributor

VineethReddy02 commented Aug 17, 2021

LGTM!

Waiting for the tests to pass...
I see e2e tests for 1.22 are failing at

        --- FAIL: kuttl/harness/targetallocator-features (35.95s)
        --- FAIL: kuttl/harness/smoke-targetallocator (35.99s)

@owais
Copy link
Contributor Author

owais commented Aug 17, 2021

@VineethReddy02 yeah these seem to be flaky tests. They fail randomly locally as well as on CI. I'm re-running the CI. Hopefully should be green in a few attempts.

@owais
Copy link
Contributor Author

owais commented Aug 17, 2021

All green now @VineethReddy02 Thanks for the review.

@owais owais requested a review from dmitryax August 18, 2021 17:50
@jpkrohling jpkrohling merged commit 2f4fe38 into open-telemetry:main Aug 19, 2021
@jpkrohling jpkrohling added this to the v0.33.0 milestone Aug 19, 2021
@owais owais deleted the allow-running-in-host-network branch August 19, 2021 12:15
shree007 pushed a commit to shree007/opentelemetry-operator that referenced this pull request Dec 12, 2021
* Allow running daemonset in hostNetwork mode

* Added daemonset e2e tests
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* Allow running daemonset in hostNetwork mode

* Added daemonset e2e tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants