feat(api): Kamelet as static resource #4808
Conversation
|
cc @oscerd @gansheer @christophd @lburgazzoli @claudio4j WDYT? Any objection to make the resources static? I think it simplifies the operator with one resource less to consider as I am not aware of any real use case for the Kamelets to be dynamic. Above all, after #4797 where the Kamelets lifecycle will be completely managed at runtime (ie, default Kamelets values). |
|
🐫 Thank you for contributing! Code Coverage Report ✔️ - Coverage unchanged. |
There was a problem hiding this comment.
From what I see the Kamelet reconciliation is doing sanity checks on properties and names. Also it computes the Kamelet properties and sets the computed list of properties on the Kamelet status for later usage of default values as application properties.
I think it is not a bad idea to check the Kamelet when a user changes/adds a Kamelet as a CR. In case we ignore changes and just add the Kamelet as source to the runtime without such a check errors will be reported late in the process at Integration runtime only. Isn't it a good idea to fail fast when the Kamelet is changed and avoid adding it to the Integration when it is broken?
In general I think having the Kamelets as CRs is a huge benefit
Thanks for the feedback. As I mentioned in the description, in order to understand this PR we need to have a look at #4797 first. In that PR we are letting the management of default properties to the runtime. This PR won't remove Kamelets as CR, it will remove its dynamic nature. However the reconciliation today its just making sure that it's a valid name [1]. We can make sure that any validation around names is performed statically when we submit the CR (this is something Kubernetes already does). Mind that this Kamelets mechanics was introduced when Kamelets were not yet a Camel thing, so, likely the reconciliation had a sense back in time. Today it really does nothing that cannot be performed statically. |
|
This is a more general question: does the change from dynamic to static has an impact on how a modification in a Kamelet CR that has an impact on a running integration is handled by the operator ? |
No. The reconciliation only affects the same resource lifecycle. My question for feedback goes in the direction to understand if there is any usage of |
|
@squakez yeah I should have looked into PR #4797 first. makes more sense now, thx I had a look into the sanity checks on properties and names that is done on Kamelet reconciliation and it is really quite basic at the moment. I think it only makes sure to not use Also I have had a look into the properties computation done as part of the reconciliation loop and to be honest I have no idea where this should be used in the Kamelet status. Maybe we do a sync with tooling (e.g. Kaoto, ODC) if this is being used but in case the configuration property mechanism is going to change as done in PR #4797 I do not see any use case for that property list in Kamelet status anymore. So finally I am ok with removing the Kamelet reconciliation |
squakez
force-pushed
the
feat/kamelet_static_resources
branch
from
df30d3a
to
d059cb1
Compare
|
🐫 Thank you for contributing! Code Coverage Report ✔️ - Coverage unchanged. |
Remove the reconciliation loop and the kamelet.status property
squakez
force-pushed
the
feat/kamelet_static_resources
branch
from
d059cb1
to
8403c6a
Compare
|
🐫 Thank you for contributing! Code Coverage Report ✔️ - Coverage unchanged. |
As it could be used by any external tool.
squakez
force-pushed
the
feat/kamelet_static_resources
branch
from
6f77e8a
to
309e53e
Compare
This PR removes the Kamelet reconciliation loop and the kamelet.status property.
I have no knowledge of a real need to reconcile every Kamelet, so, it seems to me easier to manage the Kamelets as static resources. We can validate them adding any further schema validation in the CR, so, upon creation of the Kamelet, the cluster would complain if something is wrong or missing.
This PR has to be rebased after #4797 in order to be fully functional.
Release Note