Avoid replacing the monitor's bus with the wrong bus#298
Conversation
For a namespaces bus, if there is a cluster bus installed with the same name, the cluster bus will be captured in the monitor and used to check if a channel is for that bus. This effectively causes channels to be provisioned by the wrong bus, or not at all. Before replacing the bus reference for the monitor, we need to check the kind, name and namespace. Previously, only the name and namespace where checked.
google-prow-robot
added
the
size/S
|
|
||
| func (m *Monitor) createOrUpdateClusterBus(clusterBus *channelsv1alpha1.ClusterBus) error { | ||
| if clusterBus.Name != m.bus.GetObjectMeta().GetName() { | ||
| if m.bus.GetObjectKind().GroupVersionKind().Kind != clusterBus.Kind || |
There was a problem hiding this comment.
Does this mean we were sometimes passing a ClusterBus to createOrUpdateBus and vice-versa?
There was a problem hiding this comment.
createOrUpdateClusterBus will only receive a ClusterBus and createOrUpdateBus will only receive a Bus. The issue is that they both assign to m.bus which is a GenericBus interface that can be either.
The monitor is listening for Bus/ClusterBus so that it can receive updated parameters for channels and subscriptions. Realistically, it should only ever need to listen for either Buses or ClusterBuses, never both.
There was a problem hiding this comment.
Oh, this is a method on the monitor; I keep forgetting that these controllers aren't stateless (and each Monitor will see all the Bus & ClusterBus updates and choose which ones apply to its stored bus).
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: evankanderson, scothis The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
google-prow-robot
added
the
approved
|
/test pull-knative-eventing-integration-tests |
4 similar comments
|
/test pull-knative-eventing-integration-tests |
|
/test pull-knative-eventing-integration-tests |
|
/test pull-knative-eventing-integration-tests |
|
/test pull-knative-eventing-integration-tests |
For a namespaces bus, if there is a cluster bus installed with the same name, the cluster bus will be captured in the monitor and used to check if a channel is for that bus. This effectively causes channels to be provisioned by the wrong bus, or not at all. Before replacing the bus reference for the monitor, we need to check the kind, name and namespace. Previously, only the name and namespace where checked.
3 participants
For a namespaces bus, if there is a cluster bus installed with the same
name, the cluster bus will be captured in the monitor and used to check
if a channel is for that bus. This effectively causes channels to be
provisioned by the wrong bus, or not at all.
Before replacing the bus reference for the monitor, we need to check the
kind, name and namespace. Previously, only the name and namespace where
checked.