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

[v1.16.1] TokenCleaner#evalSecret should enqueue the key #82887

Merged
merged 1 commit into from Sep 19, 2019

Conversation

@tedyu
Copy link
Contributor

tedyu commented Sep 19, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PR fixes the panic reported in #82854

runtime.go:78] Observed a panic: &runtime.TypeAssertionError{_interface:(*runtime._type)(0x38d8420), concrete:(*runtime._type)(0x4171300), asserted:(*runtime._type)(0x371f240), missingMethod:""} (interface conversion: interface {} is *v1.Secret, not string)

Which issue(s) this PR fixes:
xref #82854

Fixes a panic in kube-controller-manager cleaning up bootstrap tokens

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


@liggitt liggitt added this to the v1.16 milestone Sep 19, 2019
@liggitt liggitt changed the title TokenCleaner#processNextWorkItem should check the type of object retrieved from queue [v1.16.1] TokenCleaner#processNextWorkItem should check the type of object retrieved from queue Sep 19, 2019
@yutedz yutedz force-pushed the yutedz:token-cleaner-type branch from 03e7695 to c8f19d7 Sep 19, 2019
@tedyu tedyu changed the title [v1.16.1] TokenCleaner#processNextWorkItem should check the type of object retrieved from queue [v1.16.1] TokenCleaner#evalSecret should enqueue the key Sep 19, 2019
@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Sep 19, 2019

this change will exercise the requeue code:

diff --git a/pkg/controller/bootstrap/tokencleaner_test.go b/pkg/controller/bootstrap/tokencleaner_test.go
index 6eea6321ba0..df56ea8a86c 100644
--- a/pkg/controller/bootstrap/tokencleaner_test.go
+++ b/pkg/controller/bootstrap/tokencleaner_test.go
@@ -110,10 +110,11 @@ func TestCleanerExpiredAt(t *testing.T) {

        secret := newTokenSecret("tokenID", "tokenSecret")
        addSecretExpiration(secret, timeString(2*time.Second))
+       secrets.Informer().GetIndexer().Add(secret)
+       cleaner.enqueueSecrets(secret)
        expected := []core.Action{}
        verifyFunc := func() {
-               secrets.Informer().GetIndexer().Add(secret)
-               cleaner.evalSecret(secret)
+               cleaner.processNextWorkItem()
                verifyActions(t, expected, cl.Actions())
        }
        // token has not expired currently
@yutedz yutedz force-pushed the yutedz:token-cleaner-type branch from c8f19d7 to 1269b9e Sep 19, 2019
@k8s-ci-robot k8s-ci-robot added size/S and removed size/XS labels Sep 19, 2019
@tedyu

This comment has been minimized.

Copy link
Contributor Author

tedyu commented Sep 19, 2019

Thanks for the test change suggestion, @liggitt

Please take another look.

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Sep 19, 2019

/lgtm
/approve

go ahead and open the cherry-pick to release-1.16

@k8s-ci-robot k8s-ci-robot added the lgtm label Sep 19, 2019
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Sep 19, 2019

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@tedyu

This comment has been minimized.

Copy link
Contributor Author

tedyu commented Sep 19, 2019

#82888 had too many changes.
I used this command:

GITHUB_USER=yutedz hack/cherry_pick_pull.sh upstream/release-1.16 82887

@tedyu

This comment has been minimized.

Copy link
Contributor Author

tedyu commented Sep 19, 2019

/test pull-kubernetes-integration

@liggitt liggitt removed the sig/apps label Sep 19, 2019
@k8s-ci-robot k8s-ci-robot merged commit 7350505 into kubernetes:master Sep 19, 2019
25 checks passed
25 checks passed
cla/linuxfoundation yutedz authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-conformance-kind-ipv6 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-alpha-features Skipped.
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
@k8s-ci-robot k8s-ci-robot modified the milestones: v1.16, v1.17 Sep 19, 2019
k8s-ci-robot added a commit that referenced this pull request Sep 27, 2019
…7-upstream-release-1.16

[v1.16.1] Automated cherry pick of #82887: TokenCleaner#evalSecret should enqueue the key
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.