Labelling this PR as size/S

GCE e2e build/test failed for commit 20032d42acbc11c50e323581fae2a996ee0ab943.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e build/test failed for commit 95c3004b3c17ac45162e26c738aff45251b15906.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit 95c3004b3c17ac45162e26c738aff45251b15906.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Will this make v1.2?

@luxas it's been unclear to me if anyone wants it...

@k8s-bot test this please, issue #IGNORE (Jenkins got stuck at some point?)

@lavalamp @liggitt @deads2k Can any of you think of a problem this would cause? It seems like the git hashes would mismatch any time the go versions differed.

wouldn't kubectl --match-server-version complain about a client built with a different go version than the server? that seems less than ideal

edit: read the description, which called that out. still think that's less than ideal.

GCE e2e build/test passed for commit 95c3004b3c17ac45162e26c738aff45251b15906.

• Build Log
• Test Artifacts
• Internal Jenkins Results

wouldn't kubectl --match-server-version complain about a client built with a different go version than the server? that seems less than ideal

An argument to fix MatchesServerVersion, not one to avoid adding this information. It's a little esoteric, but as long as MatchesServerVersion is updated to skip trying to match on go compiler level I don't see it as a problem.

Fortunately matchesServerVersion doesn't have to be backwards compatible

heh, true

I think the go version skew between client & server is a) hard to get in practice b) worth noting when it happens. So I think this is probably net good. LGTM

So what's the consensus here? Do we want to update MatchesServerVersion to ignore the go version? Or is this fine as-is?

So what's the consensus here? Do we want to update MatchesServerVersion to ignore the go version? Or is this fine as-is?

I'd like to ignore the go version and base the decision on whether server and client were built from the same code.

I vote for it being fine as-is. I think client & server built with
different versions of go should be considered different. The only way you
can convince me otherwise is if cross-platform client/server combos would
trip over this.

On Thu, Mar 3, 2016 at 5:01 AM, David Eads notifications@github.com wrote:

So what's the consensus here? Do we want to update MatchesServerVersion to
ignore the go version? Or is this fine as-is?

I'd like to ignore the go version and base the decision on whether server
and client were built from the same code.

—
Reply to this email directly or view it on GitHub
#19905 (comment)
.

The way it is is certainly less work for me. :)

Otherwise I will probably need to clear out the client and server Go versions in MatchesServerVersion, since I'd prefer to keep using reflect.DeepEqual instead of enumerating all fields.

(It's also possible there's code elsewhere that compares versions, which would also need to account for Go versions? I can do another pass to see. It's kinda unfortunate that MatchesServerVersion is in pkg/client/unversioned/helper.go instead of pkg/version, where it'd probably be more appropriate, but that's also a more in-depth refactoring effort.)

If the platform isn't in the go version then I'm good with it the way it
is. @deads2k why do you think it will be problematic?

On Thu, Mar 3, 2016 at 4:17 PM, Jeff Grafton notifications@github.com
wrote:

The way it is is certainly less work for me. :)

Otherwise I will probably need to clear out the client and server Go
versions in MatchesServerVersion, since I'd prefer to keep using
reflect.DeepEqual instead of enumerating all fields.

(It's also possible there's code elsewhere that compares versions, which
would also need to account for Go versions? I can do another pass to see.
It's kinda unfortunate that MatchesServerVersion is in
pkg/client/unversioned/helper.go instead of pkg/version, where it'd
probably be more appropriate, but that's also a more in-depth refactoring
effort.)

—
Reply to this email directly or view it on GitHub
#19905 (comment)
.

@lavalamp what the version looks like now (from the most recent e2e run of this PR):

2016/01/21 20:13:42 e2e.go:303: Running: 'kubectl version --match-server-version=false'
Client Version: version.Info{Major:"1", Minor:"2+", GitVersion:"v1.2.0-alpha.6.361+832d81820d7497", GitCommit:"832d81820d749749409495d7536274ee76e706ce", GitTreeState:"clean", GoVersion:"go1.4.2"}
Server Version: version.Info{Major:"1", Minor:"2+", GitVersion:"v1.2.0-alpha.6.361+832d81820d7497", GitCommit:"832d81820d749749409495d7536274ee76e706ce", GitTreeState:"clean", GoVersion:"go1.4.2"}

So no platform info.

@k8s-bot test this please, issue #22435

It seems go1.5.3 doesn't like spaces in the ldflag value, so I switched to using the ISO8601 format for the build date.

GCE e2e build/test passed for commit 2baff5e7e4cf5a3e24b2f302f92fa3864573e229.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e build/test passed for commit 38e7d9bd30bcc789be93f5511c73afd0f61d9cd7.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@ixdy Please squash and maybe @lavalamp could LGTM so we could get this in.
Will be valuable to have merged

lgtm

apply the label once you squash.

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

Sorry for the noise. Interactive rebased a bit too far, but fixed now.

Thanks for review.

@k8s-bot ok to test
@k8s-bot test this

pr builder appears to be missing, activating due to 'lgtm' label.

GCE e2e build/test passed for commit dda45ab030cde5d1a9b3c6c1da1923cf733c2703.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e build/test passed for commit fb663f2.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

GCE e2e build/test passed for commit fb663f2.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue