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

#50102 Task 1: Move apimachinery/pkg/watch.Until into client-go/tools/watch.UntilWithoutRetry #66906

Merged
merged 4 commits into from Aug 15, 2018

Conversation

tnozicka
Copy link
Contributor

@tnozicka tnozicka commented Aug 2, 2018

What this PR does / why we need it:
This is a split off from #50102 to go in smaller pieces.

Moves apimachinery/pkg/watch.Until into client-go/tools/watch.UntilWithoutRetry and adds context so it is cancelable.

Release note:

NONE

Dev release note:

`apimachinery/pkg/watch.Until` has been moved to `client-go/tools/watch.UntilWithoutRetry`.
While switching please consider using the new `client-go/tools/watch.UntilWithSync` or `client-go/tools/watch.Until`.

/cc @smarterclayton @kubernetes/sig-api-machinery-pr-reviews
/milestone v1.12
/priority important-soon
/kind bug
(bug after the main PR which is this split from)

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Aug 2, 2018
@k8s-ci-robot k8s-ci-robot added this to the v1.12 milestone Aug 2, 2018
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 2, 2018
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2018
@tnozicka tnozicka mentioned this pull request Aug 2, 2018
5 tasks
@tnozicka tnozicka changed the title #50102 task 1: Move apimachinery/pkg/watch.Until into client-go/tools/watch.UntilWithoutRetry #50102 Task 1: Move apimachinery/pkg/watch.Until into client-go/tools/watch.UntilWithoutRetry Aug 2, 2018
@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 2, 2018

/test pull-kubernetes-kubemark-e2e-gce-big

@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 2, 2018

/retest

@fedebongio
Copy link
Contributor

/assign @wenjiaswe

@k8s-ci-robot
Copy link
Contributor

@fedebongio: GitHub didn't allow me to assign the following users: wenjiaswe.

Note that only kubernetes members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @wenjiaswe

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.

@sttts
Copy link
Contributor

sttts commented Aug 3, 2018

Didn't we discuss this before? Why this move?

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 3, 2018
@sttts
Copy link
Contributor

sttts commented Aug 3, 2018

Also many people use this helper I would expect. This move breaks them.

@tnozicka
Copy link
Contributor Author

tnozicka commented Aug 3, 2018

Didn't we discuss this before?

I recall discussing where to put it, thought client-go was what you recommended unless I got you wrong.

Why this move?

Renaming was the consensus between reviewers on #50102.

The move is necessary because of import restrictions and because the other Until* functions need to include informers.

a) moving the package breaks them anyways
b) this function is broken and going away (internal only) to be replaced with one that can retry failed watches in next PR

I don't want to break them twice by moving the function for v1.12 and renaming it in 1.13 where the rest of usages will be replaced and also having to rename the Retry function back to Until. That's why also the rename now since we are already breaking them with package move.

@sttts
Copy link
Contributor

sttts commented Aug 3, 2018

/cancel hold

Missed this reasoning in the PR description.

Please provide a dev-release-note.

@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Pull Request Needs Approval

@tnozicka @kubernetes/sig-api-machinery-misc

Action required: This pull request must have the status/approved-for-milestone label applied by a SIG maintainer. If the label is not applied within 3 days, the pull request will be moved out of the v1.12 milestone.

Pull Request Labels
  • sig/api-machinery: Pull Request will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/bug: Fixes a bug discovered during the current release.
Help

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2018
@neolit123
Copy link
Member

/sig api-machinery
/sig cli

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 67071, 66906, 66722, 67276, 67039). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit b6f0aed into kubernetes:master Aug 15, 2018
@tnozicka tnozicka deleted the rename-until branch August 15, 2018 11:32
k8s-github-robot pushed a commit that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from #50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs #66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)
k8s-publishing-bot added a commit to kubernetes/apimachinery that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/client-go that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/apiserver that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/kube-aggregator that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/sample-apiserver that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/sample-controller that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/apiextensions-apiserver that referenced this pull request Aug 23, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
sttts pushed a commit to sttts/apimachinery that referenced this pull request Sep 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
sttts pushed a commit to sttts/client-go that referenced this pull request Sep 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
sttts pushed a commit to sttts/apiserver that referenced this pull request Sep 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
sttts pushed a commit to sttts/kube-aggregator that referenced this pull request Sep 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
sttts pushed a commit to sttts/sample-apiserver that referenced this pull request Sep 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
sttts pushed a commit to sttts/sample-controller that referenced this pull request Sep 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
sttts pushed a commit to sttts/apiextensions-apiserver that referenced this pull request Sep 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/apimachinery that referenced this pull request Sep 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/client-go that referenced this pull request Sep 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/apiserver that referenced this pull request Sep 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/kube-aggregator that referenced this pull request Sep 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/sample-apiserver that referenced this pull request Sep 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/sample-controller that referenced this pull request Sep 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
k8s-publishing-bot added a commit to kubernetes/apiextensions-apiserver that referenced this pull request Sep 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
wking added a commit to wking/kubernetes that referenced this pull request Oct 29, 2018
3d4a02a (Rename Until to UntilWithoutRetry, 2018-08-02, kubernetes#66906)
performed the rename it claimed, after which there was no longer an
Until function in this package.  3d4a02a had been spun out of [1],
which is still in flight with an improved Until [2].  But until that
lands, don't reference it in the godocs ;).

[1]: kubernetes#50102
[2]: kubernetes@a241de7#diff-b7ca7f71d0c92e184f946e29d5734a4eR96
tamalsaha pushed a commit to kmodules/shared-informer that referenced this pull request Aug 13, 2020
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

#50102 Task 2: Add UntilWithSync

**What this PR does / why we need it**:
This is a split off from kubernetes/kubernetes#50102 to go in smaller pieces.

Introduces UntilWithSync based on informer.

**Needs kubernetes/kubernetes#66906 first**
/hold

**Release note**:
```release-note
NONE
```

/priority important-soon
/kind bug
(bug after the main PR which is this split from)

Kubernetes-commit: c4f355a2ad9692f5459541d4e4d94fcbc5f7d946
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. milestone/needs-approval priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants