fix(trait): watch for resource versions...

[squakez]

...instead of for their content.

With this PR we fix a potentially risky behavior, as we were inspecting Configmap/Secrets content to address any change. Instead of looking directly the content, we watch for the changing resource version, which should be the way provided by the platform to know when a resource has changed without inspecting its content.

Release Note

fix(trait): watch for resource versions

[lburgazzoli]

Note that resourceVersion changes also in case of any of the metadata changes

[squakez]

Note that resourceVersion changes also in case of any of the metadata changes

Thanks. Not sure if there is any better way to deal with this, any idea? At least we remove the inspection of a secret, with the tradeoff to potentially reload an Integration when there is a metadata change as well.

[lburgazzoli]

Note that resourceVersion changes also in case of any of the metadata changes

Thanks. Not sure if there is any better way to deal with this, any idea? At least we remove the inspection of a secret, with the tradeoff to potentially reload an Integration when there is a metadata change as well.

I don't think there is a better solution so is a matter of document the behavior.

Maybe, should we honor the mount.hot-reload option ? In some cases, the user may not have full control of the configmap/secrets i.e. when those resources are generated by another controller.

[squakez]

Maybe, should we honor the mount.hot-reload option ? In some cases, the user may not have full control of the configmap/secrets i.e. when those resources are generated by another controller.

The option is already used in the controller to wake up the Integration and kick the logic:

camel-k/pkg/controller/integration/integration_controller.go

Line 191 in 825f0ce

if integration.Spec.Traits.Mount == nil || !pointer.BoolDeref(integration.Spec.Traits.Mount.HotReload, false) {

- do you think we need to include this again in the getIntegrationSecretAndConfigmapResourceVersions func? I think it may be a duplicated check, but definetely it's something that should not hurt.

[lburgazzoli]

Maybe, should we honor the mount.hot-reload option ? In some cases, the user may not have full control of the configmap/secrets i.e. when those resources are generated by another controller.

The option is already used in the controller to wake up the Integration and kick the logic:

camel-k/pkg/controller/integration/integration_controller.go

Line 191 in 825f0ce

if integration.Spec.Traits.Mount == nil || !pointer.BoolDeref(integration.Spec.Traits.Mount.HotReload, false) {

• do you think we need to include this again in the getIntegrationSecretAndConfigmapResourceVersions func? I think it may be a duplicated check, but definetely it's something that should not hurt.

I don't have the full picture but my reasoning is that, the hot reload flag prevents a reconciliation to happen but, in case the reconciliation is triggered by i.e. a non destructive trait change, then we ends up in the same case as #5192, which effectively will end up in a restart/reload.

That said, this seems to be the case as today anyhow so not sure if we want to fix it now or have some better reasoning as part of #5192,

[squakez]

@lburgazzoli I've added a check on the presence of the hot-reload flag and a short documentation notice to warn about the behavior. Thanks.

[squakez]

Check failure relates to #5197