🐛 Prevent source.Channel from shutting down immediately #1345
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @charith-elastic. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. |
k8s-ci-robot
added
the
needs-ok-to-test
k8s-ci-robot
added
the
size/M
| watchStartCtx, cancel := context.WithTimeout(ctx, c.CacheSyncTimeout) | ||
| defer cancel() | ||
| if err := watch.src.Start(watchStartCtx, watch.handler, c.Queue, watch.predicates...); err != nil { | ||
| if err := watch.src.Start(ctx, watch.handler, c.Queue, watch.predicates...); err != nil { |
There was a problem hiding this comment.
Unfortunately, what we do today is the correct behaviour for source.Kind, it has a different interpretation of the purpose of its ctx argument. Let's merge this as source.Channel doesn't work at all currently, I will open a n issue so we can clarify our interface definitions and update one or the other accordingly
There was a problem hiding this comment.
(This change will break the waitForSync functionality I believe)
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, charith-elastic 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 |
k8s-ci-robot
added
the
approved
We need kubernetes-sigs/controller-runtime#1345, otherwise souce.Channel doesn't do anything which renders the promotion_job_creator useless as it never gets triggered.
Because of an incorrectly used context,
source.Channelgets shutdown almost immediately before it gets a chance to process any events. More details can be found in #1343.Also fixes a minor bug in
source.Channelwhere the destination append operation wasn't protected by a lock as it used to.Fixes #1343