Skip to content
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

feat: import external Camel applications #4942

Merged
merged 8 commits into from
Jan 9, 2024

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Nov 29, 2023

With this PR we enable the possibility to import any Camel application deployed on Kubernetes manually or via other tooling and be able to monitor it as a "synthetic" Integration.
We can control the same deployment objects we are using for managed Integrations: Deployment, CronJob and Knative Services.

It works by annotating the specific object we want to import with the camel.apache.org/integration=my-it label. The operator creates a synthetic Integration and monitor the resources after the object which created it. In order to monitor the sibling Pods, we need the user to properly add an annotation in the resource template.

$ kubectl label deploy my-camel-sb-svc camel.apache.org/integration=my-it
$ kubectl patch deployment my-camel-sb-svc --patch '{"spec": {"template": {"metadata": {"labels": {"camel.apache.org/integration": "my-it"}}}}}'
$ k get it
NAME    PHASE     RUNTIME PROVIDER   RUNTIME VERSION   KIT   REPLICAS
my-it   Running                                               1
$ k scale deploy my-camel-sb-svc --replicas 2
$ k get it
NAME    PHASE     RUNTIME PROVIDER   RUNTIME VERSION   KIT   REPLICAS
my-it   Running                                               2
$ k delete deploy my-camel-sb-svc
deployment.apps "my-camel-sb-svc" deleted
$ k get it
NAME    PHASE                 RUNTIME PROVIDER   RUNTIME VERSION   KIT   REPLICAS
my-it   Application Missing                                                  0

We're covering the main corner cases such as removal of the object which generated the import or the missing selector which would not allow the possibility to monitor the sibling pods. It's a MVP that sets the framework for any possible future feature around monitoring.

Release Note

feat: import external Camel applications

@squakez squakez added the kind/feature New feature or request label Nov 29, 2023
Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ⚠️ - Coverage changed: 33.6% --> 33.3% (Coverage difference: -.3%)

@lburgazzoli
Copy link
Contributor

quick comment before looking at the code: we can maybe simplify it by using a static label camel.apache.org/supervise=true and take the name of the deployment/etc/

@squakez
Copy link
Contributor Author

squakez commented Nov 29, 2023

quick comment before looking at the code: we can maybe simplify it by using a static label camel.apache.org/supervise=true and take the name of the deployment/etc/

Unfortunately not. Or at least, it would not be that easy. The way we are watching objects is quite limited to the presence of the camel.apache.org/integration label. If we remove such a requirement, we'd need to watch all objects. You will see later that we already had to remove the filter from Pods, in order to be able to watch also the non managed ones. I think that if we remove this constraint we may degrade the performance too much.

@lburgazzoli
Copy link
Contributor

quick comment before looking at the code: we can maybe simplify it by using a static label camel.apache.org/supervise=true and take the name of the deployment/etc/

Unfortunately not. Or at least, it would not be that easy. The way we are watching objects is quite limited to the presence of the camel.apache.org/integration label. If we remove such a requirement, we'd need to watch all objects. You will see later that we already had to remove the filter from Pods, in order to be able to watch also the non managed ones. I think that if we remove this constraint we may degrade the performance too much.

mh, I see.
we may need to think how to make the process simpler

Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ⚠️ - Coverage changed: 33.6% --> 33.3% (Coverage difference: -.3%)

Copy link
Contributor

github-actions bot commented Dec 1, 2023

🐫 Thank you for contributing!

Code Coverage Report ⚠️ - Coverage changed: 33.6% --> 33.5% (Coverage difference: -.1%)

Copy link
Contributor

github-actions bot commented Dec 1, 2023

🐫 Thank you for contributing!

Code Coverage Report ⚠️ - Coverage changed: 33.6% --> 33.5% (Coverage difference: -.1%)

@squakez squakez marked this pull request as ready for review December 1, 2023 15:06
Copy link
Contributor

github-actions bot commented Dec 1, 2023

🐫 Thank you for contributing!

Code Coverage Report ⚠️ - Coverage changed: 33.6% --> 33.5% (Coverage difference: -.1%)

Copy link
Contributor

github-actions bot commented Dec 4, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.6% --> 34.1% (Coverage difference: +.5%)

2 similar comments
Copy link
Contributor

github-actions bot commented Dec 5, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.6% --> 34.1% (Coverage difference: +.5%)

Copy link
Contributor

github-actions bot commented Dec 5, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.6% --> 34.1% (Coverage difference: +.5%)

@squakez
Copy link
Contributor Author

squakez commented Dec 5, 2023

An additional comment which may be useful in the future. Before reaching this state, I've worked on a few experiments. The result of them may return useful eventually.

The first experiment was to let the operator assume the control of the managed fields of a Deployment: 1e4aaaa#diff-de2d026ce0dc415e68ee971f300e7320de019cc68c5171cf76425859587f1a85 - this solution was discarded because it clearly overlap the external deployment procedure (ie, triggering a new deployment).

The second one had the same problem (overlapping external deployment): https://github.com/squakez/camel-k/tree/feat/it_import_delete_deploy - in this case we were deleting the deployment, in order to let the operator create and manage one from scratch.

Although they are aggressive approaches, they may be used eventually in a sort of "dry-run" operation to let the user decide if a given deployment can be substituted with a managed Integration (at the price of forcefully deleting the original deployment).

@squakez squakez force-pushed the feat/it_import_monitor_only branch from 0f82384 to 8910f11 Compare January 3, 2024 15:22
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🐫 Thank you for contributing! 🐫

Unable to create Coverage Report ⚠️.
Merge conflicts found.

@squakez
Copy link
Contributor Author

squakez commented Jan 3, 2024

@lburgazzoli I think I've addressed the concerns of your last review. About the possibility to miss an event, now this should be covered by the same monitoring action which is in charge to be reconciled and eventually capture any possible missing delete event.

Copy link
Contributor

github-actions bot commented Jan 3, 2024

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.6% --> 34.2% (Coverage difference: +.6%)

@squakez squakez force-pushed the feat/it_import_monitor_only branch from facacb4 to d133d64 Compare January 3, 2024 16:09
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.6% --> 34.2% (Coverage difference: +.6%)

@squakez squakez force-pushed the feat/it_import_monitor_only branch 2 times, most recently from baf1682 to 473732b Compare January 9, 2024 11:24
@squakez squakez force-pushed the feat/it_import_monitor_only branch from 473732b to 802aea9 Compare January 9, 2024 11:29
@squakez squakez merged commit 2bbe92b into apache:main Jan 9, 2024
14 of 15 checks passed
@squakez squakez deleted the feat/it_import_monitor_only branch January 9, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants