Conversation
jakedt
left a comment
jakedt
left a comment
There was a problem hiding this comment.
I think we should invert the model and just filter out events for which the only thing that changed were things in the status subresource. As it stands right now you're going to filter out things that the user will expect to be able to change, and you're going to filter out reconciliations on downstream resources, e.g. jobs.
kleesc
force-pushed
the
subresource-event-filter-predicate
branch
from
ffacfa1 to
e0b8cfa
Compare
Updated to filter only on ManagedDatabases generation change. Other types will update when resourceVersion is changed. Doesn't look like Jobs were using the generation field anyways... So I'm guessing we can't just make the assumption that an update to the status of any object type will increment its generation. |
jakedt
left a comment
jakedt
left a comment
There was a problem hiding this comment.
I need to think about the event filter some more. There are a lot of cases where we want to run a reconciliation that I think this will filter. For example, we will get awakened when the job or secret changes, but the underlying mdb will not have changed, we want to run reconciliation and I think this will filter those cases. Rather than filter, can we just focus on converging by making the status blocks state dependent and reproduceable?
| oneMigration.log.Info("Migration job is complete") | ||
|
|
||
| // TODO: should we write the metric here or wait until cleanup? | ||
| } else if job.Status.Active == 0 && job.Status.Conditions != nil { |
There was a problem hiding this comment.
Can you break just this part out into a separate PR.
kleesc
force-pushed
the
subresource-event-filter-predicate
branch
from
e0b8cfa to
7804ad6
Compare
3 participants
Add event filter for
ManagedDatabaseto ignore updates to subresources unless it's theManagedDatabase's currentVersion, in which case we want to reconcile the next migration.