Reconcile utils new alg

[shawkins]

My continued appologies, I'm not having a good day. Via the IDE I still ended up targeting upstream, instead of origin.

That should be cleanup up now. Here's even further refinements to make clearer why I want to differentate the behavior.

[shawkins]

This is a foundamental question with the new design, that if there is no explicit event filtering, we should propagate the new event for our update or more recent (larger resource version). So this should be false based on that.

@csviri regardless of event filtering my expectation is that is something is added to the cache, it is something that we don't then care to get an event about.

I've updated things here to leave it more up to the caller to decide what to do in each of the cases.

Could you provide an example where someone adds a version to the temporary resource cache, but still wants that event?

[csviri]

This is a foundamental question with the new design, that if there is no explicit event filtering, we should propagate the new event for our update or more recent (larger resource version). So this should be false based on that.

@csviri regardless of event filtering my expectation is that is something is added to the cache, it is something that we don't then care to get an event about.

I've updated things here to leave it more up to the caller to decide what to do in each of the cases.

Hi @shawkins thx for the PR, tests are failing, can take a look a bit later more in depth into this PR.

Could you provide an example where someone adds a version to the temporary resource cache, but still wants that event?

Yeah, this is up to the discussion, my original idea was to provide a backwards compatible way for the case if some implemented a controller, that at the end of the reconciliation call UpdateControl.patchResource and the event resulted from that patch is triggered the reconiliation - as a I framework I think we cannot make an assumption in general that no one is expecting that it will trigger the reconiliation. So wanted to have a way to have still the guarantee that up to date resources is in cache always, but we want to trigger an event. Therefore I added this option:

java-operator-sdk/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java

Lines 20 to 31 in 793a56d

	private boolean filterPatchEvent = true;
	/**
	* The event from resource primary updates are filtered by default, thus does not trigger the
	* reconciliation. Setting this to false will turn the filtering off.
	*
	* @since 5.3.0
	*/
	public T filterPatchEvent(boolean filterPatchEvent) {
	this.filterPatchEvent = filterPatchEvent;
	return (T) this;
	}

And we pass that flag also to in the reconciler for the utils, like here:

java-operator-sdk/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

Line 395 in 793a56d

return ReconcileUtils.serverSideApplyPrimary(context, resource, filterEvent);

However, now I'm thinking - and especially if this makes the implementation simlper - that we could rather just simply have a reSchedule() method without time delay here:

java-operator-sdk/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java

Line 26 in 793a56d

public T rescheduleAfter(long delay) {

So we will produce that event explicitly in the event processor after reconciliation. I would acutally go rather with this option (will update the PR), and let you optimize filtering / caching part as in this PR, if that works for you.

thank you!

[csviri]

see: 00f3245

[csviri]

Could you provide an example where someone adds a version to the temporary resource cache, but still wants that event?

Ahh there is one more place, when we adding a finalizer we want to receive an event (and having an up-to-date resource) but we can also have there an explicit additional event.

[shawkins]

Hi @shawkins thx for the PR, tests are failing, can take a look a bit later more in depth into this PR.

Sure, I'll have a look.

I would acutally go rather with this option (will update the PR), and let you optimize filtering / caching part as in this PR, if that works for you.

An immediate re-reconciliation makes sense, and gives more control to the calling logic.