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.

Sign up for GitHub

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

Jump to bottom

[prometheus] add prometheus RBAC roles #400

Merged
merged 1 commit into from
Feb 4, 2020
Merged

[prometheus] add prometheus RBAC roles #400

merged 1 commit into from
Feb 4, 2020

Conversation

jr0d
Copy link
Contributor

@jr0d jr0d commented Feb 4, 2020

No description provided.

@jr0d jr0d requested a review from a team February 4, 2020 20:21
@jr0d jr0d force-pushed the jr/prometheus-ingress-roles branch from 8a14e83 to 7d29f7b Compare February 4, 2020 20:22
alejandroEsc
alejandroEsc reviewed Feb 4, 2020
View reviewed changes
Copy link
Contributor

@alejandroEsc alejandroEsc left a comment

Choose a reason for hiding this comment

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

Could you test putting these resources together in another subdirectory mesosphere-rbac? This way we can trace the changes easier and manage updates to the chart later.

@jr0d jr0d force-pushed the jr/prometheus-ingress-roles branch from 6963ea1 to 8e6308b Compare February 4, 2020 21:38
@jr0d
Copy link
Contributor Author

jr0d commented Feb 4, 2020

@alejandroEsc done

@gracedo
Copy link
Contributor

gracedo commented Feb 4, 2020

Along the same vein as @alejandroEsc's suggestion, could we move ingressRBAC to under mesosphereResources in values.yaml?

@jr0d jr0d force-pushed the jr/prometheus-ingress-roles branch from 8e6308b to 757b628 Compare February 4, 2020 21:54
@jr0d
Copy link
Contributor Author

jr0d commented Feb 4, 2020

@gracedo done

@jr0d jr0d mentioned this pull request Feb 4, 2020
Merged
5 tasks
@alejandroEsc
Copy link
Contributor

@jr0d did you push up the changes?

@jr0d
Copy link
Contributor Author

jr0d commented Feb 4, 2020

@alejandroEsc yes, I opted to put them in the ingress-rbac directory as mesosphere is a bit out of date.

@jr0d jr0d added the ready ready label Feb 4, 2020
GoelDeepak
GoelDeepak approved these changes Feb 4, 2020
View reviewed changes
Copy link
Contributor

@GoelDeepak GoelDeepak left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm. I like the suggestion of putting the changes in a sub-directory.

@jr0d jr0d merged commit fd59bed into master Feb 4, 2020
@jr0d jr0d deleted the jr/prometheus-ingress-roles branch February 4, 2020 22:48
mesosphere-teamcity pushed a commit that referenced this pull request Feb 4, 2020
Merge pull request #400 from mesosphere/jr/prometheus-ingress-roles […
3538290 
…prometheus] add prometheus RBAC roles
@alejandroEsc
Copy link
Contributor

Ok, that sounds good, thanks @jr0d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alejandroEsc alejandroEsc alejandroEsc left review comments

@gracedo gracedo gracedo approved these changes

@GoelDeepak GoelDeepak GoelDeepak approved these changes

Assignees

@jr0d jr0d

Labels
ready ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jr0d @gracedo @alejandroEsc @GoelDeepak