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

Add Bridge to opentelemetry-kube-stack #1138

Merged
merged 6 commits into from
Apr 19, 2024

Conversation

jaronoff97
Copy link
Contributor

Closes #1079

This PR adds the ability to create an opamp bridge as part of the chart. This creates the bridge and its clusterroles/bindings as well as convenience for adding the necessary labels when applicable for the collector.

@jaronoff97 jaronoff97 requested a review from a team April 12, 2024 21:50
@JaredTan95
Copy link
Member

Seeing this PR, I thought of a digression. As we all know, helm won't apply the crd in the chart crds directory when upgrading an chart. When we upgrade the chart version with the chart/crds directory changes, there will always be breaking changes or some additional crd apply. (Related issue: #985, open-telemetry/opentelemetry-operator#2314)

I see this and this PR both include crd content, will there be the same problem?

@jaronoff97
Copy link
Contributor Author

the way around this is to include crds in the templates directory, it's a hack but is kind of the only thing that can be done to resolve this effectively. You can see an example of me doing this to handle updates here though idk if that's how we want to do all of this. Either way, we should probably discuss this not in this PR but rather in an issue or in slack

@JaredTan95
Copy link
Member

Yes, I actually know that there will be no problem with this PR, but I think it is a little strange, maybe it is the hack way you said. But overall for me, lgtm.

@jaronoff97 jaronoff97 requested a review from JaredTan95 April 15, 2024 20:48
charts/opentelemetry-kube-stack/values.yaml Outdated Show resolved Hide resolved
charts/opentelemetry-kube-stack/values.yaml Outdated Show resolved Hide resolved
charts/opentelemetry-kube-stack/values.yaml Outdated Show resolved Hide resolved
charts/opentelemetry-kube-stack/values.yaml Outdated Show resolved Hide resolved
charts/opentelemetry-kube-stack/values.yaml Show resolved Hide resolved
charts/opentelemetry-kube-stack/values.yaml Outdated Show resolved Hide resolved
@jaronoff97 jaronoff97 requested a review from TylerHelmuth April 19, 2024 17:52
@TylerHelmuth TylerHelmuth merged commit 45f7def into open-telemetry:main Apr 19, 2024
3 checks passed
12ushan pushed a commit to giffgaff/opentelemetry-helm-charts that referenced this pull request Jul 22, 2024
* add bridge in

* match name of bridge

* updates from feedback

* envfromfirst

---------

Co-authored-by: Jared Tan <[email protected]>
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.

[opentelemetry-kube-stack] Enable OpAMP Bridge creation
3 participants