[Umbrella] Make client-go lighter and easier to consume #127888

aojea · 2024-10-06T09:38:09Z

Description

The current structure of client-go, with its dependencies on k8s.io/api and k8s.io/apimachinery present some significant challenges:

Dependency bloat: projects end up pulling in a large number of transitive dependencies, increasing binary size and potentially leading to conflicts.
Versioning headaches: Updating client-go often necessitates updating the dependent projects as well, which can be complex and time-consuming, especially with the need for coordinated releases.
Tight coupling: This structure creates tight coupling between client-go and the underlying API definitions, making necessary to update the clients to be able to use the new API types.

How to repro

Using the workqueue example https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/client-go/examples/workqueue and creating a new project, after doing go mod tidy the go.mod file looks like:

go.mod

module client-go-example

go 1.23

require (
        k8s.io/api v0.31.1
        k8s.io/apimachinery v0.31.1
        k8s.io/client-go v0.31.1
        k8s.io/klog/v2 v2.130.1
)

require (
        github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
        github.com/emicklei/go-restful/v3 v3.11.0 // indirect
        github.com/fxamacker/cbor/v2 v2.7.0 // indirect
        github.com/go-logr/logr v1.4.2 // indirect
        github.com/go-openapi/jsonpointer v0.19.6 // indirect
        github.com/go-openapi/jsonreference v0.20.2 // indirect
        github.com/go-openapi/swag v0.22.4 // indirect
        github.com/gogo/protobuf v1.3.2 // indirect
        github.com/golang/protobuf v1.5.4 // indirect
        github.com/google/gnostic-models v0.6.8 // indirect
        github.com/google/go-cmp v0.6.0 // indirect
        github.com/google/gofuzz v1.2.0 // indirect
        github.com/google/uuid v1.6.0 // indirect
        github.com/imdario/mergo v0.3.6 // indirect
        github.com/josharian/intern v1.0.0 // indirect
        github.com/json-iterator/go v1.1.12 // indirect
        github.com/mailru/easyjson v0.7.7 // indirect
        github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
        github.com/modern-go/reflect2 v1.0.2 // indirect
        github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
        github.com/spf13/pflag v1.0.5 // indirect
        github.com/x448/float16 v0.8.4 // indirect
        golang.org/x/net v0.26.0 // indirect
        golang.org/x/oauth2 v0.21.0 // indirect
        golang.org/x/sys v0.21.0 // indirect
        golang.org/x/term v0.21.0 // indirect
        golang.org/x/text v0.16.0 // indirect
        golang.org/x/time v0.3.0 // indirect
        google.golang.org/protobuf v1.34.2 // indirect
        gopkg.in/inf.v0 v0.9.1 // indirect
        gopkg.in/yaml.v2 v2.4.0 // indirect
        gopkg.in/yaml.v3 v3.0.1 // indirect
        k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
        k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
        sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
        sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
        sigs.k8s.io/yaml v1.4.0 // indirect
)

and its size is

ls -lah main
-rwxr-x--- 1 aojea primarygroup 51M Oct  6 08:41 main

Proposal

Modularization: Breaking down client-go into smaller, more focused modules would allow users to import only the functionalities they need, reducing dependency bloat and improving maintainability.
Generic clients and Versioned APIs: Providing generic clients that can use versioned API clients would enable users to target specific Kubernetes versions without being forced to update the entire client-go library.
Reducing dependencies: eliminate unnecessary dependencies within client-go and its related projects.

Design Details

k8s.io/api

Ideally this should be a self contained module without dependencies, but it depends on apimachinery for the generated code and registering the types

kubernetes/staging/src/k8s.io/api/go.mod

Lines 3 to 13 in 7b28a11

    
           module k8s.io/api 
        
           go 1.23.0 
        
           godebug default=go1.23 
        
           require ( 
        
           	github.com/gogo/protobuf v1.3.2 
        
           	github.com/stretchr/testify v1.9.0 
        
           	k8s.io/apimachinery v0.0.0 
        
           )

It requires k8s.io/apimachinery because of:

schema

register.go:     "k8s.io/apimachinery/pkg/runtime"
register.go:     "k8s.io/apimachinery/pkg/runtime/schema"
zz_generated.deepcopy.go:        runtime "k8s.io/apimachinery/pkg/runtime"

metav1

register.go:     metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
zz_generated.deepcopy.go:        v1 "k8s.io/apimachinery/pkg/apis/meta/v1"

utils for types

"k8s.io/apimachinery/pkg/util/validation"
zz_generated.deepcopy.go:  intstr "k8s.io/apimachinery/pkg/util/intstr"
resource/v1alpha3/types.go:     "k8s.io/apimachinery/pkg/util/validation"

NOTE This validation can be easily avoided and it seems is only to match a constant (cc: @pohly )

kubernetes/staging/src/k8s.io/api/resource/v1alpha3/types.go

Line 186 in 7b28a11

    
           const PoolNameMaxLength = validation.DNS1123SubdomainMaxLength // Same as for a single node name.

k8s.io/apimachinery

This is the more tricky, and it is bringing a lot of dependencies

kubernetes/staging/src/k8s.io/apimachinery/go.mod

Lines 3 to 33 in 7b28a11

    
           module k8s.io/apimachinery 
        
           go 1.23.0 
        
           godebug default=go1.23 
        
           require ( 
        
           	github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 
        
           	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc 
        
           	github.com/fxamacker/cbor/v2 v2.7.0 
        
           	github.com/gogo/protobuf v1.3.2 
        
           	github.com/golang/protobuf v1.5.4 
        
           	github.com/google/gnostic-models v0.6.8 
        
           	github.com/google/go-cmp v0.6.0 
        
           	github.com/google/gofuzz v1.2.0 
        
           	github.com/google/uuid v1.6.0 
        
           	github.com/moby/spdystream v0.4.0 
        
           	github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f 
        
           	github.com/onsi/ginkgo/v2 v2.19.0 
        
           	github.com/spf13/pflag v1.0.5 
        
           	github.com/stretchr/testify v1.9.0 
        
           	golang.org/x/net v0.28.0 
        
           	golang.org/x/time v0.3.0 
        
           	gopkg.in/evanphx/json-patch.v4 v4.12.0 
        
           	gopkg.in/inf.v0 v0.9.1 
        
           	k8s.io/klog/v2 v2.130.1 
        
           	k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2 
        
           	k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 
        
           	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd 
        
           	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 
        
           	sigs.k8s.io/yaml v1.4.0

k8s.io/client-go

These module contains all the generated code, the typed clients and the dynamic client. Thanks to @skitt we already have Generics to share code in client-go

In addition to the dynamic client we could have a Generic client, that will be able to work with different versions of k8s.io/api, allowing users to work with skew between k8s.io/client-go and k8s.io/api, though this requires a very high bar on these modules to avoid breaking clients and provide stability across the ecosystem.

kubernetes/staging/src/k8s.io/client-go/go.mod

Lines 3 to 37 in 7b28a11

    
           module k8s.io/client-go 
        
           go 1.23.0 
        
           godebug default=go1.23 
        
           require ( 
        
           	github.com/gogo/protobuf v1.3.2 
        
           	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da 
        
           	github.com/golang/protobuf v1.5.4 
        
           	github.com/google/gnostic-models v0.6.8 
        
           	github.com/google/go-cmp v0.6.0 
        
           	github.com/google/gofuzz v1.2.0 
        
           	github.com/google/uuid v1.6.0 
        
           	github.com/gorilla/websocket v1.5.0 
        
           	github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 
        
           	github.com/peterbourgon/diskv v2.0.1+incompatible 
        
           	github.com/spf13/pflag v1.0.5 
        
           	github.com/stretchr/testify v1.9.0 
        
           	go.uber.org/goleak v1.3.0 
        
           	golang.org/x/net v0.28.0 
        
           	golang.org/x/oauth2 v0.21.0 
        
           	golang.org/x/term v0.23.0 
        
           	golang.org/x/time v0.3.0 
        
           	google.golang.org/protobuf v1.34.2 
        
           	gopkg.in/evanphx/json-patch.v4 v4.12.0 
        
           	k8s.io/api v0.0.0 
        
           	k8s.io/apimachinery v0.0.0 
        
           	k8s.io/klog/v2 v2.130.1 
        
           	k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2 
        
           	k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 
        
           	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd 
        
           	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 
        
           	sigs.k8s.io/yaml v1.4.0 
        
           )

Ideally we should be able to split the API types from the tooling and allow consumers to use generics, something like

import (
  corev1 "k8s.io/api/core/v1"
	k8s.io/client-go/generic
)


func main(
	client := generic.NewClient[corev1.Pod]
	list, err := client.Namespace("foo").List()

)

and use generic controllers, we already have some intree

kubernetes/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/internal/generic/interface.go

Lines 27 to 59 in a7fcc89

    
           type Controller[T runtime.Object] interface { 
        
           	// Meant to be run inside a goroutine 
        
           	// Waits for and reacts to changes in whatever type the controller 
        
           	// is concerned with. 
        
           	// 
        
           	// Returns an error always non-nil explaining why the worker stopped 
        
           	Run(ctx context.Context) error 
        
           	// Retrieves the informer used to back this controller 
        
           	Informer() Informer[T] 
        
           	// Returns true if the informer cache has synced, and all the objects from 
        
           	// the initial list have been reconciled at least once. 
        
           	HasSynced() bool 
        
           } 
        
           type NamespacedLister[T any] interface { 
        
           	// List lists all ValidationRuleSets in the indexer for a given namespace. 
        
           	// Objects returned here must be treated as read-only. 
        
           	List(selector labels.Selector) (ret []T, err error) 
        
           	// Get retrieves the ValidationRuleSet from the indexer for a given namespace and name. 
        
           	// Objects returned here must be treated as read-only. 
        
           	Get(name string) (T, error) 
        
           } 
        
           type Informer[T any] interface { 
        
           	cache.SharedIndexInformer 
        
           	Lister[T] 
        
           } 
        
           // Lister[T] helps list Ts. 
        
           // All objects returned here must be treated as read-only. 
        
           type Lister[T any] interface {

Tasks

remove unnecessary dependencies, per example, testing frameworks only offer cosmetic benefits but bring dependencies, despite they are not used to build the binary the make more noisy the dependency tree remove unnecessary dependency on k8s.io/api #127876
consolidate, per example, we have our own fork of go-yaml Create a fork of go-yaml kubernetes-sigs/yaml#76 so we can switch all the dependencies to it to have just one path and also avoid possible code drifting
stricter rules for client-go and apimachinery exported code, @pohly already has setup a job as optional apidiff: enable trial build with controller-manager test-infra#33579
Avoid adding a dependency to k8s.io/api on an apimachinery constant #127889

References

Notes

I don't create a KEP because there are a considerable number of dimensions in this problem, so as first step I just want to gather feedback and take a more iterative approach, there are things like dependency pruning that can be worked out without a KEP, once we reach a point we need to implement some user facing change I will open the corresponding KEP

/area code-organization
/sig architecture
/sig api-machinery

The text was updated successfully, but these errors were encountered:

aojea · 2024-10-06T09:39:56Z

Tagging some of the people I know will be interested/impacted
/cc @deads2k @liggitt @dims @fedebongio @jpbetz @BenTheElder @thockin @pohly @alvaroaleman @howardjohn

pohly · 2024-10-06T12:20:57Z

Here's one more thought: k8s.io/client-go contains lots of loosely connected packages. Would it make sense to break it apart into smaller modules?

If a simple Go client doesn't need k8s.io/client-go/tools/record, it shouldn't depend on github.com/golang/groupcache/lru.

I'm aware that this raises challenges for our publishing bot because we don't want to change the import paths. For example, if k8s.io/client-go/tools/record was its own module, where do we publish the code? Can k8s.io/client-go/tools/record redirect to github.com/kubernetes/client-go-tools-record or does github.com/kubernetes/client-go have to become a multi-module repository?

Can Go handle the transition of a package into its own module? For v0.31.0, k8s.io/client-go/tools/record has to be found as package in client-go. For a future release, it would have to be found as module.

pohly · 2024-10-06T18:36:32Z

Here's one more thought: k8s.io/client-go contains lots of loosely connected packages. Would it make sense to break it apart into smaller modules?

Let me take this one step further: if don't do this, how much can we really reduce the dependencies of client-go? It doesn't help to remove a dependency from e.g. k8s.io/api if k8s.io/client-go then still depends on that same dependency through something else. For example, how useful is #127889 for client-go? It will still depend on apimachinery.

aojea · 2024-10-06T21:38:06Z

It will still depends on apimachinery.

that is the thing, if it does not depend on apimachinery, or if apimachinery is lighter, or if we break apimachinery in smaller modules, .... but we should not use as excuse that there is already dependencies to add new, it is in the best for all of us maintainers to untangle things as much as possible

pohly · 2024-10-07T05:36:32Z

I agree that it makes sense to simplify.

But removing a dependency on "bar" from "foo" should not be based on the argument that it's being done to remove "bar" from the client-go dependency tree unless there really is a plan how that can be achieved.

aojea · 2024-10-07T06:31:51Z

why not, everything that removes dependencies that are not necessary helps

pohly · 2024-10-07T07:56:08Z

If some package becomes cleaner because it avoids some dependency, that's fine. But if it becomes more complex and client-go still depends on "bar", then it's a net loss.

aojea · 2024-10-07T08:12:16Z

If some package becomes cleaner because it avoids some dependency, that's fine. But if it becomes more complex and client-go still depends on "bar", then it's a net loss

absolutely, goal is simplifying across the stack , as soon as things require more design and consensus we need a KEP with details ... but per example, since we have now a fork of go-yaml kubernetes-sigs/yaml#76 we can switch all the dependencies to it to have just one path and also avoid possible code drifting ... things like this can help to reduce the problem scope

skitt · 2024-10-07T08:18:35Z

Related to simplifying across the stack, there are a number of in-progress transitions or duplications of code/effort in the tree. They don’t get much attention, and we tend to frown upon “blanket deprecation” PRs, but I do think it would be good to make progress on some of them, especially if they can help reduce dependency trees. For example, k8s.io/utils/pointer to k8s.io/utils/ptr (not significant here because it’s the same dependency); or k8s/io/apimachinery/pkg/util/sets and k8s.io/utils/set (client-go currently uses the former; the two aren’t quite equivalent, see kubernetes/utils#294 in particular).

skitt · 2024-10-07T08:21:56Z

Can Go handle the transition of a package into its own module? For v0.31.0, k8s.io/client-go/tools/record has to be found as package in client-go. For a future release, it would have to be found as module.

In my experience it works reasonably well. In theory, when a module is split out, the “main” module must declare a dependency on the new module, so that existing users aren’t broken on upgrades; but that defeats the purpose if the goal is to simplify dependency trees. go get usually figures out what’s going on as long as it doesn’t run into ambiguous imports.

skitt · 2024-10-07T08:30:06Z

#127889 makes me wonder if we could move parts of apimachinery that meet k8s.io/utils’ requirements to k8s.io/utils? apimachinery has a big set of dependencies, k8s.io/utils has a minimal set and is supposed to keep it that way. For example, all of apimachinery validation could be moved to k8s.io/utils if we move validation/fields, errors and sets too (for sets, replace it with k8s.io/utils sets).

Put another way, we’ve ended up with two repositories of dependency-less utilities in k8s.io, apimachinery and sets, sometimes with duplicates. I think it would help this overall endeavour if we decided where these should live.

skitt · 2024-10-07T12:32:29Z

Consolidating on sigs.k8s.io/yaml doesn’t help with the dependency tree, but it does improve consistency. See #127901

howardjohn · 2024-10-07T13:45:19Z

From my POV there are two aspects to 'light' -- low dependencies (less chance of dep hell, less licenses, etc), and smaller size (impacting final binary size which impacts end users).

WRT to the smaller size, a very substantial portion of this is due to basically any import of client-go bringing in everything in istio.io/api; you cannot just import core/v1 for example. This requires pretty deep changes throughout a lot of things - the schema, setting up the base client stuff, etc. Somehow fixing this would cut binary sizes in half, even for packages that actually depend on a large number of APIs.

(Unfortunately, even in an optimal package layout, if you depend on the tiny ConfigMap you also depend on the huge Pod)

howardjohn · 2024-10-07T16:09:02Z

From my POV there are two aspects to 'light' -- low dependencies (less chance of dep hell, less licenses, etc), and smaller size (impacting final binary size which impacts end users).

WRT to the smaller size, a very substantial portion of this is due to basically any import of client-go bringing in everything in istio.io/api; you cannot just import core/v1 for example. This requires pretty deep changes throughout a lot of things - the schema, setting up the base client stuff, etc. Somehow fixing this would cut binary sizes in half, even for packages that actually depend on a large number of APIs.

(Unfortunately, even in an optimal package layout, if you depend on the tiny ConfigMap you also depend on the huge Pod)

To make this more concrete, this example https://github.com/howardjohn/k8s-client-go/blob/e0023d6daa2795430e532f5813108f8087ec2014/generics/example/main.go#L19 (which compiles) is 9.8Mb stripped, 15M unstripped. With an import of k8s.io/client-go/kubernetes its instead 30M/44M.

FWIW in a not-direct comparison, using kube-rs (rust), it is more like 7.6M/11M.

alvaroaleman · 2024-10-07T16:27:34Z

To make this more concrete, this example https://github.com/howardjohn/k8s-client-go/blob/e0023d6daa2795430e532f5813108f8087ec2014/generics/example/main.go#L19 (which compiles) is 9.8Mb stripped, 15M unstripped. With an import of k8s.io/client-go/kubernetes its instead 30M/44M.

And the main reason for importing k8s.io/client-go/kubernetes even when using some sort of dynamic client is the scheme. The scheme in turn is needed to go from go type to GVK and vice versa. I don't know if there is much use-case for the latter outside of the kube-apiserver and the former would be much better solved by adding this info to the types rather than to a global (ideally together with restmapping, to avoid needing that lookup for dynamic clients).

Jefftree · 2024-10-08T20:18:49Z

/triage accepted

BenTheElder · 2024-10-08T22:34:27Z

From my POV there are two aspects to 'light' -- low dependencies (less chance of dep hell, less licenses, etc), and smaller size (impacting final binary size which impacts end users).

One other aspect we're looking for is ease of switching Kubernetes API versions specifically, ideally the transport layer etc is pretty stable and you should be able to quickly adopt and implement custom behaviors for the new types without needing to update any mess of dependencies, just a package with the types.

I've been thinking a bit about how we might approach this and avoid breaking anyone at the same time.

Here's one more thought: k8s.io/client-go contains lots of loosely connected packages. Would it make sense to break it apart into smaller modules?

I would go a step further actually: some of this should be in non-staged libraries that are actually versioned and developed with stable consumption in mind in their own repos, rather than being "staged" out as unstable v0.x

But I think the first step is making it easier to consume the API types, with which you could bring your own client or skew the client (and use dynamic or a new generic API).

If I read #127888 (comment) right, I think alvaro is roughly advocating for that part at least as well, and I have recently floated the concept to a few people (like antonio) who seem to agree.

We're in the middle of enhancements freeze right now and will probably all have more bandwidth for this soon, possibly even at KubeCon?

aojea mentioned this issue Oct 6, 2024

Avoid adding a dependency to k8s.io/api on an apimachinery constant #127889

Open

skitt mentioned this issue Oct 7, 2024

Use sigs.k8s.io/yaml instead of gopkg.in/yaml #127901

Merged

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Umbrella] Make client-go lighter and easier to consume #127888

[Umbrella] Make client-go lighter and easier to consume #127888

aojea commented Oct 6, 2024 •

edited

Loading

aojea commented Oct 6, 2024

pohly commented Oct 6, 2024 •

edited

Loading

pohly commented Oct 6, 2024 •

edited

Loading

aojea commented Oct 6, 2024 •

edited

Loading

pohly commented Oct 7, 2024

aojea commented Oct 7, 2024

pohly commented Oct 7, 2024

aojea commented Oct 7, 2024

skitt commented Oct 7, 2024

skitt commented Oct 7, 2024

skitt commented Oct 7, 2024

skitt commented Oct 7, 2024

howardjohn commented Oct 7, 2024

howardjohn commented Oct 7, 2024

alvaroaleman commented Oct 7, 2024 •

edited

Loading

Jefftree commented Oct 8, 2024

BenTheElder commented Oct 8, 2024

[Umbrella] Make client-go lighter and easier to consume #127888

[Umbrella] Make client-go lighter and easier to consume #127888

Comments

aojea commented Oct 6, 2024 • edited Loading

Description

How to repro

Proposal

Design Details

k8s.io/api

k8s.io/apimachinery

k8s.io/client-go

Tasks

References

Notes

aojea commented Oct 6, 2024

pohly commented Oct 6, 2024 • edited Loading

pohly commented Oct 6, 2024 • edited Loading

aojea commented Oct 6, 2024 • edited Loading

pohly commented Oct 7, 2024

aojea commented Oct 7, 2024

pohly commented Oct 7, 2024

aojea commented Oct 7, 2024

skitt commented Oct 7, 2024

skitt commented Oct 7, 2024

skitt commented Oct 7, 2024

skitt commented Oct 7, 2024

howardjohn commented Oct 7, 2024

howardjohn commented Oct 7, 2024

alvaroaleman commented Oct 7, 2024 • edited Loading

Jefftree commented Oct 8, 2024

BenTheElder commented Oct 8, 2024

aojea commented Oct 6, 2024 •

edited

Loading

pohly commented Oct 6, 2024 •

edited

Loading

pohly commented Oct 6, 2024 •

edited

Loading

aojea commented Oct 6, 2024 •

edited

Loading

alvaroaleman commented Oct 7, 2024 •

edited

Loading