filewatch: use apiserver FileWatch model in EngineState

[milas]

This follows the "action-first" approach to use the apiserver model
for FileWatch and dispatch simple create/update/delete actions.

I didn't touch the git manager, so some things that are no longer
needed for normal file watching but it relies on were moved over
there.

This is actually pretty similar to the previous implementation since
file watch isn't really its own resource/concept in the current design.
It's a bit incremental of the incremental - the next step would be to
remove the diffing logic and instead just react to changes to the
FileWatchSpecs within the watch manager (which would then just
the authority on FileWatchStatus) and instead push the onus for
actually creating/updating specs to someone else.

[nicks]

whew! Ya, i'm glad we tried to do these in parallel, since i'm starting to see the common themes (rather than wondering if something was particiular to Cmd)

[nicks]

ah, i hit this problem too...

the approach to this that i've been taking is that, when a subscriber sends a Create/Update, it should keep a local map of the last create/updated object, and then use that map to make create/update idempotent

I think the problem with doing it this way is that you're going to run into weird circular data flow issues where this code is reading and writing the same data....

[milas]

Ha! This is iteration 3, but I should probably just go back to the original. First, I had totally duplicated copies of the full FileWatch both places as you're suggesting. A second (short-lived) version was to rely entirely on the store to avoid duplicating here, which obviously did not pan out. I agree this final approach is problematic - I was trying to get closer to the K8s model somewhat (i.e. pretend that it's level-triggered).

I'll flip back to that - I think the next big experiment that needs to be done is shifting responsibility for the creation of the FileWatch to the resource to move towards our desired model, at which point, hopefully this becomes much more like a real reconciler, at which point it should just read specs to trigger FS watchers, and let the FS watchers do status updates in the background. At that point, shouldn't be any circular weirdness 😵

[nicks]

ya, i had a similar experience - it's simpler if the controller that does create/delete is separate from the controller that does UpdateStatus. i think that having the ChangeSummary stuff to tell the controller what changed will also help with this

[nicks]

should we reset the status when the spec changes? (I honestly don't know, this is something I struggled with too)

[milas]

I've gone back and forth as well (I think the other PR resets it 😬) - I'm guessing it's best to standardize on resetting for consistency?

Here, while there could be value in still being able to see the previous changesets, it's also actually misleading because if you add a new file to the watch paths, it of course won't get retroactively added to those events.

I think one thing that FileWatchStatus is missing is probably more actual status about the filesystem watcher. So perhaps WatchStartTime which gets updated whenever the filesystem watcher is started (or "updated"); that gives consumers another value to use against their own internal watermark, e.g. if (myLastUpdated.Before(fwStatus.LastEventTime) || myLastUpdated.Before(fwStatus.WatchStartTime); that would allow e.g. easily triggering a rebuild when the watch patterns are changed (if desired behavior).