Skip to content

Support multiple versions of a CRD #94

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

Closed
csviri opened this issue May 4, 2020 · 18 comments · Fixed by #879
Closed

Support multiple versions of a CRD #94

csviri opened this issue May 4, 2020 · 18 comments · Fixed by #879
Assignees

Comments

@csviri
Copy link
Collaborator

csviri commented May 4, 2020

Within one CRD there could be multiple versions defined, we need to explicitly support this.

@csviri csviri self-assigned this Jun 5, 2020
@stoetti
Copy link
Contributor

stoetti commented Mar 3, 2021

@csviri Is this on the roadmap in the near future?

@csviri
Copy link
Collaborator Author

csviri commented Mar 3, 2021

@stoetti yes, this should be actually one of the priorities in the close future.

pinging also @metacosm

@metacosm
Copy link
Collaborator

metacosm commented Mar 3, 2021

This should actually work already, no? If not, could you explicit a scenario that doesn't work?

@csviri
Copy link
Collaborator Author

csviri commented Mar 3, 2021

@metacosm good question, went though again on the docs:
https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning

  1. If we see this in a way that we are the client only of the API, and the conversion hooks are properly configured for a custom resource, the migration can be seamlessly done by upgrading the operator to support JUST the most recent version of a CR.
    While Kubernetes API should support multiple versions at the same time, the intention is that the clients (in this case the operator) is migrated eventually to the new version. So it's not the case that we have to support 2 versions of the same CR at the same time.
    To be honest I don't fully understand what happens if the conversion strategy is None (maybe worth to try it out).

  2. In addition to that, supporting conversion webhooks is out of the scope of the framework IMHO, or could be part of an isolated subproject of the SDK (not the framework), but that is a longer discussion and/or an other issue.

So IMHO, if nobody else sees it otherwise (@metacosm @stoetti @adam-sandor ? ) we are actually good on this. There is nothing to be done. I would even close the issue.

@stoetti
Copy link
Contributor

stoetti commented Mar 4, 2021

I gave it a try without using the conversionStrategy 'None'. In that case k8s just changes the apiVersion to the version thats marked with stored=true. This obviously leads to a deserialization-error if there is an incompatible change in the CRD.

I will work on a sample with a conversion webhook and see if there are any aspects that are worth building into a subproject. From the documentation the conversion-webhook is just a simple REST endpoint serving a specific interface.

@csviri
Copy link
Collaborator Author

csviri commented Mar 4, 2021

I gave it a try without using the conversionStrategy 'None'. In that case k8s just changes the apiVersion to the version thats marked with stored=true. This obviously leads to a deserialization-error if there is an incompatible change in the CRD.

yes, just don't know how we would support that, probably does not make sense at all to do. Not sure how this is meant to be used, probably would be worth asking on kuberentes forums.

I will work on a sample with a conversion webhook and see if there are any aspects that are worth building into a subproject. From the documentation the conversion-webhook is just a simple REST endpoint serving a specific interface.

Sure, pls do, what I mean is since this is a mapping basically between json structures there is not much that can be done on a framework level to support this. Setting up such a service is already quite simple using just an application framework (spring, quarkus).

@metacosm
Copy link
Collaborator

metacosm commented Mar 4, 2021

From my perspective, different versions should be backed by different POJOs (though I understand there might be cases where that's not practical), each "registered" using a different version annotation so there shouldn't be a deserialization issue. Of course, if there is no backing object, there really is nothing the SDK can do… :)

@csviri csviri closed this as completed Mar 4, 2021
@csviri csviri reopened this Mar 4, 2021
@csviri
Copy link
Collaborator Author

csviri commented Mar 4, 2021

(Sorry this was an accident closing it)

Maybe I don't understand this correctly, but the point of these conversion webhooks is to avoid to thinkin about multiple versions at the same time on client side. So having a conversion hook, basically makes it transparent for the client what version is actually used.
Thus, if somebody has an operator, and there is a new version of CRD (with a conversion servcie / webhook handler) in place, the operator can seamlessly migrate, just supporting one version in a time.
But will have to try it out.

@metacosm
Copy link
Collaborator

metacosm commented Mar 4, 2021

Well, this would only work if your client can handle whatever the server returns, right? So even if you only want to keep one version to deal with on the client, there still needs to be a POJO on the client that's able to handle the converted version.
That said, I'm not super familiar with conversion webhooks so maybe I'm missing something.

@metacosm metacosm closed this as completed Mar 4, 2021
@metacosm metacosm reopened this Mar 4, 2021
@csviri
Copy link
Collaborator Author

csviri commented Mar 4, 2021

So how I understand it correctly the default worflow looks like this:

  1. apply the CRD with version 1
  2. implement the operator with corresponding POJOs
    3.Update the CRD with version 2 also included (set as default) also prepare the webhook.
    At this point since there is a webhook, so operator still can work with version 1. Since if it watches the CR with version 1 the
    K8S API returns it with that version (using the conversion webhook to serve those)
  3. Update the Operator so it handles (only) version 2, deploy it - replacing the previous version of the operator. At this point it starts watching v2 resources, API serves those (without involving the conversion webhook from now on).

This means there is no point where the Operator actually needs to handle 2 CR version at the same time.

So the client have to handle only the version for which it asks for. This is good because in most cases there can be an easy migration happening.

@metacosm
Copy link
Collaborator

metacosm commented Mar 4, 2021

That's my understanding as well but the SDK has nothing to do in this scenario so this really isn't an issue, perhaps only something to document.

@csviri
Copy link
Collaborator Author

csviri commented Mar 4, 2021

Agree,
if no objection, I would close this issue, it will serve as a nice note on this topic.

@csviri csviri closed this as completed Mar 4, 2021
metacosm added a commit that referenced this issue Oct 29, 2021
While we originally planned to make it possible to register controllers
with the same CR but with different version (see #637), that behavior
should actually be forbidden since only one CR version can be served,
see #94 for more details.
metacosm added a commit that referenced this issue Oct 29, 2021
While we originally planned to make it possible to register controllers
with the same CR but with different version (see #637), that behavior
should actually be forbidden since only one CR version can be served,
see #94 for more details.
metacosm added a commit that referenced this issue Oct 29, 2021
While we originally planned to make it possible to register controllers
with the same CR but with different version (see #637), that behavior
should actually be forbidden since only one CR version can be served,
see #94 for more details.
metacosm added a commit that referenced this issue Oct 29, 2021
While we originally planned to make it possible to register controllers
with the same CR but with different version (see #637), that behavior
should actually be forbidden since only one CR version can be served,
see #94 for more details.
adam-sandor pushed a commit that referenced this issue Nov 5, 2021
While we originally planned to make it possible to register controllers
with the same CR but with different version (see #637), that behavior
should actually be forbidden since only one CR version can be served,
see #94 for more details.
@csviri
Copy link
Collaborator Author

csviri commented Jan 10, 2022

This popped up again, consider a migration path from v1 to v2 of CR and related Reconciler:

  1. Implemented an operator for version v1 CR v1
  2. CR v2 is developed and deployed with a conversion hook (does not matter which one is the storage version)
  3. The Operator will have a separate reconciler/controller which already asks for resources (maybe filtered by label) with version v2.
  4. After v2 with is tested with a subset of resources, it can be switched to just use the v2 of the CR and Controller.

In other words actually to support kinda canary testing within one operator with multiple controllers, so we would allow to have a separate controller for different version of CR.

To be done from our side, would be just to remove the current check, and create an Integration test that showcases this approach.

@csviri csviri reopened this Jan 10, 2022
@csviri
Copy link
Collaborator Author

csviri commented Jan 10, 2022

pls let me know what do you think @metacosm @adam-sandor @andreaTP

@andreaTP
Copy link
Collaborator

I completely agree with this last statement @csviri .
More specifically Kubernetes does allow to serve multiple CRDs versions while only one will be marked as storage.

I do believe that the operator-sdk should provide the infrastructure to handle this situation to end-users:

  • Guarantee the alignment with the version marked as storage comparing the installed CRD versions and the code
  • Guarantee the alignment of served CRDs versions
  • Dispatch updates only to the correct controller filtering out the rest before deserializing the CR

The end user can than decide how to handle this situation, simply throwing an exception, logging, updating the status etc...

@csviri
Copy link
Collaborator Author

csviri commented Jan 13, 2022

@andreaTP @metacosm I think we also do this after v2, checked and would like then create an e2e test for it, that would require a conversion hook etc, so this is not just for few hours to do it in good quality. Since it's not an API change will do it after v2.0.0 released, and will be one of the issues in v2.1.0.

@csviri
Copy link
Collaborator Author

csviri commented Jan 13, 2022

A workaround for now is to implement both the controllers in the code base, put there a feature flag which one to register and deploy two instances of the operator, where the feature flag is the difference.

@csviri
Copy link
Collaborator Author

csviri commented Jan 18, 2022

(related docs in kubebuilder: https://book.kubebuilder.io/multiversion-tutorial/tutorial.html )

@csviri csviri linked a pull request Jan 28, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants