[2100] Reconcile a namespace again with missing rolebinding #2101
Conversation
tekton-robot
added
the
release-note
tekton-robot
added
the
needs-ok-to-test
|
Hi @jkhelil. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
tekton-robot
added
the
size/XS
jkhelil
changed the title
|
/ok-to-test |
tekton-robot
added
ok-to-test
| logger.Infof("RoleBinding %s is missing in namespace: %s, will reconcile again the namespace %s", pipelinesSCCRoleBinding, ns.Name, ns.Name) | ||
| namespaces = append(namespaces, ns) | ||
| continue | ||
| } | ||
| return nil, err |
There was a problem hiding this comment.
can we custumize this error a bit also with error fetching rolebinding name from namespace: err
There was a problem hiding this comment.
yes done thanks @piyush-garg
There was a problem hiding this comment.
I think I was not clear, error message in the logger was fine, i am saying about this for the error we are returning to the end
There was a problem hiding this comment.
If I understand correctly, you are asking to print the error message with the logger on line number 271, then return nil, err.
Am I correct @piyush-garg ?
There was a problem hiding this comment.
I think @piyush-garg is asking to wrap the error, done now
There was a problem hiding this comment.
yes to wrap the error and return, not logger
| // https://github.com/tektoncd/operator/issues/2100 | ||
| // Reconcile a namespace again with missing RoleBinding | ||
| if errors.IsNotFound(err) { | ||
| logger.Infof("RoleBinding %s is missing in namespace: %s, will reconcile again the namespace %s", pipelinesSCCRoleBinding, ns.Name, ns.Name) |
There was a problem hiding this comment.
SHould we put this into debug
tekton-robot
added
the
approved
|
we should also cherry pick this to older releases |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: concaf, jkandasa 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 |
jkhelil
force-pushed
the
SRVKP-4268
branch
2 times, most recently
from
8b3d10f
to
09915b6
Compare
|
/lgtm |
|
/cherry-pick release-v0.70.x |
|
@khrm: new pull request created: #2113 In response to this:
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. |
|
/cherry-pick release-v0.69.x |
|
@jkandasa: new pull request created: #2117 In response to this:
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. |
Changes
fixess #2100
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make test lintbefore submitting a PRSee the contribution guide for more details.
Release Notes
None