feat: change KameletBinding name to Pipe

[squakez]

With this PR we aim to change the name of KameletBinding to a more generic Pipe. We are also deprecating any usage of KameletBinding, altough the resource will be reconciled and will work normally. Also Kamelet are promoted from v1alpha1 to v1 and an automatic conversion is realized by k8s.

Closes #2625

Release Note

feat: change KameletBinding name to Binding

[squakez]

@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.

[oscerd]

+1 from my side

[christophd]

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?

[squakez]

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.

[christophd]

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

[lburgazzoli]

@squakez are removing KameletBinding or making it deprecated ?

[squakez]

@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.

[lburgazzoli]

@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 ?

[christophd]

what about using new Binding for apiVersion v1 and existing KameletBinding is still valid for v1alpha1 apiVersion

[squakez]

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.

[astefanutti]

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.

[christophd]

@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

[squakez]

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.

[christophd]

@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

[lburgazzoli]

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

[squakez]

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.

[astefanutti]

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.:

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?

[lfabriko]

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...

[squakez]

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.

[lfabriko]

Maybe wouldn't it be useful to have something like a kamel "convert" subcommand which would prepare Pipe from the Kameletbinding?

[squakez]

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.