Test-infra: Use client-go and core API types

[kargakis]

prow hardcodes the API types in [1] and constructs its client in [2]. I guess the intention is to use the APIs from the Kubernetes repo (or maybe another location when all the refactorings settle down) and client-go but I couldn't find an issue about it.

@spxtr

[1] https://github.com/kubernetes/test-infra/blob/master/prow/kube/types.go
[2] https://github.com/kubernetes/test-infra/blob/master/prow/kube/client.go

[spxtr]

cc @apelisse who has an opinion on the matter.

I'm still not convinced it's a good idea. Kubernetes' REST API is well-defined and stable, and I only hit 12 endpoints. The client-go repo adds hundreds of thousands of lines of code and grows all binaries that link into it by over 20MB. It's fine if you're only running a few such binaries in your cluster, but prow is running hundreds at a time.

If we decide it's a good idea then I'll probably be willing to make the switch when it's clear that client-go and friends have settled down. Right now it's a bit immature. We do get some nice things from using client-go, of course. It's easier on the apiserver, for one.

[apelisse]

The client-go repo adds hundreds of thousands of lines of code and grows all binaries that link into it by over 20MB

How did you get that number?

[spxtr]

I imported _ k8s.io/client-go/pkg/api/v1 into cmd/hook and looked at the binary size afterward.

[kargakis]

It's fine if you're only running a few such binaries in your cluster, but prow is running hundreds at a time.

Is this true now with plank?

[spxtr]

Is this true now with plank?

We only run a handful of k8s clients now that plank is in place, so we can consider revisiting this.

[jianhuiz]

Could we at least use Kubernetes types so that the full pod capability can be utilized?
Currently a lot of pod spec fields not defined in prow/kube/types.go which are ignore silently when plank creating pods from the config file.

[kargakis]

Could we at least use Kubernetes types so that the full pod capability can be utilized?
Currently a lot of pod spec fields not defined in prow/kube/types.go which are ignore silently when plank creating pods from the config file.

We can live with this for now. If the need for a missing field arises, we can simply extend our type but I don't know of anything needed that's missing. Do you? On the flip side, I think we should be fine with importing k8s.io/api but the state of dep management in test-infra is not great these days.

[kargakis]

CRD controller example: https://github.com/kubernetes/kubernetes/pull/52753

Switching our controllers to use informers is going to be a bigger win than using the k8s api types (unless the latter is a prerequisite for the former).

[jianhuiz]

I recently tried to use env[].valueFrom.secretKeyRef and volumeMounts[].subPath in my job config, which took me some time to realize that prow defines its own Kubernetes types which is a subset. I can work around this but I guess there were others surprised about it. Given that go 1.9 introduced type aliases, I suppose this should not need many changes, no?

[kargakis]

Yeah, the refactoring should be simple using type aliases. One catch is making godep work and then I don't know whether we have moved away from building prow with older go versions. I think we still use a 1.8 image in Openshift but we could easily update. I can't speak about other consumers of prow though.

[spxtr]

Could we at least use Kubernetes types so that the full pod capability can be utilized?

Yes, we can just copy types.go from k/k into here. That's what the go team does for their CI, actually.

[BenTheElder]

Yes, we can just copy types.go from k/k into here. That's what the go team does for their CI, actually.

You can't though since the upstream types.go has imports as well?
https://github.com/kubernetes/kubernetes/blob/2ecb36802666b9088495d34da7ab486cd3347e65/pkg/api/types.go#L20
https://github.com/kubernetes/kubernetes/blob/2ecb36802666b9088495d34da7ab486cd3347e65/pkg/api/types.go#L677

I get that bloating the binary feels bad, but is 20mb really an issue? Also 20mb added to what initial size?

[kargakis]

I don't think 20mb more is a problem.

[munnerz]

I'd be happy to try and tackle this. I've had a fair bit of experience writing controllers as is, and we've started to use Prow internally at Jetstack.

Before I do - what limitations are there on circular dependencies? I guess I am okay to import k8s.io/api, k8s.io/apimachinery, k8s.io/client-go and k8s.io/code-generator?

I notice that vendoring was fixed a few days ago and switched to dep, which should make things easier! :tada:

[kargakis]

@munnerz not sure there are any limitations. Most people are worried about the size of the dependencies that will need to be pulled. Can we prove that the additional size will not be that much? If so, there are a lot of gains when we switch.

[jianhuiz]

@munnerz I had the PR https://github.com/kubernetes/test-infra/pull/4952 which uses k8s api types. It include very few changes by using type alias in go 1.9 which you may copy those code as a start if you like.
The problem I had was the deps about bazel. I however don't have time recently to have them fixed, besides it seems many people PREFER to have those types redefined.

[kargakis]

I've started working on this.

/assign

[kargakis]

Meta types updated in https://github.com/kubernetes/test-infra/pull/6199
Rest of the prowjob api updated in https://github.com/kubernetes/test-infra/pull/6421

Once the API is updated:

• the directory structure of the place we hold api types in prow will likely need to change
• we need to vendor client-go and code-generator

Then we should be able to start generating prow informers and refactor plank/jenkins operator to use them.

[kargakis]

/unassign
/assign stevekuznetsov

[spiffxp]

/milestone clear
@stevekuznetsov where do we stand on this?

[krzyzacy]

@spiffxp https://github.com/kubernetes/test-infra/pull/8669 is opened

[stevekuznetsov]

We done did it.

/close

[k8s-ci-robot]

@stevekuznetsov: Closing this issue.

In response to this:

We done did it.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.