Client version matching server version doesn't seem like it captures conformance appropriately

[smarterclayton]

https://github.com/cncf/k8s-conformance/blob/master/reviewing.md

says

Look at version.txt. Make sure the client and server versions match each other to the minor version level. Make sure this also matches the vX.Y subdirectory that the PR is in.

That won't be possible if the client is in a container and is produced at different intervals (using sonobuoy 1.7.3 for example). And last I saw we were recommending release-1.8 as the conformance target?

Can we clarify what version.txt is supposed to be in instructions?

[mml]

@smarterclayton not sure I follow the question. version.txt is generated by code added in heptio/kube-conformance#6. I am already working on upstreaming that so the test itself generates this data (kubernetes/kubernetes#53551).

It is possible we need to cut new version of 1.7 and 1.8 kube-conformance docker images. @timothysc would be able to do that.

In the absence of those, you can build the images yourself from kube-conformance sources, or I have some images I built for 1.7 and for 1.8.

[timothysc]

We are planning a 0.9.0 release to match with 1.8.0/1.8.1 ... and will rev appropriately.

@mml Could you poke about getting your e2e patch into 1.8.1 please?

[smarterclayton]

The problem with version is that it doesn't identify the vendor version for any distribution that has a commit history that is not in kubernetes/kubernetes. If we think version.txt is important it needs to capture more than what client/server versions return

[smarterclayton]

So in general I don't think it should be a requirement for the kubectl client to match the server. The e2e tests expect a particular kubectl version, and in practice the conformance tests are validating that the server (i.e. the conformant kubernetes implementation) matches a set of expected API and runtime behavior. I think kubectl in this case is part of the test suite, not part of the distribution. The 1.8 e2e tests run against a 1.7 kubectl and 1.7 server make less sense than verifying the 1.8 e2e tests and the 1.8 tests run correctly against a 1.7 server.

[mml]

@smarterclayton I see your point now. Perhaps it's most important that the client version part of version.txt match the directory you're submitting to. I think if we update the instructions, we're keeping the spirit of the check and unblocking you. Correct?

[smarterclayton]

Earlier the recommendation for 1.7 was to use the 1.8 suite (or at least, that was the most recent recommendation discussed in the thread). I'd slightly prefer to run 1.8 against a 1.7 server since there are more tests, but I can change to use the 1.7 test suite if necessary.

…

On Fri, Oct 13, 2017 at 4:28 PM, Matt Liggett ***@***.***> wrote: @smarterclayton <https://github.com/smarterclayton> I see your point now. Perhaps it's most important that the client version part of version.txt match *the directory you're submitting to*. I think if we update the instructions, we're keeping the spirit of the check and unblocking you. Correct? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_pwGk0jExmSIi1y7VA3bIXPT3icYZks5sr8fQgaJpZM4PuTRI> .

[mml]

For now the instructions specify that the tests should match. The reason for that is that we think the definition of "Kubernetes 1.7" is codified on the 1.7 branch. You could imagine a future where Kubernetes N.{M+1} is the first version without behavior X, and so running those tests doesn't prove that the cluster is N.M.

If we need to discuss versioning further, happy to do so. Maybe on the mailing list?

[mml]

(Oh, and I will send a PR to fix the instructions.)

[smarterclayton]

For now the instructions specify that the tests should match. The reason for that is that we think the definition of "Kubernetes 1.7" is codified on the 1.7 branch. You could imagine a future where Kubernetes N.{M+1} is the first version without behavior X, and so running those tests doesn't prove that the cluster is N.M.

We had a discussion (EDIT: somewhere, can't find it) where we said to use 1.8 against 1.7. However, if the fixes have been delivered to correct the issues in the 1.7 tests then I can update it again and run it.

[aaronlevy]

I'm also a little confused by this - why would tests @ v1.7.3 be the canonical version for all v1.7 clusters? https://github.com/cncf/k8s-conformance/blob/master/sonobuoy-conformance-1.7.yaml#L95

Shouldn't it either be highest patch version available, or at least same version that you're deploying / testing conformance? (I've never been aware of discussions to use next minor release)

[smarterclayton]

I updated to build against release-1.7 kubectl and e2e and this is my version output:

Client Version: version.Info{Major:"1", Minor:"7+", GitVersion:"v1.7.9-beta.0.30+0a69594976467a", GitCommit:"0a69594976467a56597ad805585b1947d8b4cec0", GitTreeState:"clean", BuildDate:"2017-10-17T15:46:49Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"7", GitVersion:"v1.7.6+a08f5eeb62", GitCommit:"c84beff", GitTreeState:"clean", BuildDate:"2017-10-17T15:10:20Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}
-----
oc v3.7.0-alpha.1+b507a7f-1152
kubernetes v1.7.6+a08f5eeb62
features: Basic-Auth GSSAPI Kerberos SPNEGO

Server https://internal-api.prtest-060e08d-10.origin-ci-int-gce.dev.rhcloud.com:8443
openshift v3.7.0-alpha.1+b507a7f-1152
kubernetes v1.7.6+a08f5eeb62

This is using the release-1.7 branch kubectl, paired with the release-1.7 branch e2e tests, against OpenShift with Kube 1.7.6+patches.

https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/16904/test_pull_request_origin_extended_conformance_k8s/10/

[smarterclayton]

Does this satisfy the version constraints as written?

[dankohn]

@smarterclayton Please note that I'm also happy for you to suggest revisions in the reviewing directions: #9 (comment)

[timothysc]

My personal opinion is that we can tolerate +1 on the minor and it should be ok. There were was conflict on #24

[dankohn]

This was resolved in 19212f4#diff-832893d512e4929e92c510d2d3bdb772