Set (cluster)bus.Kind ourselves #418
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: greghaynes If they are not already assigned, you can assign the PR to them by writing 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 |
knative-prow-robot
added
the
size/XS
| func (r *Reconciler) createOrUpdateBus(bus *channelsv1alpha1.Bus) error { | ||
| if r.bus.GetObjectKind().GroupVersionKind().Kind != bus.Kind || | ||
| bus.Namespace != r.bus.GetObjectMeta().GetNamespace() || | ||
| if bus.Namespace != r.bus.GetObjectMeta().GetNamespace() || |
There was a problem hiding this comment.
the Kind needs to be checked because r.bus is a GenericBus and may not be a Bus. See #298
greghaynes
force-pushed
the
fix/bus-update-reconciler
branch
from
7e37545 to
9673572
Compare
This property is not set in the new version of a bus when UpdateFunc is called from an informer. Fixes knative#417
greghaynes
force-pushed
the
fix/bus-update-reconciler
branch
from
9673572 to
0fb7ef9
Compare
greghaynes
changed the title
| if r.bus.GetObjectKind().GroupVersionKind().Kind != clusterBus.Kind || | ||
| clusterBus.Name != r.bus.GetObjectMeta().GetName() { | ||
| // this is not our clusterbus | ||
| r.logger.Infof("Ignoring update for bus (%v/%v) that is not ours.", clusterBus.Namespace, clusterBus.Name) |
There was a problem hiding this comment.
ClusterBuses don't have a namespace.
I've been trying to normalize logging references to resources by using the respective reference.
%s is more idiomatic for string values.
- only start the needed informer for either Bus or ClusterBus, we don't need both - consolidate the createOrUpdateBus and createOrUpdateClusterBus methods - use BusReference to check if the updated bus is the reconciled bus Refs knative#418
|
I'm generally 👍 on this PR as it unblocks meaningful bus updates. I attempted to resolve the root issue with #420. |
|
I'm fine waiting for #420 as its a much nicer fix. |
* Cleanup Bus/ClusterBus reconciler - only start the needed informer for either Bus or ClusterBus, we don't need both - consolidate the createOrUpdateBus and createOrUpdateClusterBus methods - use BusReference to check if the updated bus is the reconciled bus Refs #418 * Remove redudant BusReference passed to Reconciler#Run() * Create copy of Bus before calling createOrUpdateBus * Review feedback * Clarify that the workqueue key is
…perator Latest of the serving Operator
Also make sure that a dead letter sink is resolved for channel before the source actually sends the event (moving this check from Requirement phase to Setup).
3 participants
Proposed Changes
This property is not set in the new version of a bus when UpdateFunc is
called from an informer.
Fixes #417
Release Note