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

Remove TPR resource in favor of CRD #48152

Closed
deads2k opened this issue Jun 27, 2017 · 11 comments
Closed

Remove TPR resource in favor of CRD #48152

deads2k opened this issue Jun 27, 2017 · 11 comments
Assignees
@deads2k
Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Milestone
v1.8

Comments

@deads2k
Copy link
Contributor

deads2k commented Jun 27, 2017 •
edited by enisoc
Loading

In 1.6 TPR was indicated as unstable and subject to change [1]. Near the end of 1.6 we talked about a new API and how we would transition [2]. In 1.7 we updated the docs to announce deprecation and provided the alternative and a clean semi-automatic migration path [3]. They were specifically called out in the release notes [4]. There is acknowledgement in the community that they will be removed [5], and following our deprecation policy we can remove them in 1.8 and focus our efforts on adding features like validation to CRDs.

@kubernetes/sig-api-machinery-misc @enisoc @smarterclayton @sttts @nikhita
@deads2k deads2k self-assigned this Jun 27, 2017
@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 27, 2017
@enisoc enisoc added this to the v1.8 milestone Jun 27, 2017
@enisoc enisoc mentioned this issue Jun 27, 2017
Closed
@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 27, 2017 via email

@enisoc
Copy link
Member

enisoc commented Jun 27, 2017

The migration helper intentionally leaves behind the etcd data for TPRs, for rollback and to prevent watchers from seeing deletes.

Should we clean up this data eventually? Is there a precedent for how we handle etcd data for resources that were deprecated and removed?

@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 27, 2017 via email

@k82cn
Copy link
Member

k82cn commented Jun 28, 2017

/cc

@deads2k deads2k mentioned this issue Jun 28, 2017
Closed
@jon-uw jon-uw mentioned this issue Jun 28, 2017
Closed
@MHBauer MHBauer mentioned this issue Jun 28, 2017
Closed
@deads2k deads2k mentioned this issue Jun 30, 2017
Merged
@sttts
Copy link
Contributor

sttts commented Jul 5, 2017

Lgtm. Let's proceed with this.

deads2k pushed a commit to deads2k/kubernetes that referenced this issue Jul 5, 2017
Merge pull request kubernetes#48353 from deads2k/tpr-17-delete-01
c10cc3d 
Automatic merge from submit-queue (batch tested with PRs 48480, 48353)

remove tpr api access

xref kubernetes#48152

TPR tentacles go pretty deep. This gets us started by removing API access and we'll move down from there.

@kubernetes/sig-api-machinery-misc 
@ironcladlou this should free up the GC implementation since TPRs will no longer be present and failing.

```release-note
Removing TPR api access per kubernetes#48152
```
@deads2k deads2k mentioned this issue Jul 5, 2017
Merged
k8s-github-robot pushed a commit that referenced this issue Jul 5, 2017
Merge pull request #48498 from deads2k/tpr-18-delete-02
96d8ab7 
Automatic merge from submit-queue (batch tested with PRs 45467, 48091, 48033, 48498)

bulk delete of tpr packages

related to #48152

Bulk delete of the TPR code.  I made the minimal changes outside the delete to try to keep it easy to review.
@deads2k deads2k mentioned this issue Jul 7, 2017
Merged
@enisoc
Copy link
Member

enisoc commented Jul 7, 2017 •
edited
Loading

We need at least exactly one PR with release-note-action-required. I put it on #48353 since that was the first step. I also added a link to the migration guide in the release note.

@deads2k
Copy link
Contributor Author

deads2k commented Jul 7, 2017

We need at least exactly one PR with release-note-action-required. I put it on #48353 since that was the first step. I also added a link to the migration guide in the release note.

Makes sense. I put the release-note on that one, but forgot about the extra "action required" bit.

@sttts sttts mentioned this issue Jul 8, 2017
Merged
k8s-github-robot pushed a commit that referenced this issue Jul 8, 2017
Merge pull request #48546 from deads2k/tpr-19-ripples
a2e463f 
Automatic merge from submit-queue (batch tested with PRs 48497, 48604, 48599, 48560, 48546)

remove dead code

This removes the dead code cruft since we stopped serving TPRs.

ref #48152
k8s-github-robot pushed a commit that referenced this issue Jul 8, 2017
Merge pull request #48604 from sttts/sttts-remove-tpr-example
ed66bfd 
Automatic merge from submit-queue (batch tested with PRs 48497, 48604, 48599, 48560, 48546)

client-go: remove TPR example

Now that the TPR api is gone (#48152).
@maciaszczykm maciaszczykm mentioned this issue Jul 18, 2017
Closed
@enisoc enisoc mentioned this issue Jul 26, 2017
Closed
@sttts
Copy link
Contributor

sttts commented Aug 29, 2017

This is done.

@sttts sttts closed this as completed Aug 29, 2017
@spiffxp
Copy link
Member

spiffxp commented Oct 20, 2017

I agree that this decision was well communicated, and do not wish to revert it, but I wanted to revisit it for a moment as prior art for future deprecations. Did we remove TPR's a release too early?

If TPR's were actually alpha, then I think we did fine. But https://v1-6.docs.kubernetes.io/docs/api-reference/v1.6/#thirdpartyresource-v1beta1-extensions shows them as v1beta1

CRD's don't show up in the API reference docs in v1.7 https://v1-7.docs.kubernetes.io/docs/api-reference/v1.7/

Using the table listed in https://kubernetes.io/docs/reference/deprecation-policy/ as an example, it seems like we should have kept TPR's around until v1.9:

  • X+2 = 1.6 -> v1beta1 TPR
  • X+3 = 1.7 -> v1beta1 CRD, v1beta1 TPR deprecated
  • X+4 = 1.8 -> v1beta CRD, v1beta1 TPR deprecated... but we instead removed
  • X+5 = 1.9 -> v1beta1 CRD

Am I misinterpreting the deprecation policy, or did we make an exception here?

@liggitt
Copy link
Member

liggitt commented Oct 20, 2017

For the extensions group specifically, "beta" doesn't necessarily mean beta. Calling the extensions API group beta was a misnomer and pre-dated crisp alpha/beta/ga definitions and the deprecation policy. The objects within that API group were a mix of alpha and beta quality APIs.

TPR's were actually alpha. They did not complete the gathered requirements for beta, and had known issues. The user guide was updated in 1.5 to indicate they were not considered stable.

@enisoc
Copy link
Member

enisoc commented Oct 20, 2017

In addition, at the time when we did this, the Beta deprecation policy was to keep the old API around for "3 months, 1 release". It was later changed without any announcement (that I saw) to "6 months, 2 releases":

kubernetes/website@8f96772#diff-20e68d228e6f9fe6ccc143526a382670R78

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

@deads2k deads2k

Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Milestone
v1.8
Development

No branches or pull requests
8 participants
@spiffxp @sttts @liggitt @smarterclayton @enisoc @deads2k @k82cn @k8s-ci-robot