You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "squakez (via GitHub)" <gi...@apache.org> on 2023/03/23 17:04:28 UTC
[GitHub] [camel-k] squakez opened a new pull request, #4156: feat: change KameletBinding name to Binding
squakez opened a new pull request, #4156:
URL: https://github.com/apache/camel-k/pull/4156
With this PR we aim to change the name of `KameletBinding` to a more generic `Binding`. The change are simple string replacements and in one case changing a struct which collided.
Closes #2625
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
feat: change KameletBinding name to Binding
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] christophd commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1482587944
no objections on that renaming. The reason for the renaming is a more generic approach where the binding is also there to bind non-Kamelet (Knative, Kafka) resources with each other, is that correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1481562798
@oscerd @lburgazzoli @christophd this PR can change other dependencies around Kamelets world. Once we agree if this is okey, we should also promote the API to `v1` so that the preiovus `v1alpha1` can be still used by users of Camel K 1.x versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] oscerd commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1482519523
+1 from my side
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] christophd commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1482636132
I can take care of updating Yaks and releasing a new version that uses Binding. I think it will be a matter of hours to align with that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486565101
I am starting to think that, in order to ease the upgrade from v1 to v2, it really makes sense to keep both `v1` and `v1alpha1`, at least for the first release (deprecating `v1alpha1`, of course). Right now, I'm exactly having issues in the upgrade scenario.
@christophd probably we will need to update Yaks again accordingly. Let's wait until this aspect is clarified, and once the other checks are green, we can have a new release of Yaks as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez merged pull request #4156: feat: change KameletBinding name to Pipe
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #4156:
URL: https://github.com/apache/camel-k/pull/4156
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Pipe
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1542165263
> Maybe wouldn't it be useful to have something like a `kamel` "convert" subcommand which would prepare Pipe from the Kameletbinding?
It could, but, since we're planning not to add any further development to the CLI, it is probably some work we don't want to do. I think that any conversion should be done manually. We can come out with some utility script for sure, as in theory, a `Pipe` could be extracted from a running `KameletBinding` replacing API version and Kind. However there are aspects like stopping the previous Integration, deleting the binding, etc, which every user may want to have control.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486534406
That's correct. Right now I am not really thinking in any upgrade strategy. A new major installation has to be performed from scratch. I understand this can result in some problem for any running integration, let me open a separate issue to discuss about a possible solution to this problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4156: feat: change KameletBinding name to Pipe
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1540466644
May I please ask, how should be tested status of KameletBinding/Pipe in case of upgrade scenario? It needs the prior naming for released operator...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "astefanutti (via GitHub)" <gi...@apache.org>.
astefanutti commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486547810
> the main issue would be that you cannot upgrade an operator to v2 so I think we should either support KameletBinding and Binding for some time or having an automated upgrade strategy (maybe via a webhook) ? @astefanutti WDYT ?
@lburgazzoli changing resource names in Kubernetes isn't possible (the name field is immutable after creation), which makes existing versioning and conversion mechanisms inapplicable to this case, e.g., for CRD conversion webhook:
https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#permissible-mutations
Practically, simply changing the CRD name will create a new type, and leave the existing CRD as is, along with its instances, which will be ignored by the v2 operator.
To provide a smoother migration path, the simplest option would be to support both KameletBinding and Binding resource types as you suggested. It may also be possible to write an ad-hoc conversion controller as a more advanced strategy.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "astefanutti (via GitHub)" <gi...@apache.org>.
astefanutti commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1490031258
It seems, for a reason that is yet to be identified, with `v1` version, the `object` type validation is stricter.
If the `type: object` definitions are removed, and only `x-kubernetes-preserve-unknown-fields` is left, it works, that is equivalent to any types, e.g.:
```yaml
properties:
additionalProperties:
properties:
default:
description: default is a default value for undefined object
fields.
# type: object
x-kubernetes-preserve-unknown-fields: true
deprecated:
type: boolean
description:
type: string
enum:
items:
description: 'JSON represents any valid JSON value. These
types are supported: bool, int64, float64, string, []interface{},
map[string]interface{} and nil.'
# type: object
x-kubernetes-preserve-unknown-fields: true
type: array
example:
description: 'JSON represents any valid JSON value. These
types are supported: bool, int64, float64, string, []interface{},
map[string]interface{} and nil.'
# type: object
x-kubernetes-preserve-unknown-fields: true
```
Interestingly, that was the intent of #1914, with using the `+kubebuilder:validation:Type=""` marker on those fields.
Something added those `type: object` lines afterwards, maybe an upgrade of kubebuilder?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lburgazzoli commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486512971
> > @squakez are removing KameletBinding or making it deprecated ?
>
> As we are moving to a new version (I still need to create either `v1beta1` or promote to `v1` directly - to be discussed) I don't think it makes sense to deprecate.
the main issue would be that you cannot upgrade an operator to v2 so I think we should either support KameletBinding and Binding for some time or having an automated upgrade strategy (maybe via a webhook) ? @astefanutti WDYT ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Pipe
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1540501770
I think the best upgrade strategy is to leave the KameletBinding type to run from v1 to v2. Then, the user should upgrade from KameletBinding to Pipe manually if this is required, ie, stopping the old KameletBinding and starting a new Pipe with the same KLB spec.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] christophd commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486560089
@squakez YAKS 0.15.0 has been released with support for the new Binding resources (I hope it works).
Could you please update the YAKS version in this PR in GitHub workflow action (https://github.com/apache/camel-k/blob/main/.github/actions/e2e-knative-yaks/action.yml#L109). As YAKS needs to support both KameletBinding and Binding for a while you also need to set an env var when running the tests (`YAKS_CAMELK__KAMELET_BINDING_API_VERSION=v1`) maybe here https://github.com/apache/camel-k/blob/main/.github/actions/e2e-knative-yaks/exec-tests.sh#L126
The `__` in the env var name is a bug that I will fix in 0.15.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lburgazzoli commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1487637866
> I am starting to think that, in order to ease the upgrade from v1 to v2, it really makes sense to keep both `v1` and `v1alpha1`, at least for the first release (deprecating `v1alpha1`, of course). Right now, I'm exactly having issues in the upgrade scenario.
>
+1
I understand the complexity but I think is quite not realistic that everyone consuming camel-k APIs would be able to migrate it to v2 straight away
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lburgazzoli commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486498606
@squakez are removing KameletBinding or making it deprecated ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486504145
> @squakez are removing KameletBinding or making it deprecated ?
As we are moving to a new version (I still need to create either `v1beta1` or promote to `v1` directly - to be discussed) I don't think it makes sense to deprecate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1482589959
> no objections on that renaming. The reason for the renaming is a more generic approach where the binding is also there to bind non-Kamelet (Knative, Kafka) resources with each other, is that correct?
Yes, that's correct. Mind that we'll need to update Yaks configuration to let the tests here to pass. However I'm not sure how we can do this ahead of time as we have a circular reference between Camel K and Yaks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4156: feat: change KameletBinding name to Pipe
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1542087342
Maybe wouldn't it be useful to have something like a `kamel` "convert" subcommand which would prepare Pipe from the Kameletbinding?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] christophd commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486526690
what about using new Binding for apiVersion `v1` and existing KameletBinding is still valid for `v1alpha1` apiVersion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] christophd commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1486657164
@squakez YAKS 0.15.0 should be able to work with both KameletBindings and Bindings already. It is based on the apiVersion you set via env var. In order to use the new Binding you have to use `v1` in this setting already regardless of the fact that the CRD for the Binding resource still uses `v1alpha1` at the moment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4156: feat: change KameletBinding name to Binding
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4156:
URL: https://github.com/apache/camel-k/pull/4156#issuecomment-1488792778
I managed to have both version of APIs, and in theory we are not needing a webhook because `Binding` is new and `KameletBinding` is going to be dropped. However, I am having some problem with the new `v1.Kamelet`. For some reason, I am no longer able to run a reconciliation because it fails when trying to patch the `Status: Ready`. This is the kind of error I got on almost all the Kamelets when I try to apply them from `kubectl` (it is the same error reported in operator log):
```
The Kamelet "timer-source-2" is invalid:
* spec.definition.properties.contentType.default: Invalid value: "string": spec.definition.properties.contentType.default in body must be of type object: "string"
* spec.definition.properties.message.example: Invalid value: "string": spec.definition.properties.message.example in body must be of type object: "string"
* spec.definition.properties.period.default: Invalid value: "integer": spec.definition.properties.period.default in body must be of type object: "integer"
```
@astefanutti @lburgazzoli any idea where this is coming from? Mind that there is no change at all between `v1alpha1` and `v1`, but when a `v1.Kamelet` is created, now, that's what happens. I even tried to run with `validate=false` to avoid any kind of validation, but it's the same error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org