Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a metadata client to client-go that can read PartialObjectMetadata #77819

Merged
merged 1 commit into from Jul 3, 2019

Conversation

@smarterclayton
Copy link
Contributor

commented May 13, 2019

This client exposes operations on generic metadata (get, list, watch,
delete) and allows patch operations. The client always uses protobuf and
requests the server transform the response into the appropriate object.
Using this client simplifies the work of generic controllers by allowing
them to treat all objects the same, and also improves performance both in
the amount of data sent as well as allowing protobuf on CRD resources.

First commit is from #77817 which this PR also tests

Part of https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190322-server-side-get-to-ga.md

/kind api-change

TODO:

  • Improve tests around fallback to JSON decoding of various different types
A new client `k8s.io/client-go/metadata.Client` has been added for accessing objects generically. This client makes it easier to retrieve only the metadata (the `metadata` sub-section) from resources on the cluster in an efficient manner for use cases that deal with objects generically, like the garbage collector, quota, or the namespace controller. The client asks the server to return a `meta.k8s.io/v1 PartialObjectMetadata` object for list, get, delete, watch, and patch operations on both normal APIs and custom resources which can be encoded in protobuf for additional work. If the server does not yet support this API the client will gracefully fall back to JSON and transform the response objects into PartialObjectMetadata.
@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

@k8s-ci-robot k8s-ci-robot requested review from caesarxuchao and deads2k May 13, 2019

@smarterclayton smarterclayton changed the title Add a metadata client to client-go that can read PartialObjectMetadata WIP: Add a metadata client to client-go that can read PartialObjectMetadata May 13, 2019

@smarterclayton smarterclayton force-pushed the smarterclayton:client branch from de39039 to ab89aac May 13, 2019

@k8s-ci-robot k8s-ci-robot added size/XXL and removed size/XL labels May 13, 2019

@smarterclayton smarterclayton force-pushed the smarterclayton:client branch 3 times, most recently from c14a133 to 56b51ad May 13, 2019

@yliaog

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

/cc

@k8s-ci-robot k8s-ci-robot requested a review from yliaog May 13, 2019

@fedebongio

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

@smarterclayton smarterclayton force-pushed the smarterclayton:client branch from 411bab0 to d001164 Jun 24, 2019

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2019

Updated with feedback addressed (or commented otherwise)

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2019

/retest

@smarterclayton smarterclayton force-pushed the smarterclayton:client branch from d001164 to 0fe4fba Jul 2, 2019

Add a metadata client to client-go that can read PartialObjectMetadata
This client exposes operations on generic metadata (get, list, watch, delete)
and allows patch operations. The client always uses protobuf and requests
the server transform the response into the appropriate object. Using this
client simplifies the work of generic controllers by allowing them to treat
all objects the same, and also improves performance both in the amount of
data sent as well as allowing protobuf on CRD resources.

@smarterclayton smarterclayton force-pushed the smarterclayton:client branch from 0fe4fba to 21f5e64 Jul 2, 2019

@liggitt

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

the shape of the client lgtm. The tests that were there seemed reasonable (I could take them as-is or with refactoring as @lavalamp mentioned), but I wasn't positive whether the protobuf and json PartialObjectMetadata cases were exercised.

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

They should be in the integration tests

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

The two downstream PRs will consume this may generate additional comments around use, although it was more safety and behavior here. They add the informers (which I can split into its own pull) which effectively copies the dynamic informer but strips out the type unsafety stuff (a metadata informer is >>> more constrained than a dynamic informer)

@liggitt

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 3, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 024c7bd into kubernetes:master Jul 3, 2019

23 checks passed

cla/linuxfoundation smarterclayton authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.