fix(trait): revert persisted status

[squakez]

The work done in #5153 is based on the wrong assumption that a trait status can be persisted at any point of the Integration/IntegrationKit lifecycle This cannot be always true, because the resource move towards different .status.phase with the execution of its logic or not depending on the phase. Therefore we cannot persist in a consistent way, as the status would reflect the latest execution of a trait which may happen in a different phase (and would be overridden by the new execution phase). The best way to deal with this, at this stage, seems to be the usage of .status.conditions in order to warn that some trait value was overwritten.

Release Note

fix(trait): revert persisted status

[lburgazzoli]

The best way to deal with this, at this stage, seems to be the usage of .status.conditions in order to warn that some trait value was overwritten.

In general, I would recommend not to rely on conditions as those are primarily designed to be consumed by observers rather than the operator itself. A golden rule of controller is that, with some very little exceptions, the status sub resource should not be used to determine the current state.

[squakez]

The best way to deal with this, at this stage, seems to be the usage of .status.conditions in order to warn that some trait value was overwritten.

In general, I would recommend not to rely on conditions as those are primarily designed to be consumed by observers rather than the operator itself. A golden rule of controller is that, with some very little exceptions, the status sub resource should not be used to determine the current state.

Yes. The point for the consume of conditions was more directed to a human observer that wants to understand why, for instance, an Quarkus native Integration is instructing the Kit to use 4GB of memory (which is the sensible default configuration the operator overrides).

[lburgazzoli]

Yes. The point for the consume of conditions was more directed to a human observer that wants to understand why, for instance, an Quarkus native Integration is instructing the Kit to use 4GB of memory (which is the sensible default configuration the operator overrides).

should that be part of the kit condition/status then ?

[squakez]

should that be part of the kit condition/status then ?

I think it is already happening. We stores conditions of certain traits overridden properties and deprecation notices. It may be polished a little bit (see #5027) but the logic to store them is already there.

[lburgazzoli]

should that be part of the kit condition/status then ?

I think it is already happening. We stores conditions of certain traits overridden properties and deprecation notices. It may be polished a little bit (see #5027) but the logic to store them is already there.

so I'm a little bit confusing about what is the .status.conditions we are talking about here :)

[squakez]

so I'm a little bit confusing about what is the .status.conditions we are talking about here :)

NP. Let me try to summarize in shorter terms. .status.conditions already exists and it is meant to provide some info about possible info/warnings the user needs to know. Sometime ago I introduced another field, .status.traits whose goal was to store the executed traits in order to make the operator logic more consistent. However, this development was made based on the wrong assumptions provided in this PR description. The aim of this PR is to remove .status.traits which is inconsistent. Any user interested in troubleshooting an Integration can still rely on .status.conditions as it was doing previously.

[lburgazzoli]

so I'm a little bit confusing about what is the .status.conditions we are talking about here :)

NP. Let me try to summarize in shorter terms. .status.conditions already exists and it is meant to provide some info about possible info/warnings the user needs to know. Sometime ago I introduced another field, .status.traits whose goal was to store the executed traits in order to make the operator logic more consistent. However, this development was made based on the wrong assumptions provided in this PR description. The aim of this PR is to remove .status.traits which is inconsistent. Any user interested in troubleshooting an Integration can still rely on .status.conditions as it was doing previously.

ok then, I was a little bit confused because the .status.traits has been the center of a number of things recently, like checksum, copy to kit, etc so I missed a little bit the context.

sorry for the noise

[squakez]

ok then, I was a little bit confused because the .status.traits has been the center of a number of things recently, like checksum, copy to kit, etc so I missed a little bit the context.

sorry for the noise

Probably I was the one making noise with wrong development ;) - Fixing all those wrong stuff though.

[squakez]

--- PASS: TestNativeHighMemoryIntegrations (1834.10s)
    --- PASS: TestNativeHighMemoryIntegrations/java_native_support (875.74s)
        --- PASS: TestNativeHighMemoryIntegrations/java_native_support/java_native_same_should_not_rebuild (2.21s)
        --- PASS: TestNativeHighMemoryIntegrations/java_native_support/java_native_should_rebuild (428.39s)
    --- PASS: TestNativeHighMemoryIntegrations/groovy_native_support (495.43s)
    --- PASS: TestNativeHighMemoryIntegrations/kotlin_native_support (451.77s)
PASS
ok  	github.com/apache/camel-k/v2/e2e/native	1834.133s
--- PASS: TestNativeIntegrations (884.52s)
    --- PASS: TestNativeIntegrations/unsupported_integration_source_language (0.76s)
    --- PASS: TestNativeIntegrations/xml_native_support (412.21s)
    --- PASS: TestNativeIntegrations/automatic_rollout_deployment_from_jvm_to_native_kit (462.53s)
        --- PASS: TestNativeIntegrations/automatic_rollout_deployment_from_jvm_to_native_kit/yaml_native_should_not_rebuild (2.70s)
PASS
ok  	github.com/apache/camel-k/v2/e2e/native	1330.983s