kubeapi with kubevirt

[deitch]

This is intended as a potential replacement for #3730

As described in this comment, kubevirt implemented most of their work correctly, but the final clientset was built in a single module with lots of other things, which led to a messy go.mod that makes it very difficult to include in something else.

We leverage their kubevirt.io/api, but copy the minimum needed to fill in the gaps from kubevirt.io/client-go.

This should be compared carefully to #3730, and see if anything is missing.

@naiming-zededa do you mind running your tests again?

[eriknordmark]

@deitch A set of conflicts to resolve.

I looked over go.mod that don't see anything that stands out.
If @naiming-zededa can re-test with it and also check whether it has the needed APIs that would be great.

Run tests for now.

[deitch]

Rebased, fixed all of the conflicts, checked that we still get sane k8s.io versions, and checked that still can build with -tags kubevirt. Looking good. Time for tests

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8f77232) 19.73% compared to head (a882f20) 19.70%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3762      +/-   ##
==========================================
- Coverage   19.73%   19.70%   -0.04%     
==========================================
  Files         235      235              
  Lines       51710    51710              
==========================================
- Hits        10205    10189      -16     
- Misses      40767    40780      +13     
- Partials      738      741       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[deitch]

Weird yetus complaint here

golangcilint: import 'github.com/lf-edge/eve/pkg/pillar/base' is not allowed from list 'Main' (depguard)

Why would it not be allowed?

[eriknordmark]

Weird yetus complaint here

golangcilint: import 'github.com/lf-edge/eve/pkg/pillar/base' is not allowed from list 'Main' (depguard)

Why would it not be allowed?

Some discussion in golangci/golangci-lint#3877 says it can happen if there is no config file for golangcilint. But why now?

[deitch]

But why now?

No idea. I think we can ignore it for now.

[deitch]

The "PR build" job failure is the same docker hub rate limit issue. It is trying to build the eve-build-runner image and is running into the issue pulling golang.

[zedi-pramodh]

How can we be in sync with upstream changes in Kubevirt if we take this approach of copying the interface definition in our code ?

[deitch]

We can't. In any case, we would have to update.

I plan on opening a PR asking them to move this part of their code to a different library, to remove all of the painful dependencies. In the meantime, this will do.

[zedi-pramodh]

Ok that makes sense. We can live with this until then.

[naiming-zededa]

@deitch i tried to port the 'kubevirt.go' and some change of 'kubeapi.go' into our poc branch, and got those errors:

#31 29.48 # github.com/lf-edge/eve/pkg/pillar/kubeapi
#31 29.48 kubeapi/kubeapi.go:104:78: undefined: kubevirtapi.GroupName
#31 29.48 kubeapi/kubeapi.go:104:98: cannot use kubevirtapi.GroupVersion (variable of type "k8s.io/apimachinery/pkg/runtime/schema".GroupVersion) as string value in struct literal
#31 29.48 kubeapi/kubeapi.go:109:46: undefined: shallowCopy
#31 29.48 kubeapi/kubeapi.go:119:9: cannot use &kubevirtClient{…} (value of type *kubevirtClient) as *KubevirtClientset value in return statement: *kubevirtClient does not implement *KubevirtClientset (type *KubevirtClientset is pointer to interface, not interface)
#31 29.48 kubeapi/kubeapi.go:119:45: unknown field clientSet in struct literal of type kubevirtClient
#31 29.48 kubeapi/kubeapi.go:325:28: clientset.VirtualMachineInstance undefined (type *KubevirtClientset is pointer to interface, not interface)
#31 29.48 kubeapi/kubeapi.go:332:23: clientset.VirtualMachineInstance undefined (type *KubevirtClientset is pointer to interface, not interface)
#31 35.34 make: *** [Makefile:99: vet] Error 1

[deitch]

@naiming-zededa I did manage to vet and build it locally, so I wonder why yours fails. Could be I missed something.

Can you write here exactly how you do that, including pointers to branches and such? I'll try to replicate it locally.

[naiming-zededa]

@deitch in my poc branch, first i added the whole file from your kubevirt.go in the kubeapi directory, then this this patch:
patch-diff.txt

[naiming-zededa]

then I did 'go mod tidy' and 'go mod vendor' in the pkg/pillar directory. I didn't remove the kubevirt client api used in the other files inside poc branch, not sure if that matters.

[deitch]

That patch doesn't look right to me at first glance, but I haven't dug deeper yet.

You can diff my branch off of master on eve.

Can you link to your poc branch? How is it different than master?

[deitch]

then I did 'go mod tidy' and 'go mod vendor' in the pkg/pillar directory. I didn't remove the kubevirt client api used in the other files inside poc branch, not sure if that matters.

I am quite sure it does.

I built my branch based on the code changes from Milan's branch, removed the references to kubevirt's client-go, and then built up the modules correctly.

[deitch]

@naiming-zededa I got it working, mostly. The reason it failed is that you still had references to kubevirt.io/client-go (with all of its messy dependencies) in 2 other places:

• hypervisor/kubevirt.go
• cmd/zedkube/vnc.go

Once I got rid of those, the issues went away. There is a branch here on my clone of eve, and I also opened a PR against your poc-dec3 branch in your repo. Feel free to merge that, or use it just a source and close it.

[deitch]

Rebased to resolve conflict. And checked, builds fine both with and without -tags kubevirt.

And everything is pretty much ok except for that odd yetus error. @eriknordmark

[eriknordmark]

Run eden.

I think we should merge this to master. @naiming you presumably want to catch up with the 100+ commits in master.

[naiming-zededa]

@eriknordmark @deitch we'll figure out to submit the rest of the changes for single node into master

[deitch]

Sure thing @naiming-zededa. You might have an easier time just applying the remaining changes to a new branch based on master post-this-PR. If you just apply yours, and share the new branch, I'm happy to go in and match it to this one without the kubevirt.io/client-go dependency

[milan-zededa]

@deitch Thank you for taking care of the k8s dependency issues!

[deitch]

@milan-zededa my pleasure. I just took your work and adapted it; good teamwork. When you are ready with the next one, base it on this and hit me up if you need help.