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(CRD): Traits configuration schema #3235

Merged
merged 14 commits into from
Jul 8, 2022

Conversation

tadayosi
Copy link
Member

@tadayosi tadayosi commented Apr 26, 2022

This addresses #1614. This tries to restore the unmerged pull req #1624.

This PR moves the traits configuration into the API. This enables the generation of the openAPI schema for the traits configuration into the Integration, IntegrationPlatform and IntegrationKit CRDs.

Release Note

feat(CRD): Traits configuration schema

UPDATE

Now this enhancement is completed and open for reviews, I outline what this enhancement changes on the Traits API for reviewers.

API packaging

There are mainly 3 possibilities for packaging the new Traits API schema types.

  • pkg/apis/camel/v1
  • pkg/apis/camel/v1/trait
  • pkg/apis/camel/trait/v1

Each has its pros and cons, but this pull req has chosen the 2nd option (pkg/apis/camel/v1/trait); This option avoids creating a new API traits.camel.apache.org/v1 but still can pack the types into a dedicated package and enjoy maintenability.

NOTE: I'll later check if this packaging option really works well with the https://github.com/djencks/gen-crd-api-reference-docs tool in script/gen_crd/gen_crd_api.sh.

Schema changes on CRs

By introducing the strongly-typed Traits API, it also imposes changes on the following CRDs: integrations, integrationkits, and integrationplatforms.

However, backward compatibility is also respected in this implementation. See the Backward compatibility section below.

Builtin traits

Trait properties under spec.traits.<trait-id>.configuration are now defined directly under spec.traits.<trait-id>.

traits:
  container:
    configuration:
      enabled: true
      name: my-integration

↓↓↓

traits:
  container:
    enabled: true
    name: my-integration

Addons

In the new API, addon traits need to be moved to spec.traits.addons because there's no type-safe way to accept arbitrary addon IDs in the strongly-typed Traits type:

type Traits struct {
	...
	Addons map[string]AddonTrait `json:"addons,omitempty"`
}
type AddonTrait struct {
	RawMessage `json:",inline"`
}

Thus an addon trait spec.traits.<addon-id> is now moved to spec.traits.addons.<addon-id>, and addon trait properties under spec.traits.<addon-id>.configuration are now defined under spec.traits.addons.<addon-id>.

traits:
  master:
    configuration:
      labelKey: my-group
      labelValue: same-group
      resourceName: my-lock

↓↓↓

traits:
  addons:
    master:
      labelKey: my-group
      labelValue: same-group
      resourceName: my-lock

Backward compatibility

Backward compatibility is very important, as otherwise we would end up needing to start developing camel.apache.org/v2 version just for the new typed Traits API. To achieve backward compatibility between 1.9.x and 1.10.x, the following logic is implemented in this pull req:

  • The Configuration field with RawMessage type is provided for each trait type so that the existing integrations etc. resources from <=1.9.x can be read from the new Camel K.
    type Trait struct {
        // Can be used to enable or disable a trait. All traits share this common property.
        Enabled *bool `property:"enabled" json:"enabled,omitempty"`
    
        // Legacy trait configuration parameters.
        // Deprecated: for backward compatibility.
        Configuration *Configuration `json:"configuration,omitempty"`
    }
    
    // Deprecated: for backward compatibility.
    type Configuration struct {
        RawMessage `json:",inline"`
    }
  • When the old integrations etc. resources are read, the legacy configuration in each trait (if any) is migrated to the new Trait API fields, but if there are already some values defined on the new API fields they precede the legacy ones.
  • The Traits type needs to have placeholders for legacy addon configurations. These are also used for migration to spec.traits.addons.<addon-id> at runtime.
      // Deprecated: for backward compatibility.
      Keda *TraitSpec `property:"keda" json:"keda,omitempty"`
      // Deprecated: for backward compatibility.
      Master *TraitSpec `property:"master" json:"master,omitempty"`
      // Deprecated: for backward compatibility.
      Strimzi *TraitSpec `property:"strimzi" json:"strimzi,omitempty"`
      // Deprecated: for backward compatibility.
      ThreeScale *TraitSpec `property:"3scale" json:"3scale,omitempty"`
      // Deprecated: for backward compatibility.
      Tracing *TraitSpec `property:"tracing" json:"tracing,omitempty"`

@tadayosi tadayosi added the status/wip Work in progress label Apr 26, 2022
@tadayosi
Copy link
Member Author

tadayosi commented Apr 27, 2022

  • The main part is done. Now I'll see and fix what makes the e2e tests fail.

-> Done

@tadayosi
Copy link
Member Author

tadayosi commented Apr 28, 2022

  • One more issue to address is that the addons are not listed in the Traits struct. We need to find a way to extend Traits from addons.

    One option would be to add an Addons struct along with the Traits, which would serialise as a list of RawMessage (as traits currently), and have a mechanism to contribute/update the CRD with the addon structural schema.

-> Done

@tadayosi tadayosi force-pushed the Issue-1614-traits-scheme branch 3 times, most recently from 3869c0f to 204f770 Compare May 16, 2022 14:45
@tadayosi tadayosi marked this pull request as draft June 3, 2022 05:57
@tadayosi tadayosi force-pushed the Issue-1614-traits-scheme branch 2 times, most recently from 79074c2 to f020796 Compare June 21, 2022 06:01
@tadayosi tadayosi removed the status/wip Work in progress label Jul 1, 2022
@tadayosi tadayosi changed the title WIP - feat(CRD): Traits configuration schema feat(CRD): Traits configuration schema Jul 1, 2022
@tadayosi tadayosi marked this pull request as ready for review July 1, 2022 07:27
@tadayosi tadayosi force-pushed the Issue-1614-traits-scheme branch 3 times, most recently from 665d96b to a65ab58 Compare July 2, 2022 02:46
Since OLM 0.19.0 the max total size of bundle is increased from ~1MB to
~4MB and now the bloated Camel K Operator bundle requires >1.3MB, so
upgrading OLM to >= 0.19.0 is necessary.
https://groups.google.com/g/operator-framework/c/79UO6oGwuTs
@tadayosi
Copy link
Member Author

tadayosi commented Jul 5, 2022

The openshift-build workflow is failing at known flaky tests, which should be irrelevant to this pull req.

@astefanutti astefanutti added the kind/feature New feature or request label Jul 6, 2022
Copy link
Member

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impressive! I've enjoyed everything I've reviewed there!

@tadayosi
Copy link
Member Author

tadayosi commented Jul 8, 2022

Thanks! OK, let me go ahead and merge...

@tadayosi tadayosi merged commit 35ed0ee into apache:main Jul 8, 2022
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.

None yet

2 participants