generic image change trigger doesn't reconcile on resource changes

[tnozicka]

If I have a DC with ImageChangeTrigger upon creation it will have the image resolved and rewritten by the controller. But when I manually patch the DC image after to be "" it won't get reconciled and resolved.

I would expect it to reconcile the image value by resolving the trigger.

[tnozicka]

originates here: #17404 and https://bugzilla.redhat.com/show_bug.cgi?id=1515902

[bparees]

@tnozicka your team owns deploymentconfigs. that would include their imagechangetrigger behavior.

[tnozicka]

@bparees I though this generic trigger controller is across resources and just injects resolved image into whatever resource that is.

[tnozicka]

we got rid of our own (DC) trigger controller in favour of this generic one I believe

[bparees]

1. no one owns that controller, so if you need changes in it, you need to make them.
2. that controller watches for imagestreams that change and pokes the affected resource. It does not watch for specific resources to change. That would be the responsibility of your DC config change trigger.

[mfojtik]

@bparees that controller watching image streams / images and updating different resources sounds like you own that controller as it serves as interface for triggering based on image changes? :-)

@tnozicka so the problem here is the trigger controller won't re-populate the image field after you change it manually? Why would you change that field manually when it is controlled by the controller?

To me that looks like an error case when you patched it by a mistake. In that case oc rollout latest should be able to fix you?

[tnozicka]

@tnozicka so the problem here is the trigger controller won't re-populate the image field after you change it manually?

Yes, it doesn't and I believe it should reconcile the state you declared. Image comes from image stream; if it doesn't overwrite it so the state corresponds to what you've declared.

Why would you change that field manually when it is controlled by the controller?

e.g. oc apply does (because the field is "" at creation) that but it should get reconciled either way

It also leaves the DC in bad state and we already have 1 GH issue and 1 BZ for 3.7, my vote is for P1.

[tnozicka]

/cc @smarterclayton

[smarterclayton]

Before, resolution was done by the generic DC trigger controller. It sounds like when that was removed we missed fixing the trigger controller to resolve?

Would be helpful to verify behavior before and after trigger controller was removed. Also, surprised we don’t have an e2e test verifying this.

[smarterclayton]

Was this an actual regression?

[smarterclayton]

Ie are we sure we are not compatible with how DC have always worked?

[bparees]

@bparees that controller watching image streams / images and updating different resources sounds like you own that controller as it serves as interface for triggering based on image changes? :-)

@mfojtik Everything that's triggered by an image change has to contribute its own trigger/reactor, devex is not going to own those triggers/reactors for every resource in openshift. If you can show me the trigger isn't even being invoked when the image changes, i'll consider us owning it, otherwise I don't.

[tnozicka]

3.6 is not working either but it syncs on next DC edit. I've found out that 3.7 syncs on master restart.

I think I see why it is not working in DC reactor. @bparees I am fine taking it but maybe since you say we own the reactor, that should move to /pkg/apps? @mfojtik WDYT?

/assign @tnozicka
/unassign @mfojtik