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

Avoid allocating when performing VisitRulesFor on service accounts #75264

Merged
merged 1 commit into from Mar 21, 2019

Conversation

@smarterclayton
Copy link
Contributor

commented Mar 11, 2019

Service account authorization checks are done frequently and were
observed to perform 7% of allocations on a system running e2e tests.
The allocation comes from when we walk the authorization rules to
find matching service accounts.

Optimize the check for service account names to avoid allocating.

Allocations over about 5m of a 1.12 server running e2es.

image

4892626	5.21%	5.21%	4892626	5.21%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/authentication/serviceaccount.MakeUsername	(inline)
0	0.00%	5.21%	4869687	5.19%	net/http.HandlerFunc.ServeHTTP	
0	0.00%	5.21%	4892626	5.21%	github.com/openshift/origin/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac.(*RBACAuthorizer).Authorize	
0	0.00%	5.21%	4892626	5.21%	github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/registry/rbac/validation.appliesToUser	
0	0.00%	5.21%	4892626	5.21%	github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/registry/rbac/validation.appliesTo	
0	0.00%	5.21%	4892626	5.21%	github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/registry/rbac/validation.(*DefaultRuleResolver).VisitRulesFor	
0	0.00%	5.21%	4869687	5.19%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/server/filters.WithMaxInFlightLimit.func1	
0	0.00%	5.21%	4869687	5.19%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/server/filters.WithCORS.func1	
0	0.00%	5.21%	4861494	5.18%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/server/filters.(*timeoutHandler).ServeHTTP.func1	
0	0.00%	5.21%	4869687	5.19%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithImpersonation.func1	
0	0.00%	5.21%	4869687	5.19%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithAuthorization.func1	
0	0.00%	5.21%	4869687	5.19%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithAuthentication.func1	
0	0.00%	5.21%	4869687	5.19%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithAudit.func1	
0	0.00%	5.21%	4892626	5.21%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/authorization/union.unionAuthzHandler.Authorize	
0	0.00%	5.21%	4892626	5.21%	github.com/openshift/origin/vendor/k8s.io/apiserver/pkg/authorization/union.(*unionAuthzHandler).Authorize	
0	0.00%	5.21%	4892626	5.21%	github.com/openshift/origin/pkg/authorization/authorizer/browsersafe.(*browserSafeAuthorizer).Authorize	

/kind bug

NONE
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested review from jianhuiz and mbohlool Mar 11, 2019

smarterclayton added a commit to smarterclayton/origin that referenced this pull request Mar 11, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes/kubernetes#75264
@mfojtik

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2019

/lgtm

@fejta-bot

This comment has been minimized.

Copy link

commented Mar 11, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Mar 11, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79
@mbohlool

This comment has been minimized.

Copy link
Member

commented Mar 11, 2019

/assign @yliaog

openshift-publish-robot pushed a commit to openshift/kubernetes-apiserver that referenced this pull request Mar 11, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes/kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79

Kubernetes-commit: 09c604de63251a4e0cfbba48666a9bc3521281d0
@fejta-bot

This comment has been minimized.

Copy link

commented Mar 11, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot

This comment has been minimized.

Copy link

commented Mar 12, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@yliaog

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

Looks good, what are the resulting improvement with this optimization? how much allocations saved?

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

All of the allocations.

Avoid allocating when performing VisitRulesFor on service accounts
Service account authorization checks are done frequently and were
observed to perform 7% of allocations on a system running e2e tests.
The allocation comes from when we walk the authorization rules to
find matching service accounts.

Optimize the check for service account names to avoid allocating.

@smarterclayton smarterclayton force-pushed the smarterclayton:optimize_rbac_visit branch from dca3839 to 4c87a14 Mar 13, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 13, 2019

deads2k added a commit to deads2k/kubernetes that referenced this pull request Mar 14, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79

deads2k added a commit to deads2k/kubernetes that referenced this pull request Mar 14, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79

openshift-publish-robot pushed a commit to openshift/kubernetes-apiserver that referenced this pull request Mar 15, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes/kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79

Kubernetes-commit: 6f004cdba85ed63979cceadade5e393f1e52ebc2
@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

/refresh

@liggitt

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

/lgtm

@fejta-bot

This comment has been minimized.

Copy link

commented Mar 21, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit ed4258e into kubernetes:master Mar 21, 2019

17 checks passed

cla/linuxfoundation smarterclayton authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

openshift-publish-robot pushed a commit to openshift/kubernetes-apiserver that referenced this pull request Apr 3, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes/kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79

Kubernetes-commit: 17771bd0b97834077c70654d3213b0c4bfd3b1cd

soltysh pushed a commit to soltysh/kubernetes that referenced this pull request May 13, 2019

UPSTREAM: 75264: Optimize authorization service account check
Reduces total allocations about 5% in profiles, see kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.