apache
fix(#5097): Remove secondary IntegrationPlatform in favor of using IntegrationProfile
- Remove secondary IntegrationPlatform mode
- Reduce logic to a single IntegrationPlatform per operator instance
- Introduce IntegrationProfile custom resource definition
- Let user customize a subset of IntegrationPlatform settings in IntegrationProfile
- Load IntegrationProfile settings when integration resource is annotated to select the profile
- Remove platform creation as part of the platform trait (avoids duplicate platform resources)
- Save trait configuration used to build the integration kit on the resource spec for future reference
Release Note
fix(#5097): Remove secondary IntegrationPlatform in favor of using IntegrationProfile
:warning: Unit test coverage report - coverage decreased from 35.6% to 35.1% (-0.5%)
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
:warning: Unit test coverage report - coverage decreased from 35.6% to 35.1% (-0.5%)
- we can't remove/rename any field from custom resource for backward compatibility reason. Anything we want to remove we must deprecate the fields and eventually remove them in a major upgrade. Same for the feature itself. We should deprecate it before removing to give the user the possibility to upgrade accordingly.
Regarding the CRD changes: do you refer to the removed trait options on the platform trait? What about auto migrating local IntegrationPlatform CRs to IntegrationProfile CRs?
- we should avoid decreasing the coverage that much. Hopefully you can include some unit test to raise the coverage to be at least even.
I just need to remove more code 😄 Yes, my plan was to add more unit tests
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.3% (-0.5%)
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.3% (-0.5%)
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.5% (-0.3%)
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.5% (-0.3%)
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.5% (-0.3%)
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.5% (-0.3%)
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.7% (-0.1%)
:warning: Unit test coverage report - coverage decreased from 35.8% to 35.7% (-0.1%)
@squakez @lburgazzoli @gansheer @claudio4j the PR is now ready for review. If you want to have another look 🙏
I'd suggest to follow-up with these tasks in separate PRs:
- Further simplification of integration platform logic with deprecate/remove namespace local integration platforms (strictly have only 1-1 relationship of operator and integration platform)
- Restrict the settings that a user is able to set in integration profile and builder traits (admin related settings only settable via integration platform)
Regarding the CRD changes: do you refer to the removed trait options on the platform trait? What about auto migrating local IntegrationPlatform CRs to IntegrationProfile CRs?
I think we should first translate the local IntegrationPlatform to a IntegrationProfile without removing it as this may have impacts to CI/CD pipelines, similar to what has been done for the transition from KameletBinding to Pipe
I think we should first translate the local
IntegrationPlatformto aIntegrationProfilewithout removing it as this may have impacts to CI/CD pipelines, similar to what has been done for the transition fromKameletBindingtoPipe
The local IntegrationPlatform should still be a valid option after the changes in this PR
I think we should first translate the local
IntegrationPlatformto aIntegrationProfilewithout removing it as this may have impacts to CI/CD pipelines, similar to what has been done for the transition fromKameletBindingtoPipeThe local IntegrationPlatform should still be a valid option after the changes in this PR
bet gets translated to an IntegrationProfile ?
bet gets translated to an IntegrationProfile ?
no, the resources can coexist at the moment
bet gets translated to an IntegrationProfile ?
no, the resources can coexist at the moment
do you think we can do the translation ? just to simplify the logic on the operator
do you think we can do the translation ? just to simplify the logic on the operator
The resources are not exactly the same. The IntegrationProfile has only a subset of settings available on the IntegrationPlatform.
Do you have some auto translation in mind? or that the user needs to manually switch to using IntegrationProfile instead of IntegrationPlatform? I think the latter one is doable with some documentation. If we really want to simplify the logic on the operator regarding IntegrationPlatform we should remove the local platform feature and tell users to switch to IntegrationProfile
do you think we can do the translation ? just to simplify the logic on the operator
The resources are not exactly the same. The IntegrationProfile has only a subset of settings available on the IntegrationPlatform.
Do you have some auto translation in mind? or that the user needs to manually switch to using IntegrationProfile instead of IntegrationPlatform? I think the latter one is doable with some documentation. If we really want to simplify the logic on the operator regarding IntegrationPlatform we should remove the local platform feature and tell users to switch to IntegrationProfile
No I don't so I'm fine with the current logic for the time being (which could be improved once we get https://github.com/apache/camel-k/pull/5119 merged).
I was mainly looking if there were a way to preserve backward compatibility in term of CRD/CR but simplify things internally.
LGTM. Might be a good idea to regenerate the CRDs.
@gansheer I did a make generate and make bundle - am I missing something else?
:warning: Unit test coverage report - coverage decreased from 36.2% to 36% (-0.2%)
@valdar @rinaldodev please have a look at this may impact https://github.com/apache/camel-k/pull/5119/