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.

Sign up for GitHub

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

Jump to bottom

Proposal: Refactor pkg/api/unversioned #37530

Closed
smarterclayton opened this issue Nov 27, 2016 · 9 comments
Closed

Proposal: Refactor pkg/api/unversioned #37530

smarterclayton opened this issue Nov 27, 2016 · 9 comments
Assignees
@smarterclayton
Labels
needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one.

Comments

@smarterclayton
Copy link
Contributor

smarterclayton commented Nov 27, 2016 •
edited
Loading

As part of #33900 and also long running desires to clean up some incomplete abstractions, we need to normalize and resolve the status of pkg/api/unversioned (which is versioned) by moving it to a versioned package.

  1. All "common" server logic will be in pkg/apis/meta/v1, or associated with that package.
  2. Any internal versions of those objects will be in pkg/apis/meta/internalversion
  3. All other objects (versioned or internal) will directly reference the types in pkg/apis/meta/v1
  4. Any interfaces or abstractions that deal with ObjectMeta in its "v1" form will be located in that package (like unstructured and the ListAccessor interfaces)
  5. Types in other groups that duplicate these objects will be removed (eg. v1.ListOptions)
  6. External and internal group versions will call a AddToGroupVersion method from pkg/apis/meta/v1 that registers common types, and other group versions will stop directly registering *Options and other reusable types.
  7. 'pkg/watch/versioned' is part of this interface and should be folded in to pkg/apis/meta/v1

Refactors to perform:

  1. Move pkg/api/unversioned to pkg/apis/meta/v1 and use metav1 to reference it
    This identifies that these types are "versioned" and will be part of the "meta.k8s.io" API group in Proposal for alternate API representations of resources #33900. This group composes all of our common, cross group reusable bits. We use metav1 to reference it internally to be consistent with our other versioned API package names.
  2. Move pkg/watch/versioned into pkg/apis/meta/v1, and move the decoder/encoder in that package into pkg/client/restclient/watch
    The watch event is versioned and has minor conversion logic. The decoders and encoders are used only be pkg/apiserver and pkg/client/restclient.
  3. Move pkg/runtime/unstructured*.go into pkg/apis/meta/v1/unstructured
    The unstructured types implicitly depend on the metav1 schema (they cannot be used against any other type) and should be identified as versioned locked. In the future, we could introduce abstractions that let us pick the appropriate unstructured type.
  4. Move pkg/api/types.go#ListOptions|ExportOptions|DeleteOptions to pkg/apis/meta
    These are internal types deserialized from client input and used by client libraries and pkg/apiserver.
  5. Move ObjectMeta into pkg/apis/meta/v1 and remove the internal ObjectMeta
    ObjectMeta is part of the metav1 api (used by unstructured) and so should not be represented differently. All internal code should be adapted to use this field.
  6. Remove pkg/api/meta/metatypes and use pkg/apis/meta/v1 instead
  7. Remove most methods from pkg/api/meta in favor of methods in pkg/apis/meta/v1.
  8. Copy a temporary version of pkg/api/unversioned as required by heapster/metrics/v1alpha temporarily to preserve internal code until we can update heapster to use the new package.
@smarterclayton smarterclayton self-assigned this Nov 27, 2016
@smarterclayton smarterclayton mentioned this issue Nov 28, 2016
Merged
@smarterclayton
Copy link
Contributor Author

@kubernetes/sig-api-machinery

@smarterclayton smarterclayton mentioned this issue Nov 28, 2016
Merged
@deads2k
Copy link
Contributor

deads2k commented Nov 28, 2016

Any internal versions of those objects will be in pkg/apis/meta

pkg/apis/meta/internalversion instead perhaps?

Move pkg/runtime/unstructured*.go into pkg/apis/meta/v1/unstructured
The unstructured types implicitly depend on the metav1 schema (they cannot be used against any other type) and should be identified as versioned locked. In the future, we could introduce abstractions that let us pick the appropriate unstructured type.

It seems like this "dependency" only extends to the metadata representation. Will we get to have an unstructure type that includes a metav1 metadata section?

@sttts
Copy link
Contributor

sttts commented Nov 28, 2016

We can move pkg/apis/meta straight into k8s.io/genericapiserver in #37551 👍

@smarterclayton
Copy link
Contributor Author

Yeah probably. It has some overlap with client as well.

@smarterclayton
Copy link
Contributor Author

pkg/apis/meta/internalversion instead perhaps?

Not opposed.

It seems like this "dependency" only extends to the metadata representation.

Yes. Not sure exactly how to structure it, it's possible it should just be a layer on top. That's what Accessor was originally supposed to abstract.

@lavalamp
Copy link
Member

lavalamp commented Nov 30, 2016 via email

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Nov 30, 2016 via email

k8s-github-robot pushed a commit that referenced this issue Dec 3, 2016
Merge pull request #37532 from smarterclayton/remove_export
efa42b9 
Automatic merge from submit-queue

Remove ExportOptions from api/internal and use unversioned

Should only have one internal object in use

Part of #37530
k8s-github-robot pushed a commit that referenced this issue Dec 4, 2016
Merge pull request #37534 from smarterclayton/move_unversion
81d788d 
Automatic merge from submit-queue (batch tested with PRs 36816, 37534)

Move pkg/api/unversioned to pkg/apis/meta/v1

This moves code from using pkg/api/unversioned to pkg/apis/meta/v1 with the `metav1` local package name.

Built on top of #37532 (the first three commits related to ExportOptions)

Part of #37530
This was referenced Dec 4, 2016
Merged
Closed
k8s-github-robot pushed a commit that referenced this issue Dec 11, 2016
Merge pull request #38058 from smarterclayton/unstructured
543cfa1 
Automatic merge from submit-queue (batch tested with PRs 38058, 38523)

Move unstructured types under `meta/v1` to indicate they are typed

Add a few abstractions that make them simpler to use from generic code that does not need accessors. Move OwnerReference to meta/v1 and remote metatypes.go

Part of #37530
@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label May 31, 2017
@k8s-github-robot
Copy link

@smarterclayton There are no sig labels on this issue. Please add a sig label by:
(1) mentioning a sig: @kubernetes/sig-<team-name>-misc
(2) specifying the label manually: /sig <label>

Note: method (1) will trigger a notification to the team. You can find the team list here.

@smarterclayton
Copy link
Contributor Author

smarterclayton commented May 31, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smarterclayton smarterclayton

Labels
needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@lavalamp @sttts @smarterclayton @deads2k @k8s-github-robot @k8s-ci-robot