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

Clarify RBAC in TargetAllocator readme. Issue ref: #2734 #2739

Merged
merged 8 commits into from
Mar 11, 2024

Conversation

avillela
Copy link
Contributor

@avillela avillela commented Mar 8, 2024

Description:

Updated TargetAllocator README to clarify RBAC.

Link to tracking Issue(s):

Testing:

n/a - no Operator code was modified

Documentation:

Updated TargetAllocator README

@avillela avillela requested a review from a team March 8, 2024 17:36
@avillela avillela requested a review from a team March 8, 2024 17:41
Copy link
Contributor

@matej-g matej-g left a comment

Choose a reason for hiding this comment

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

Nice work, thanks for the enhancement 🙇

cmd/otel-allocator/README.md Show resolved Hide resolved
.chloggen/avillela-2734.yaml Outdated Show resolved Hide resolved
Copy link
Member

@pavolloffay pavolloffay left a comment

Choose a reason for hiding this comment

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

LGTM,

I would recommend removing the changelog entry before merging

@avillela
Copy link
Contributor Author

avillela commented Mar 11, 2024

LGTM,

I would recommend removing the changelog entry before merging

@pavolloffay The CI process erred out when I left it out initially (see screen shot below), which is why I put it in. Is there a way to bypass that?

Screenshot 2024-03-11 at 13 39 41

@pavolloffay pavolloffay added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Mar 11, 2024
@pavolloffay
Copy link
Member

@pavolloffay The CI process erred out when I left it out initially, which is why I put it in. Is there a way to bypass that?

It's expected, maintainers are adding skip changelog label for PRs that don't need a changelog

@avillela
Copy link
Contributor Author

@pavolloffay The CI process erred out when I left it out initially, which is why I put it in. Is there a way to bypass that?

It's expected, maintainers are adding skip changelog label for PRs that don't need a changelog

Ah! TIL 😄

Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

two small changes, otherwise this looks great!

cmd/otel-allocator/README.md Outdated Show resolved Hide resolved
cmd/otel-allocator/README.md Outdated Show resolved Hide resolved
avillela and others added 2 commits March 11, 2024 15:28
Co-authored-by: Jacob Aronoff <jaronoff97@users.noreply.github.com>
Co-authored-by: Jacob Aronoff <jaronoff97@users.noreply.github.com>
@jaronoff97 jaronoff97 merged commit be172d9 into open-telemetry:main Mar 11, 2024
31 checks passed
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
open-telemetry#2739)

* Clarify RBAC in TargetAllocator readme. open-telemetry#2734

* Add changelog

* Add links to PodMonitor and ServiceMonitor APIs

* Add supported deployment modes

* Got the PrometheusCR roles and base TA roles mixed up - fixed it now.

* Remove changelog - not needed for readme update

* Update cmd/otel-allocator/README.md

Co-authored-by: Jacob Aronoff <jaronoff97@users.noreply.github.com>

* Update cmd/otel-allocator/README.md

Co-authored-by: Jacob Aronoff <jaronoff97@users.noreply.github.com>

---------

Co-authored-by: Jacob Aronoff <jaronoff97@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify which RBAC permissions are needed for different Target Allocator features
6 participants