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

Copy desired Service ports when reconciling #299

Merged
merged 2 commits into from
May 27, 2021

Conversation

thib92
Copy link
Contributor

@thib92 thib92 commented May 27, 2021

Copy services from the desired service when reconciling.

For now it only copies the ports attribute. We might need to copy additional fields as well. Let me know!

Fixes #256
Fixes #295

@thib92 thib92 requested review from a team May 27, 2021 09:22
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 27, 2021

CLA Signed

The committers are authorized under a signed CLA.

jpkrohling
jpkrohling previously approved these changes May 27, 2021
@jpkrohling
Copy link
Member

Looks good to me: once you confirm that you also don't see any other fields that should be copied, I'll merge.

@mergify mergify bot dismissed jpkrohling’s stale review May 27, 2021 12:30

Pull request has been modified.

@thib92
Copy link
Contributor Author

thib92 commented May 27, 2021

The ServiceSpec does have its fair share of options, most of them I'm not too familiar with.

However, I think that using the OpenTelemetry Operator, only the ports field can be messed with (at least at the moment), so I think it's safe to keep it like that.

@jpkrohling jpkrohling merged commit d97d94a into open-telemetry:main May 27, 2021
@jpkrohling
Copy link
Member

Thank you for this PR!

@thib92 thib92 deleted the fix-port-reconcile branch May 27, 2021 12:41
shree007 pushed a commit to shree007/opentelemetry-operator that referenced this pull request Dec 12, 2021
* Copy desired Service ports when reconciling

* Fix update service test
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* Copy desired Service ports when reconciling

* Fix update service test
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.

OpenCensus port is not exposed by the Collector Service expectedServices function not copying desired spec
2 participants