adjust to newer k8s APIs

[sigma]

What does this PR achieve? Why do we need it?

This updates kubernetes dependencies in multiple ways:

• update client APIs to newer version (v8.0.0)
• remove the dependency to github.com/kubernetes/kubernetes (which is not meant to be vendored)

Background: we're considering managing Kubernetes with vgo, and heketi is one of the few obstacles to get there. See kubernetes/kubernetes#65683 for details about the problem, and kubernetes/kubernetes#66305 for an early attempt at using this patched version as part of kubernetes.

Notes for the reviewer

[centos-ci]

Can one of the admins verify this patch?

[humblec]

@sigma thanks for the patch. indeed we need to update the client to latest. The kube client ensure the backward compatibility. However we need to make sure old heketi-cli or client is compatible with the chanegs we are making in heketi server.

Did you get a chance to verify this at your end ?

[humblec]

@sigma there is a conflict on glide.lock which need to be solved.

[sigma]

@humblec no I haven't verified that.

Moreover I'm looking at it from a k8s-centric angle, and all the code changes are in dead code from that perspective, so it's a rather uninformed change on my side. I view it more as a conversation starter :)

[phlogistonjohn]

Background: we're considering managing Kubernetes with vgo, and heketi is one of the few obstacles to get there. See kubernetes/kubernetes#65683 for details about the problem, and kubernetes/kubernetes#66305 for an early attempt at using this patched version as part of kubernetes.

Ah, the joys of mutual dependencies. :-)

You've made me aware of vgo / go modules, which makes me quite happy to know is coming.

This is based on v7.0.0 to limit the number of moving pieces on my side for now.
I'm not a glusterfs user, and not entirely sure what kind of validation would be needed beyond "make test".

I think we'd need:

• Passing unit tests
• Passing functional tests
• A test running a heketi built with these dependencies works with the older version(s) of k8s.

The first two are automated as part of our PR process, but I think the last bullet needs manual testing. I think the test case would be:

• Deploy new heketi build in older k8s
• Verify that basic operations using the "kubeexec" executor works

I'm also not sure what k8s compatibility heketi targets, so I picked kubernetes/client-go v7.0.0 that's compatible with k8s 1.9 to 1.11, but that's a bit arbitrary.

I think this will depend heavily on what the downstream deployments are doing to be honest. We need to be cautious here I think. Do you happen to know what the oldest possible version we could support with changes like these is?

Moreover I'm looking at it from a k8s-centric angle, and all the code changes are in dead code from that perspective, so it's a rather uninformed change on my side. I view it more as a conversation starter :)

Understandable. However, I'm not 100% sure I understand what you mean by dead code. You mean that the in-tree provisioner is being deprecated correct?

One nit about the change so far, usually when other contributors make changes tickle glide and it goes and makes changes to dependencies that are not related to the task at hand we usually ask to have that part reverted to the current form. Would you mind doing that?

The unit test suite running under Travis failed to build on Go 1.7.5. Is this expected?

[sigma]

I think this will depend heavily on what the downstream deployments are doing to be honest. We need to be cautious here I think. Do you happen to know what the oldest possible version we could support with changes like these is?

I'm not exactly an expert on the compatibility expectations, but my understanding is that k8s client code (which is the gist of heketi's dependency to k8s) is backward- and forward-compatible with k8s server with a 1 minor version skew (which is why k8s client-go v7.0.0, which corresponds to kubernetes release 1.10.x would work with 1.9.x, 1.10.x and 1.11.x). So v7.0.0 is the oldest version of the client code that would work with the latest version of k8s. You could use prior versions of the client code, but compatibility with newer k8s releases wouldn't be guaranteed
(that's on paper, in reality the compatibility range might be wider but that's not supported)

Understandable. However, I'm not 100% sure I understand what you mean by dead code. You mean that the in-tree provisioner is being deprecated correct?

Oh no, what I mean is that from the perspective of k8s vendoring heketi, we need only a few packages (client/api/go-client, pkg/glusterfs/api and pkg/utils), none of which are affected by the changes required for heketi to vendor k8s.io/{client-go,api,apimachinery}

One nit about the change so far, usually when other contributors make changes tickle glide and it goes and makes changes to dependencies that are not related to the task at hand we usually ask to have that part reverted to the current form. Would you mind doing that?

sure I can try to do that.

The unit test suite running under Travis failed to build on Go 1.7.5. Is this expected?

Unfortunately it looks like k8s.io/apimachinery in (more) recent versions doesn't support Go 1.7.x indeed. If support for that Go version is important, I guess we have yet another problem :)

[sigma]

FYI I pushed an updated commit, that minimizes (I think, there are still a fair amount of pieces moving for good reasons) the changes in dependencies.

[obnoxxx]

@sigma , thanks for your patch!

Ideally, I'd want heketi master to always work with kubernetes master, and heketi release versions to be locked down to specific kubernetes versions. Not sure if that (master) is actually feasible, of if kubernetes master is too fast moving a target...

[sigma]

@obnoxxx so if that's the goal, I think consuming the last release of k8s client code should achieve that with relatively little effort since:

• the client code is supposed to be forward-compatible with the next version of the server
• it's not gonna as much of a moving target as client code at master

I'm happy to do that (basically moving the libraries to v8), but I'm still unable to validate beyond "make test". Any chance somebody familiar with glusterfs might want to partner with me on the validation side?

[phlogistonjohn]

I'm happy to do that (basically moving the libraries to v8), but I'm still unable to validate beyond "make test". Any chance somebody familiar with glusterfs might want to partner with me on the validation side?

Can you be a bit more specific about what challenges you're running into? Or is it a matter that you are unsure of what to do next? I'm open to helping test your changes.
(For quick questions you can also try hopping on #heketi on freenode irc and pinging us there )

[sigma]

@phlogistonjohn yeah, I'm utterly unfamiliar with glusterfs and heketi, it just so happens that the current state stands in the way of migrating k8s from godeps to vgo, which I'm trying to work my way towards.
As such, anything beyond running automated tests is out of my league :)
Thanks for your help !

[phlogistonjohn]

@sigma no problem. IMO, one of the most convenient ways to do some basic testing of this stack is gluster-kubernetes project which has a vagrant environment and basic test suite. It's how I got familiarized with this environment.

As a first step I'd get a stock environment going and try running the tests there. Then you should be able to do a container build of heketi (see the extras/docker/fromsource directory). Try changing the gluster-kubernetes templates to point at your test build of the heketi image and see if it able to still connect to the gluster pods over the k8s api. Ping us if you get stuck on or need additional details on any of the steps.

[sigma]

ok, I think I'm now in a valid state: I checked that the "complex" tests in gluster-kubernetes passed with my changes (and they definitely do test the kubeexec path, cause it failed miserably initially):

...
|=====
| TEST SUMMARY:
| PASS...: test-setup.sh
| PASS...: test-gk-deploy.sh:1: Test basic deployment
| PASS...: test-gk-deploy.sh:2: Test basic deployment idempotence
| PASS...: test-gk-deploy.sh
| PASS...: run-basic.sh:1
| PASS...: test-dynamic-provisioning.sh:1: Test dynamic provisioning
| PASS...: test-dynamic-provisioning.sh
| PASS...: run-basic.sh:2
| PASS...: run-basic.sh
| PASS...: test-teardown.sh

[phlogistonjohn]

ok, I think I'm now in a valid state: I checked that the "complex" tests in gluster-kubernetes passed with my changes (and they definitely do test the kubeexec path, cause it failed miserably initially):

Cool!
That's the right way to test, isn't it? Break it first and then fix it :-)

I'll enable the centos-ci to run on this pr and try to find time to give the changes a proper review soon.

[phlogistonjohn]

ok to test

[phlogistonjohn]

Looks like the Go 1.7.5 tests are failing with a legitimate compile error:

go build -i -ldflags "-X main.HEKETI_VERSION=v7.0.0-131-g5d6b450-HEAD -extldflags '-z relro -z now'" -o heketi
# github.com/heketi/heketi/vendor/k8s.io/apimachinery/pkg/util/net
vendor/k8s.io/apimachinery/pkg/util/net/http.go:283: req.URL.Hostname undefined (type *url.URL has no field or method Hostname)
make: *** [heketi] Error 2

I am of the opinion that we should drop support for Go 1.7 anyway, so I'm not too worried about it, but I don't think I want to drop the dependency until after we cut the next Heketi release. Which I want to be soonish. I have created PR #1326 to help kick off that part of the discussion.

[munnerz]

(driving by) now that #1326 has merged, can this be revisited? It seems like most of the testing/verification of this patch has already been done 😄

[sigma]

I rebased after #1373

This is pretty much the same change as before, only touching 2 files instead of 1 in pkg/remoteexec/kube now.

Oh and also using a newer version of kubernetes client libraries, because time flies :)

[sigma]

great, we have issues with go < 1.10 now :'(
I wish k8s libs didn't jump on all latest go features...

[sigma]

So I have a problem. k8s client libraries v8.0.0 are the last ones that build with the currently supported go versions.

On the other hand, kubernetes/api@2094d62 (in v10.0.0) changed the type of PersistentVolumeSource.Glusterfs, which means that if I use anything but v10, we won't be able to vendor the result in the context of k8s.io/kubernetes, which was the whole point of this pull request 😱

So at this point, unless I'm missing something trivial it's either go 1.8-1.10 compatibility, or k8s usability

@dims @phlogistonjohn any suggestions?

Note that it was brought up in kubernetes/kubernetes#60195 and was deemed WAI

[sigma]

Maybe it'd be worth exploring the possibility of having release branches for k8s integration ?

[sigma]

Oh no, I'm dumb: the part that refers to PersistentVolumeSource.Glusterfs is not vendored in k8s.io/kubernetes, so it would be "safe" (albeit very much not ideal) to proceed with vendoring v8.0.0 for now. Of course, that still means upgrades of those libraries will be incompatible with older go versions down the road.
My brain hurts :)

[munnerz]

I've opened #1542 which rebases this PR, and additionally bumps it to use client-go v1.10.0 which should hopefully make depending on heketi/heketi from kubernetes/kubernetes possible 😄