Skip to content

Add finalizers for RBAC issue#2646

Closed
grac3gao-zz wants to merge 1 commit into
knative:masterfrom
grac3gao-zz:finalizers
Closed

Add finalizers for RBAC issue#2646
grac3gao-zz wants to merge 1 commit into
knative:masterfrom
grac3gao-zz:finalizers

Conversation

@grac3gao-zz
Copy link
Copy Markdown
Contributor

Fixes #2642

Proposed Changes

  • Add finalizers for RBAC issue with configmappropagations

Release Note


Docs

@knative-prow-robot knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 25, 2020
@grac3gao-zz grac3gao-zz requested a review from grantr February 25, 2020 17:48
@matzew
Copy link
Copy Markdown
Member

matzew commented Feb 25, 2020

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 25, 2020
@matzew
Copy link
Copy Markdown
Member

matzew commented Feb 25, 2020

@grac3gao I've already tried that earlier, by directly editing the ClusterRole on my cluster

k get clusterrole knative-eventing-channel-broker-controller -oyaml

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  annotations:
    manifestival: new
  creationTimestamp: "2020-02-25T13:55:35Z"
  labels:
    eventing.knative.dev/release: v20200225-066b68b7
  name: knative-eventing-channel-broker-controller
  resourceVersion: "242055"
  selfLink: /apis/rbac.authorization.k8s.io/v1/clusterroles/knative-eventing-channel-broker-controller
  uid: b32f69d5-13d8-4c12-9f8a-8a153c26f72a
rules:
- apiGroups:
  - configs.internal.knative.dev
  resources:
  - configmappropagations
  - configmappropagations/status
  verbs:
  - get
  - list
  - create
  - update
  - delete
  - patch
  - watch
- apiGroups:
  - configs.internal.knative.dev
  resources:
  - configmappropagations/finalizers
  verbs:
  - update

@matzew
Copy link
Copy Markdown
Member

matzew commented Feb 25, 2020

I did create a new namespace, and did than label it:

{
  "level": "info",
  "ts": "2020-02-25T18:20:59.984Z",
  "logger": "controller.knative-eventing-namespace-controller",
  "caller": "controller/controller.go:403",
  "msg": "Reconcile succeeded. Time taken: 31.704µs.",
  "commit": "066b68b",
  "knative.dev/controller": "knative-eventing-namespace-controller",
  "knative.dev/traceid": "1dc3a971-b43b-4474-ac9e-f86ae8653490",
  "knative.dev/key": "test"
}
{
  "level": "info",
  "ts": "2020-02-25T18:21:00.017Z",
  "logger": "controller.knative-eventing-namespace-controller",
  "caller": "controller/controller.go:403",
  "msg": "Reconcile succeeded. Time taken: 33.403µs.",
  "commit": "066b68b",
  "knative.dev/controller": "knative-eventing-namespace-controller",
  "knative.dev/traceid": "2f5a1567-84ae-42a4-9beb-2ca13f3dddae",
  "knative.dev/key": "test"
}
{
  "level": "error",
  "ts": "2020-02-25T18:21:09.624Z",
  "logger": "controller.knative-eventing-namespace-controller",
  "caller": "namespace/namespace.go:113",
  "msg": "Error reconciling Namespace",
  "commit": "066b68b",
  "knative.dev/controller": "knative-eventing-namespace-controller",
  "knative.dev/traceid": "ab57af22-2607-41c6-aded-3871d32d63ea",
  "knative.dev/key": "test",
  "error": "configMapPropagation: configmappropagations.configs.internal.knative.dev \"eventing\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>",
  "stacktrace": "knative.dev/eventing/pkg/reconciler/namespace.(*Reconciler).Reconcile\n\tknative.dev/eventing/pkg/reconciler/namespace/namespace.go:113\nknative.dev/eventing/vendor/knative.dev/pkg/controller.(*Impl).processNextWorkItem\n\tknative.dev/eventing/vendor/knative.dev/pkg/controller/controller.go:394\nknative.dev/eventing/vendor/knative.dev/pkg/controller.(*Impl).Run.func2\n\tknative.dev/eventing/vendor/knative.dev/pkg/controller/controller.go:343"
}
{
  "level": "error",
  "ts": "2020-02-25T18:21:09.624Z",
  "logger": "controller.knative-eventing-namespace-controller",
  "caller": "controller/controller.go:409",
  "msg": "Reconcile error",
  "commit": "066b68b",
  "knative.dev/controller": "knative-eventing-namespace-controller",
  "error": "configMapPropagation: configmappropagations.configs.internal.knative.dev \"eventing\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>",
  "stacktrace": "knative.dev/eventing/vendor/knative.dev/pkg/controller.(*Impl).handleErr\n\tknative.dev/eventing/vendor/knative.dev/pkg/controller/controller.go:409\nknative.dev/eventing/vendor/knative.dev/pkg/controller.(*Impl).processNextWorkItem\n\tknative.dev/eventing/vendor/knative.dev/pkg/controller/controller.go:395\nknative.dev/eventing/vendor/knative.dev/pkg/controller.(*Impl).Run.func2\n\tknative.dev/eventing/vendor/knative.dev/pkg/controller/controller.go:343"
}
{
  "level": "info",
  "ts": "2020-02-25T18:21:09.624Z",
  "logger": "controller.knative-eventing-namespace-controller",
  "caller": "controller/controller.go:396",
  "msg": "Reconcile failed. Time taken: 107.081428ms.",
  "commit": "066b68b",
  "knative.dev/controller": "knative-eventing-namespace-controller",
  "knative.dev/traceid": "ab57af22-2607-41c6-aded-3871d32d63ea",
  "knative.dev/key": "test"
}
{
  "level": "info",
  "ts": "2020-02-25T18:21:09.624Z",
  "logger": "controller.knative-eventing-namespace-controller.event-broadcaster",
  "caller": "record/event.go:274",
  "msg": "Event(v1.ObjectReference{Kind:\"Namespace\", Namespace:\"\", Name:\"test\", UID:\"ba7cfd53-6a61-4d6d-bc5b-8def1da980c9\", APIVersion:\"v1\", ResourceVersion:\"244342\", FieldPath:\"\"}): type: 'Warning' reason: 'NamespaceReconcileFailure' Failed to reconcile Namespace: configMapPropagation: configmappropagations.configs.internal.knative.dev \"eventing\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>",
  "commit": "066b68b",
  "knative.dev/controller": "knative-eventing-namespace-controller"
}
...

@matzew
Copy link
Copy Markdown
Member

matzew commented Feb 25, 2020

@grac3gao I guess we need more, this comment from @grantr made me wonder, what's missing:

#2519 (comment)

This should include all permissions the broker controller needs.

@matzew
Copy link
Copy Markdown
Member

matzew commented Feb 26, 2020

/close

replaced by #2653

@knative-prow-robot
Copy link
Copy Markdown
Contributor

@matzew: Closed this PR.

Details

In response to this:

/close

replaced by #2653

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.

@grac3gao-zz grac3gao-zz deleted the finalizers branch March 25, 2020 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RBAC issue with configmappropagations

4 participants