-
Notifications
You must be signed in to change notification settings - Fork 363
Improve configmap test coverage #6158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@squakez -- please review this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature you're asking for is a Camel feature available since version 4.8: https://camel.apache.org/manual/using-propertyplaceholder.html#_resolving_property_placeholders_on_cloud
We can include the test for sure, but to make it work also for the plain runtime I think it should be run with the camel.main.cloud-properties-location explicitly. Can you give it a try please?
|
Correct, and setting
The default runtime currently has awareness of how to handle this scenario, somehow, as it works (as shown in these tests). What I am questioning is why similar logic can't be used to auto-handle this scenario. |
|
Ok, I got your point. The "problem" is that the plain runtime is not aware of where it is running (it could not be necessarily on a cloud environment), so, it must be instructed specifically where to look for by setting the following camel property We can think to add something on Camel core, in order to be able to automatically parse any file with |
|
This PR has been automatically marked as stale due to 90 days of inactivity. |
|
I finally ended up creating the request and making the development: see https://issues.apache.org/jira/browse/CAMEL-22489 - we can run this again when the new runtime is available (likely quarkus 3.29.0). |
|
Camel Quarkus 3.29.0 is on vote. As soon as it is released (likely next week) we'll run this against the new runtime and be able to merge it. |
|
Let's merge this and we can evaluate if it passes nightly tests against |
Address #6157.
The goal of the PR is to address missing coverage as it pertains to configmaps that were generated with the
--from-filefunction, specifically for.propertiesfiles (e.g.kubectl create configmap my-cm-properties-file --from-file=./files/my.properties).This was built off the
tags/v2.6.0.Additions:
This PR highlights that these new tests pass uses the default
quarkusruntime, but when switching toplain-quarkusruntime, it fails.Tested via:
makemake imageskubectl create namespace camel-kmake install-k8s-globalkubectl patch itp camel-k -n camel-k -p '{"spec":{"traits":{"camel":{"runtimeProvider":"plain-quarkus"}}}}' --type=mergemake test-commonError from plain-quarkus:
NOTE: The initial
makewas failing onkeystore_test.godue to something with the cert -- did not troubleshoot, but noting it for awareness.Release Note