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

Make ApiServerSource v1beta1 the stored version and add a migration tool #3630

Closed
capri-xiyue opened this issue Jul 17, 2020 · 5 comments · Fixed by #4031
Closed

Make ApiServerSource v1beta1 the stored version and add a migration tool #3630

capri-xiyue opened this issue Jul 17, 2020 · 5 comments · Fixed by #4031
Assignees
Labels
area/api area/test-and-release Test infrastructure, tests or release kind/feature-request priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. proposal/0.18
Milestone

Comments

@capri-xiyue
Copy link
Contributor

capri-xiyue commented Jul 17, 2020

Problem
In 0.17, we promoted ApiServerSource to v1beta1.
This is to track that we should make v1beta1 the storage version for 0.18, and we should add a migration tool for it.
Similar to: https://github.com/knative/eventing/blob/master/config/pre-install/v0.16.0/storage-version-migration.yaml.
But need to check whether the upgrade issue for v1alpha1 apiseversource gets solved or not:
https://github.com/knative/eventing/pull/2872/files#r401917238

Persona:
Which persona is this feature for?

Exit Criteria
ApiServerSource in v1beta1 and able to migrate

Time Estimate (optional):
How many developer-days do you think this may take to resolve?

Additional context (optional)
#3611

@lberk lberk added area/api area/test-and-release Test infrastructure, tests or release proposal/0.18 priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jul 27, 2020
@grantr grantr added this to the Backlog milestone Jul 27, 2020
@nachocano
Copy link
Contributor

/assign @nlopezgi

@nlopezgi
Copy link
Contributor

@n3wscott could you provide an update about https://github.com/knative/eventing/pull/2872/files#r401917238. I'm ready to start working on this issue but it seems something like #3895 will not be sufficient from what in can understand from the comment above? Thanks for any advice!

@grantr grantr modified the milestones: Backlog, v0.18.0 Aug 25, 2020
@nlopezgi
Copy link
Contributor

nlopezgi commented Sep 1, 2020

I've been looking in more detail at the effort required for this issue:
There are differences between v1alpha1 and v1beta1 for ApiServerSource that make the migration tool non trivial.
controller and controllerSelector defined in ApiServerResource have been deprecated.
The documentation mentions
"Per-resource controller flag will no longer be supported in v1alpha2, please use Spec.Owner as a GKV."
"Per-resource owner refs will no longer be supported in v1alpha2, please use Spec.Owner as a GKV."

My thought is that for this task its necessary to create a tool similar to #3110 for ApiServerSource.
The tool will look for v1alpha1 ApiServerSource and convert it to a v1beta1 ApiServerSource by looking at the controller and controllerSelector fields and converting them to the appropriate field.

I currently will not have the cycles to finish this task in the next ~2weeks. Unassigning myself for now.

/unassign

@capri-xiyue
Copy link
Contributor Author

capri-xiyue commented Sep 9, 2020

It looks like controller and controllerSelector defined in ApiServerResource have been deprecated since v1alpha2 and this is a break feature change.
Based on the code comments, it looks like in v1alpha1, we supported Per-resource owner refs and Per-resource controller flag. But after v1alpha2, we no longer support this Per-resource filter. I don't think we can create a tool similar to #3110 for ApiServerSource since this deprecation is a break feature change.
I checked Kubernetes Deprecation Policy](https://kubernetes.io/docs/reference/using-api/deprecation-policy/)
In k8s,
with whole REST resources, an individual field which was present in API v1 must exist and function until API v1 is removed. Unlike whole resources, the v2 APIs may choose a different representation for the field, as long as it can be round-tripped. For example a v1 field named "magnitude" which was deprecated might be named "deprecatedMagnitude" in API v2. When v1 is eventually removed, the deprecated field can be removed from v2.
But we didn't have deprecatedController and deprecatedControllerSelector defined in ApiServerSource v1alpha2 to make the roundtrip work.

I think for the migration of ApiSeverSource from v1alpha1 to v1alpha2 or v1beta1, we may have to introduce break change unless we add deprecatedController and deprecatedControllerSelector in ApiServerSource v1alpha2 and v1beta1.

What do you think of it? @n3wscott

@capri-xiyue
Copy link
Contributor Author

/assign @capri-xiyue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api area/test-and-release Test infrastructure, tests or release kind/feature-request priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. proposal/0.18
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants