feat(runtime): get rid off camel k runtime dependency - WIP #5090
Conversation
|
✔️ Unit test coverage report - coverage increased from 34.8% to 35.3% (+0.5%) |
|
@squakez 👍 this looks good to me so far |
|
✔️ Unit test coverage report - coverage increased from 34.8% to 35.8% (+1%) |
1 similar comment
|
✔️ Unit test coverage report - coverage increased from 34.8% to 35.8% (+1%) |
|
✔️ Unit test coverage report - coverage increased from 34.8% to 35.8% (+1%) |
1 similar comment
|
✔️ Unit test coverage report - coverage increased from 34.8% to 35.8% (+1%) |
|
✔️ Unit test coverage report - coverage increased from 34.8% to 36.3% (+1.5%) |
|
understanding how camel-main works, we can see the |
When I first saw this change, I wondered about that. Does this mean we could remove the camel-k bits from CQ? And could that change be done now so that it's complete in time for the next CQ LTS release? AFAIK, camel-k has not consumed the camel-k-runtime extension from CQ yet. |
That's correct. As soon as this development is complete, we can remove the related ck-runtime bits from CQ and in general make the dependency process much smoother. It's possible that however we'll need to include the catalog into CQ or extend the one available there, but it should be done via maven plugin. Not reached yet that part, I expect this development to last still a bit :) |
|
camel-k-runtime supports the dynamic reload of a route (no project rebuild) when the parameters are modified, for example, the master trait, if the resourceName parameter change with a I think this is fine, as there are only 3 extensions in camel-k-runtime whom are affected by this hot reload feature not available in non dev mode in camel-quarkus. |
| idx++ | ||
| } | ||
| e.ApplicationProperties["camel.main.source-location-enabled"] = "true" | ||
| e.ApplicationProperties["camel.main.routes-include-pattern"] = fmt.Sprintf("file:%s/**", camel.SourcesMountPath) |
There was a problem hiding this comment.
this is likely not working as expected as the interceptors then won't work.
also some interceptors are per routes so the general route discovery is likely not enough
some work must be done on the camel side before using this approach. this is essential the reason why there is still a camel-k extension in camel-quarkus at this stage.
hot reload is not what we really want and MUST be disabled, if something changes, the pod must be restarted otherwise it is gonna be hard to do proper upgrade as things may change and fail out of control. |
By hot reload I meant the possibility of changing a configuration parameter that doesn't require a maven rebuild, like the master trait example I provided earlier, which just works. |
ah ok, I had the impression it was about hot reload of routes which in reality never happens as the pod is restarted |
|
✔️ Unit test coverage report - coverage increased from 35.6% to 36.5% (+0.9%) |
|
✔️ Unit test coverage report - coverage increased from 35.6% to 36.6% (+1%) |
|
✔️ Unit test coverage report - coverage increased from 35.6% to 36.8% (+1.2%) |
|
✔️ Unit test coverage report - coverage increased from 35.6% to 36.8% (+1.2%) |
Secret must be mounted anyhow and can be used normally without the usage of that syntax.
By including in the application.properties we're not limited by the xml format which prevent to include any kind of character.
It failed when we had odd number of dependencies
|
✔️ Unit test coverage report - coverage increased from 35.8% to 37.9% (+2.1%) |
| func (t *mountTrait) setConfigLocations(container *corev1.Container, configPaths []string) { | ||
| if configPaths != nil { | ||
| envvar.SetVar(&container.Env, corev1.EnvVar{ | ||
| Name: "QUARKUS_CONFIG_LOCATIONS", |
There was a problem hiding this comment.
We have established a contract for which each runtime (quarkus and in future spring boot and eventually plain camel main) must honor in order to be camel-k compatible which states where the config maps, secrets and resources are projected.
Given that each runtime has it's own machinery to discover/load properties I think this goes in the wrong direction as implies the operator to have a deep knowledge of the runtime, which we want to avoid.
The operator must only mount config maps, secrets and resources in the expected directories and then leave discovery part to the specific runtime.
There was a problem hiding this comment.
Sure. Most of the traits are actually only working with Quarkus (see log trait, master trait, jvm etc). The idea of this first development is to remove the Camel K runtime dependency, and eventually work to make it completely runtime agnostic (both in this trait and any other trait).
There was a problem hiding this comment.
given there is a camel-k extension in camel-quarkus, I really don't see the need for this and for computing the list of properties.
There was a problem hiding this comment.
My main goal is reducing the effort on maintaining the software and shortening the release cycle. As I am the one dealing with it with, I am trying to find efficient ways to manage it.
There was a problem hiding this comment.
Me too, hence I recommend to keep the extension and work across the stack to make the integration smooth
| // the user asked to store the entire resource without specifying any filter | ||
| // we need to list all the resources belonging to the resource | ||
| if conf.StorageType() == utilResource.StorageTypeConfigmap { | ||
| cm := kubernetes.LookupConfigmap(e.Ctx, e.Client, e.Integration.Namespace, conf.Name()) |
There was a problem hiding this comment.
The operator must NOT access to the user's config map/secrets/resources.
There was a problem hiding this comment.
True. We may rework the feature to let the user explictly specify the cm/secrets holding the properties and deprecate this part for future removal.
There was a problem hiding this comment.
Isn't this already the case ? with mount.configs we know already what config maps/secrets are configuration resources.
There was a problem hiding this comment.
No. Not all the cm/secrets are .properties file.
There was a problem hiding this comment.
as far as I remember, it does not matter.
The runtime/extensione loads them even if they are not .properties
|
@squakez I've created a draft PR here to clean up the camel-k extension that exists in camel-quarkus, once merged you should be able to leverage overrides, like: camel.k.routes.overrides[0].input.from = direct:r1
camel.k.routes.overrides[0].input.with = direct:r1overrideTo replace cron's workaround. It also enhance properties discovery and loading so you can probably remove the custom property discovery mechanism that has been implemented here. As as side note, implementing #5181 could probably help improving camel-k configuration handling. |
|
@squakez about the cron job, Once apache/camel-quarkus#5802 will be available, there is this new camel-k property you can use to gracefully shutdown the application: |
Ref #4024
Requires still some work to be fully functional, but it should give an idea where to head the new design.
Release Note