kubectl 1.6.0-beta.1 is twice the size of kubectl-1.5.3 #42573
Comments
|
Looks like the Between 1.6.0-beta.0 and 1.6.0-beta.1 we went from go1.7.4 to go1.7.5. |
|
OK, it is not due to the golang version bump: $ _output/bin/kubectl version --client
Client Version: version.Info{Major:"1", Minor:"7+", GitVersion:"v1.7.0-alpha.0.936+d6ae61c3c9b102", GitCommit:"d6ae61c3c9b10265f29ea9963e76b90768e4739d", GitTreeState:"clean", BuildDate:"2017-03-08T00:08:30Z", GoVersion:"go1.7.4", Compiler:"gc", Platform:"linux/amd64"}
$ du -h _output/bin/kubectl
114M _output/bin/kubectl |
|
Happened between https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-build-1.6/3?log#log and https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-build-1.6/4?log#log: So something in b877536...30dd660. |
|
Search complete. New smaller range: c713ef4...cdf0cae |
|
c713ef4...cdf0cae#diff-095754adfdcb15cae5584c79c20510e0 massively increases the import tree of the apiserver |
|
Pretty sure #41906 is to blame. The artifacts from a PR test build on that PR confirm: @gnufied @rootfs @saad-ali @smarterclayton is this expected? |
|
@kubernetes/release-team fyi |
|
Yikes, that is a serious problem. pkg/api/validation should *not* import
every volume plugin implementation.
We need to either find a way to snip the dependencies or revert mount
options.
This is a pretty serious regression in size, not only a layering violation.
On Mar 7, 2017, at 8:05 PM, Jeff Grafton <notifications@github.com> wrote:
@kubernetes/release-team
<https://github.com/orgs/kubernetes/teams/release-team> fyi
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#42573 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p-qfgN6Hj2OdLlrXDm-adFOul8H1ks5rjf6mgaJpZM4MUNLX>
.
|
smarterclayton
added
the
priority/important-soon
|
I am looking at options what we can do. The reason volume plugins are getting imported in api server is because - we wanted to do a creation time validation of PVs and whether underlying volume plugin supports mount options (there are many that don't). |
|
If you need to do that, create a sub package with just that code, no
external dependencies.
We really can't have "dynamic" plugins in the API code, it's a locked API.
On Mar 7, 2017, at 10:02 PM, Hemant Kumar <notifications@github.com> wrote:
I am looking at options what we can do. The reason volume plugins are
getting imported in api server is because - we wanted to do a creation time
validation of PVs and whether underlying volume plugin supports mount
options (there are many that don't).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#42573 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p4wMYOh-k_TquYpt3vDnKZWgzFhTks5rjhpQgaJpZM4MUNLX>
.
|
|
There are couple of options I am thinking:
I am thinking I will go with option#1 and open a follow up PR. |
|
@smarterclayton I still don't quite get what you mean by we can't have "dynamic" plugins in the API. These volume plugins aren't dynamic and they don't change API interface in anyway. The mount option is merely an annotation .The only thing they can "potentially" change is - whether a PV type will accept mount options or not. Having said that, I didn't expect kubectl to import |
|
Splitting your validation into separate packages is what I would recommend.
Validation happens on the server, not on the client. The client validation
is a swagger schema validation, which is based on the server side document
generated by swagger. We will eventually break client validations away,
but it's tied into other changes.
I consider this a release-blocker at this point since this is a significant
regression for all client libraries picking up pkg/api.
…On Tue, Mar 7, 2017 at 10:38 PM, Hemant Kumar ***@***.***> wrote:
@smarterclayton <https://github.com/smarterclayton> I still don't quite
get what you mean by we can't have "dynamic" plugins in the API. These
volume plugins aren't dynamic and they don't change API interface in
anyway. The mount option is merely an annotation .The only thing they can
"potentially" change is - whether a PV type will accept mount options or
not.
Having said that, I didn't expect kubectl to import api/validation. :(
I looked at decoupling validations required in CLI and validation required
in server side. They seem to be tangled and any such refactoring will
require larger work.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#42573 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_pyWdUadVyH3gZVRVRpe45mucOlbqks5rjiLTgaJpZM4MUNLX>
.
|
|
@gnufied Let's drop creation time validation of mount options for PVs for this (1.6) release, that'll get rid of the dependencies in API server, and open an issue to reimplement them without importing the full dependency tree in v1.7 (as @smarterclayton suggested, maybe multiple packages, we'll need to think about it). |
|
Added your PR to the 1.6 milestone @gnufied. cc: @ethernetdan, @kubernetes/release-team, @kubernetes/kubernetes-release-managers |
|
@calebamiles thank you. Just to make it clearer, I have opened two PRs to fix this problem.
Spoke with @saad-ali about this. My preference would be to merge #42750 since this way we still get to keep the validation. Waiting for approval from @smarterclayton @justinsb @liggitt |
Automatic merge from submit-queue (batch tested with PRs 42811, 42859) Validation PVs for mount options We are going to move the validation in its own package and we will be calling validation for individual volume types as needed. Fixes #42573
|
We ended up not merging both of above PRs, but we merged third PR I made for fixing same problem - #42811 |
Comparing sizes indicates the size has gone from 49MB => 114MB. Did we change the compilation options? Do the betas ship with more debug options enabled?
The text was updated successfully, but these errors were encountered: