Build OpenAPI Definitions per group instead of per resource

[jennybuckley]

What this PR does / why we need it:
Addresses inefficiencies in the implementation of #63893

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of fixing #66368
Not sure where the 40 seconds comes from in the referenced bug, but this PR reduced startup time of kube apiserver by ~20% on my computer

Release note:

NONE

[jennybuckley]

/sig api-machinery
/kind bug

[jennybuckley]

/retest

[fedebongio]

/assign @mbohlool

[mbohlool]

This is a copy from somewhere?

[jennybuckley]

Yes, this is copied from https://github.com/kubernetes/kube-openapi/blob/master/pkg/builder/openapi.go#L138-L151

[mbohlool]

can you reuse this please? these two copy can get out of sync. move them to somewhere common if necessary.

[jennybuckley]

I can, but it is in the kube-openapi repo, so it would mean changing it there first, and then vendoring it back in

[jennybuckley]

What if I just put a TODO for this, and also make the PR to kube-openapi repo

[mbohlool]

Adding todo and sending PR to kube-openapu repo looks good to me. Send a followup PR after the openapi PR got merged. Thanks.

[jennybuckley]

I just ran some pprof analysis on startup of a basic API server with no extensions, before and after this change.

Before this change, a total of ~16.7 million objects were allocated within the first 50 seconds of startup.
After this change, only ~12.0 million objects were allocated in the same time frame.

[smarterclayton]

Hrm, was hoping for a more dramatic reduction. Where were the bulk of the remaining allocations coming from under a cumulative sort?

[jennybuckley]

@smarterclayton About 5.5 million were coming from the openapi endpoint handler, which creates just one full openapi spec and serves it, and this change doesn't touch that code. I think the openapi spec might just take that many objects to create.

      flat  flat%   sum%        cum   cum%
         0     0%     0%    6332343 52.70%  k8s.io/kubernetes/cmd/kube-apiserver/app.CreateServerChain
         0     0%     0%    6154823 51.23%  k8s.io/kubernetes/cmd/kube-apiserver/app.Run
         0     0%     0%    5834452 48.56%  k8s.io/kubernetes/cmd/kube-apiserver/app.NewAPIServerCommand.func1
         0     0%     0%    5631494 46.87%  k8s.io/kubernetes/vendor/github.com/spf13/cobra.(*Command).execute
         0     0%     0%    5576641 46.41%  k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/handler.(*OpenAPIService).UpdateSpec
         0     0%     0%    5318267 44.26%  k8s.io/kubernetes/vendor/github.com/spf13/cobra.(*Command).ExecuteC
         0     0%     0%    5301052 44.12%  k8s.io/kubernetes/vendor/k8s.io/kube-openapi/pkg/handler.toProtoBinary
         0     0%     0%    5085712 42.33%  k8s.io/kubernetes/vendor/github.com/spf13/cobra.(*Command).Execute
         0     0%     0%    4835898 40.25%  main.main
         0     0%     0%    4733475 39.40%  runtime.main

[smarterclayton]

Ok, we can dig further after this. Can you grab a top30 and add it to the original issue so we know what’s next on the hitlist? Thanks On Sep 20, 2018, at 7:34 PM, Jenny Buckley <notifications@github.com> wrote: @smarterclayton <https://github.com/smarterclayton> About 5.5 million were coming from the openapi endpoint handler, which creates just one full openapi spec and serves it, and this change doesn't touch that code. I think the openapi spec might just take that many objects to create. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#68576 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p91UWxL-sFVjKJGLHlf79Euii8yFks5uc9GfgaJpZM4WmGPy> .

[jennybuckley]

Going to hold off on this until #69316 merges, since it also vendors in some unrelated kube-openapi changes

[jennybuckley]

Since #70260 merged, this doesn't need to vendor in kube-openapi anymore. Should be good to merge now, uses the exported function from kube-openapi
cc @mbohlool

[jennybuckley]

/retest

[mbohlool]

nit: you probably don't need this. it should work if you pass sampleObjectPtr.

[mbohlool]

only one nit comment. otherwise look good.

/lgtm

[jennybuckley]

Rebased

[lavalamp]

/lgtm
/approve
/milestone v1.13

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jennybuckley, lavalamp

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:

• staging/src/k8s.io/apiserver/OWNERS [lavalamp]

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

[fejta-bot]

/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 comment for consistent failures.

[apelisse]

/retest

[fejta-bot]

/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 comment for consistent failures.