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

Allow HPA to get custom metrics #54854

Merged
merged 1 commit into from
Nov 8, 2017
Merged

Allow HPA to get custom metrics #54854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
rbac.NewRule("get").Groups(legacyGroup).Resources("services/proxy").Names("https:heapster:", "http:heapster:").RuleOrDie(),
// allow listing resource metrics and custom metrics
rbac.NewRule("list").Groups(resMetricsGroup).Resources("pods").RuleOrDie(),
Copy link
Member

Choose a reason for hiding this comment

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

get needed here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is not necessary - HPA uses this API since PR #53205 and works fine, i.e. e2e tests are passing. See [HPA] tests here: https://k8s-testgrid.appspot.com/google-gce#gci-gce

rbac.NewRule("list").Groups(customMetricsGroup).Resources("*").RuleOrDie(),
rbac.NewRule("get", "list").Groups(customMetricsGroup).Resources("*").RuleOrDie(),
Copy link
Member

Choose a reason for hiding this comment

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

FWIW "watch" seems to be missing here - while it was there in the original role (https://github.com/GoogleCloudPlatform/k8s-stackdriver/pull/65/files)

Copy link
Member

Choose a reason for hiding this comment

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

no objection to the permission, but does the custom metrics server support watch?

Copy link
Member

Choose a reason for hiding this comment

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

Guess it's a question for @kawych :)

eventsRule(),
},
})
Expand Down
1 change: 1 addition & 0 deletions plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ items:
resources:
- '*'
verbs:
- get
- list
- apiGroups:
- ""
Expand Down