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

Add v1alpha2 API version #139

Merged
merged 7 commits into from
Apr 22, 2021
Merged

Add v1alpha2 API version #139

merged 7 commits into from
Apr 22, 2021

Conversation

squaremo
Copy link
Member

@squaremo squaremo commented Mar 29, 2021

This adds a v1alpha2 API, without changing the controller or tests to
use it (yet). The new API has roughly the desired shape, per
fluxcd/flux2#1124, but supporting only
things that the controller supports now.

TODO:

  • .spec.sourceRef rather than .spec.checkout.gitRepositoryRef
  • move git-specific fields to .spec.git
  • .spec.git.commit.author.{email,user}
  • .spec.git.checkout.ref rather than ...branch
  • generate spec reference doc from v1alpha2
  • rewrite spec guide
  • write a guide for rewriting YAMLs from v1alpha1 to v1alpha2

EDIT: I put the migration guide at the bottom of the v1alpha2 spec guide.

@squaremo squaremo force-pushed the v1alpha2-api branch 2 times, most recently from ae174aa to 5ae7ad6 Compare March 31, 2021 14:37
This adds a v1alpha2 API, without changing the controller or tests to
use it (yet). The new API has roughly the desired shape, per
fluxcd/flux2#1124, but supporting only
things that the controller supports now.

It's necessary to give the v1alpha1 type a `storageversion` marker so
that 1. code generation keeps working, and 2. tests still work, since
they still expect v1alphav1 types. v1alpha1 will be removed once the
controller and tests are ported to v1alpha2.

Signed-off-by: Michael Bridgen <michael@weave.works>
This finishes the v1alpha2 API, and rewrites everything needed so that
the controller supports it and the tests pass. For the most part, that
is just changing the location of fields. However, there's a few
notable extras:

 - check that the `sourceRef` is a git repo (that's the default), and
   that a `.spec.git` is supplied;

 - change a test that blindly patched an update object, so that it
   first gets the object it's patching. Previously, it succeeded
   because it was OK to patch everything to empty strings, but that's
   no longer the case since SourceReference.Kind is an enum.

Signed-off-by: Michael Bridgen <michael@weave.works>
This is a bit neater to read and write, and since I'm making breaking
changes anyway.

The name is now optional; an email is enough.

Signed-off-by: Michael Bridgen <michael@weave.works>
This changes the API so that the checkout field has a ref, the same as
GItRepository. This means you can check out a branch or a tag or a
particular commit. Most of these won't work unless you supply a branch
to push to as well.

An addtional change is that you can leave out the checkout altogether,
and the ref will default to that given in the GitRepository, or its
default. In the latter case, again you will need to provide a push
branch.

Signed-off-by: Michael Bridgen <michael@weave.works>
This switches the API doc generation from v1alpha1, to v1alpha2.

Signed-off-by: Michael Bridgen <michael@weave.works>
This mostly adapts the material in the v1alpha1 spec explainer doc to
the different structure of the v1alpha2 types.

Signed-off-by: Michael Bridgen <michael@weave.works>
This commit adds a step-by-step guide to rewriting specs for v1alpha2.

Signed-off-by: Michael Bridgen <michael@weave.works>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @squaremo 🏅

@squaremo
Copy link
Member Author

Should this also remove the v1alpha1 types and CRD definition @stefanprodan?

@stefanprodan
Copy link
Member

I think we need to test what happens with and without v1alpha1 after an upgrade if the manifests in the repo are on v1alpha1. For the other controllers we removed the alpha version when we switched to beta but this is different as we are moving from one alpha to another.

squaremo added a commit to fluxcd/image-reflector-controller that referenced this pull request Apr 19, 2021
The image automation part of the API has changed structure (see [1]),
and had a version bump from `v1alpha1` to `v1alpha2`. Since the types
here are also in `image.toolkit.fluxcd.io`, there will be less
complication if they also get a version bump even though they aren't
changing.

For the automation types, leaving the old version `v1alpha1`
definitions in place leads to Kubernetes trying to upgrade objects and
dropping most fields (which have changed location), so they have been
removed. That would not happen with the types here, but for
consistency I am removing the old API version here too.

[1] fluxcd/image-automation-controller#139

Signed-off-by: Michael Bridgen <michael@weave.works>
@squaremo
Copy link
Member Author

squaremo commented Apr 19, 2021

I think we need to test what happens with and without v1alpha1 after an upgrade if the manifests in the repo are on v1alpha1.

If you have a v1alpha1 ImageUpdateAutomation object, and you apply a CRD with both v1alpha1 and v1alpha2 definitions, then Kubernetes attempts to convert your object to v1alpha2 and loses most of the fields. I tried this by applying the CRD YAML with kubectl, then fetching the object with kubectl get imageupdateautomation -o yaml. I presume a controller using v1alpha2 would see the same thing.

The other situation is if you have a v1alpha1 object and apply the CRD with just the v1alpha2 definition. When I apply a CRD which defines only v1alpha2, it fails with the error:

The CustomResourceDefinition "imageupdateautomations.image.toolkit.fluxcd.io" is invalid: status.storedVersions[0]: Invalid value: "v1alpha1": must appear in spec.versions

.. which makes me think applying it as-is in a flux install / bootstrap will make it fail.

squaremo added a commit to fluxcd/image-reflector-controller that referenced this pull request Apr 19, 2021
The image automation part of the API has changed structure (see [1]),
and had a version bump from `v1alpha1` to `v1alpha2`. Since the types
here are also in `image.toolkit.fluxcd.io`, there will be less
complication if they also get a version bump even though they aren't
changing.

For the automation types, leaving the old version `v1alpha1`
definitions in place leads to Kubernetes trying to upgrade objects and
dropping most fields (which have changed location), so they have been
removed. That would not happen with the types here, but for
consistency I am removing the old API version here too.

[1] fluxcd/image-automation-controller#139

Signed-off-by: Michael Bridgen <michael@weave.works>
hiddeco pushed a commit to fluxcd/image-reflector-controller that referenced this pull request Apr 21, 2021
The image automation part of the API has changed structure (see [1]),
and had a version bump from `v1alpha1` to `v1alpha2`. Since the types
here are also in `image.toolkit.fluxcd.io`, there will be less
complication if they also get a version bump even though they aren't
changing.

[1] fluxcd/image-automation-controller#139

Signed-off-by: Michael Bridgen <michael@weave.works>
hiddeco pushed a commit to fluxcd/image-reflector-controller that referenced this pull request Apr 21, 2021
The image automation part of the API has changed structure (see [1]),
and had a version bump from `v1alpha1` to `v1alpha2`. Since the types
here are also in `image.toolkit.fluxcd.io`, there will be less
complication if they also get a version bump even though they aren't
changing.

[1] fluxcd/image-automation-controller#139

Signed-off-by: Michael Bridgen <michael@weave.works>
hiddeco pushed a commit to fluxcd/image-reflector-controller that referenced this pull request Apr 21, 2021
The image automation part of the API has changed structure (see [1]),
and had a version bump from `v1alpha1` to `v1alpha2`. Since the types
here are also in `image.toolkit.fluxcd.io`, there will be less
complication if they also get a version bump even though they aren't
changing.

[1] fluxcd/image-automation-controller#139

Signed-off-by: Michael Bridgen <michael@weave.works>
hiddeco pushed a commit to fluxcd/image-reflector-controller that referenced this pull request Apr 21, 2021
The image automation part of the API has changed structure (see [1]),
and had a version bump from `v1alpha1` to `v1alpha2`. Since the types
here are also in `image.toolkit.fluxcd.io`, there will be less
complication if they also get a version bump even though they aren't
changing.

[1] fluxcd/image-automation-controller#139

Signed-off-by: Michael Bridgen <michael@weave.works>
@stefanprodan stefanprodan merged commit 7c39649 into main Apr 22, 2021
@stefanprodan stefanprodan deleted the v1alpha2-api branch April 22, 2021 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants