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

introduce v1alpha2 api with auto conversion #1529

Closed
wants to merge 1 commit into from

Commits on Nov 6, 2019

  1. wip: introduce v1alpha2 api with auto conversion

    This introduces a new api version `v1alpha2` while still allowing user
    to use `v1alpha1` API.
    
    - Only one version of the CRD can be stored, so `v1alpha1` is the one
    stored
    - All the version of the CRD **must** be schema compatible (before 1.16)
      - `v1alpha1` gets **all** the fields, meaning :
        - the new ones (from `v1alpha2`, …)
        - the old deprecated ones (that will be removed from `v1alpha2`)
      - we may *only* document the "frozen" `v1alpha1` field (and not the
        new one that are coming from the other api versions)
    - `kubectl` get returns the highest priority version, aka the most
      recent one, here `v1alpha2`. To make *sure* any object created from
      any version are valid (while getting), we convert them using the
      admission webhook (#blackmagic 🧙).
      - Object created using `v1alpha1` API will automatically be
      converted (see below)
      - This has the downsize of not being able to `get` v1alpha1 object
      anymore (although it should be possible using the `client-go` and
      some even more #blackmagic 🧙 hack on the webhook side probably)
    - One benefit of this approach (`apis.Convertible`) is that once
      we *only* support 1.16 (the release with conversion webhook), this
      will be usable almost as is (for the conversion webhook) 👼.
    
    This is done in a quick and dirty way:
    - Lot's of code is duplicated where it would not be needed, it was
      just quicker to do so at that point in time
    - I completely skipped the *variable interpolation* validation for
      `Parameters` as those values would/will need to be automatically
      transformed too (but it was a bit complicated for what I wanted to
      experiment on)
    - I did not implement the **reconcilier** part of this. It should be
      simple as:
      - it will use only one object (the store one, aka `v1alpha1`)
      - it will make the assumption that the objects are fully
        transformed (either by the webhook or in-memory by the
        controller), meaning there will be no *handling
        backward-compatible* piece of code in the reconcilier (all is
        contained in the `Validable`, `Defaultable` and `Convertible` impl.).
    
    Possible next steps are:
    - Split this into smaller PR.
    - Introduce a `v1alpha2` package early but not exposing it before we
      are ready to. This will allow us to not have to rush this for
      0.9, **and** to increment over this in smaller step.
    - Decide what `v1alpha2` should look like — I did this experiment with
      `TaskSpec.Inputs.Params` to `TaskSpec.Params` but we should decide
      what we want to change and target that.
    - Reduce duplication by reusing anything that didn't change from
      `v1alpha1` to `v1alpha2` in `v1alpha1` (aka refering the same type
      in go)
    - Related to this experiment, implement the variable interpolation
      validation thing **and** the reconcilier part too.
    - Need to figure out if the definition types (`Task`, `Pipeline`, …)
      need a `controller` (I don't think so but…)
    
    Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
    vdemeester committed Nov 6, 2019
    Configuration menu
    Copy the full SHA
    d75b034 View commit details
    Browse the repository at this point in the history