Pass DeleteOptions down to the Reactor #102945

chenchun · 2021-06-17T10:05:48Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

client-go: pass `DeleteOptions` down to the fake client `Reactor`

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2021-06-17T10:05:56Z

Hi @chenchun. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

fedebongio · 2021-06-17T20:10:26Z

/assign @caesarxuchao
/triage accepted

k8s-triage-robot · 2021-09-15T20:41:01Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

enj · 2021-09-27T16:13:07Z

/assign

enj · 2021-09-27T20:30:11Z

/remove-lifecycle stale

enj · 2021-09-27T20:32:49Z

staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake/generator_fake_for_type.go

+		$if .namespaced$Invokes($.NewDeleteAction|raw$($.type|allLowercasePlural$Resource, c.ns, name, &opts), &$.type|raw${})
+		$else$Invokes($.NewRootDeleteAction|raw$($.type|allLowercasePlural$Resource, name, &opts), &$.type|raw${})$end$


This needs to pass in a nil when opts is the zero value of the struct.

staging/src/k8s.io/client-go/testing/actions.go

enj · 2021-09-27T20:39:39Z

I expect a lot of unit tests to fail because of #102945 (comment)

/ok-to-test

chenchun · 2021-10-12T02:56:04Z

@enj Thanks for review. Previously lots of tests failed because I added a DeleteOptions param to NewDeleteAction and forgot to change all places to add a nil input param. I updated PR to add a new func NewDeleteActionWithOptions instead to fix the falling tests.

enj

Minor stuff.

pkg/controller/serviceaccount/tokens_controller_test.go

staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake/generator_fake_for_type.go

staging/src/k8s.io/client-go/testing/actions.go

enj

Just some extraneous whitespace.

pkg/controller/bootstrap/tokencleaner_test.go

pkg/controller/serviceaccount/tokens_controller_test.go

enj · 2021-10-28T14:43:43Z

This LGTM once the whitespace is sorted. Assigning to Jordan for review/approval since this touches generated code all over the codebase.

/assign @liggitt

chenchun · 2021-10-29T04:08:29Z

/retest-required

liggitt

one nit on the uid used in the test, lgtm otherwise

pkg/controller/bootstrap/common_test.go

Co-authored-by: Mo Khan <theenjeru@gmail.com>

enj · 2021-11-02T04:00:20Z

/lgtm

enj · 2021-11-02T04:02:23Z

/milestone v1.23
/priority important-longterm

liggitt · 2021-11-02T12:37:16Z

/approve

k8s-ci-robot · 2021-11-02T12:37:54Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~pkg/controller/OWNERS~~ [liggitt]
~~staging/src/k8s.io/apiextensions-apiserver/OWNERS~~ [liggitt]
~~staging/src/k8s.io/client-go/OWNERS~~ [liggitt]
~~staging/src/k8s.io/code-generator/OWNERS~~ [liggitt]
~~staging/src/k8s.io/kube-aggregator/OWNERS~~ [liggitt]
~~staging/src/k8s.io/sample-apiserver/OWNERS~~ [liggitt]
~~staging/src/k8s.io/sample-controller/OWNERS~~ [liggitt]

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 requested review from caesarxuchao and deads2k June 17, 2021 10:06

chenchun force-pushed the fake branch from 3223d85 to 2ab683f Compare June 17, 2021 10:08

k8s-ci-robot assigned caesarxuchao Jun 17, 2021

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 Jun 17, 2021

enj added this to Needs Triage in SIG Auth Old Jul 6, 2021

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 15, 2021

k8s-ci-robot assigned enj Sep 27, 2021

enj moved this from Needs Triage to In Review in SIG Auth Old Sep 27, 2021

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 27, 2021

enj suggested changes Sep 27, 2021

View reviewed changes

SIG Auth Old automation moved this from In Review to Changes Requested Sep 27, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 27, 2021

chenchun force-pushed the fake branch 2 times, most recently from 79bf20b to 9c03ae4 Compare October 9, 2021 06:16

k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Oct 12, 2021

enj suggested changes Oct 27, 2021

View reviewed changes

chenchun force-pushed the fake branch from 39ce0e0 to 3049217 Compare October 28, 2021 12:11

enj suggested changes Oct 28, 2021

View reviewed changes

pkg/controller/bootstrap/tokencleaner_test.go Outdated Show resolved Hide resolved

pkg/controller/serviceaccount/tokens_controller_test.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned liggitt Oct 28, 2021

chenchun force-pushed the fake branch from 3049217 to beea316 Compare October 29, 2021 01:27

liggitt reviewed Nov 1, 2021

View reviewed changes

pkg/controller/bootstrap/common_test.go Outdated Show resolved Hide resolved

chenchun and others added 2 commits November 2, 2021 10:04

Pass DeleteOptions down to the Reactor

6219704

Co-authored-by: Mo Khan <theenjeru@gmail.com>

Auto generated

888c049

chenchun force-pushed the fake branch from beea316 to 888c049 Compare November 2, 2021 02:05

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 2, 2021

k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Nov 2, 2021

k8s-ci-robot added this to the v1.23 milestone Nov 2, 2021

k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Nov 2, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 2, 2021

k8s-ci-robot merged commit 9be6780 into kubernetes:master Nov 2, 2021

SIG Auth Old automation moved this from Changes Requested to Closed / Done Nov 2, 2021

chenchun deleted the fake branch November 3, 2021 01:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass DeleteOptions down to the Reactor #102945

Pass DeleteOptions down to the Reactor #102945

chenchun commented Jun 17, 2021 •

edited by liggitt

k8s-ci-robot commented Jun 17, 2021

fedebongio commented Jun 17, 2021

k8s-triage-robot commented Sep 15, 2021

enj commented Sep 27, 2021

enj commented Sep 27, 2021

enj Sep 27, 2021

enj commented Sep 27, 2021

chenchun commented Oct 12, 2021

enj left a comment

enj left a comment

enj commented Oct 28, 2021

chenchun commented Oct 29, 2021

liggitt left a comment

enj commented Nov 2, 2021

enj commented Nov 2, 2021

liggitt commented Nov 2, 2021

k8s-ci-robot commented Nov 2, 2021

		$if .namespaced$Invokes($.NewDeleteAction\|raw$($.type\|allLowercasePlural$Resource, c.ns, name, &opts), &$.type\|raw${})
		$else$Invokes($.NewRootDeleteAction\|raw$($.type\|allLowercasePlural$Resource, name, &opts), &$.type\|raw${})$end$

Pass DeleteOptions down to the Reactor #102945

Pass DeleteOptions down to the Reactor #102945

Conversation

chenchun commented Jun 17, 2021 • edited by liggitt

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Jun 17, 2021

fedebongio commented Jun 17, 2021

k8s-triage-robot commented Sep 15, 2021

enj commented Sep 27, 2021

enj commented Sep 27, 2021

enj Sep 27, 2021

Choose a reason for hiding this comment

enj commented Sep 27, 2021

chenchun commented Oct 12, 2021

enj left a comment

Choose a reason for hiding this comment

enj left a comment

Choose a reason for hiding this comment

enj commented Oct 28, 2021

chenchun commented Oct 29, 2021

liggitt left a comment

Choose a reason for hiding this comment

enj commented Nov 2, 2021

enj commented Nov 2, 2021

liggitt commented Nov 2, 2021

k8s-ci-robot commented Nov 2, 2021

chenchun commented Jun 17, 2021 •

edited by liggitt