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

Handle deprecated fields during conversion from v1beta1 to v1 #4546

Closed
lbernick opened this issue Feb 4, 2022 · 7 comments · Fixed by #6623
Closed

Handle deprecated fields during conversion from v1beta1 to v1 #4546

lbernick opened this issue Feb 4, 2022 · 7 comments · Fixed by #6623
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@lbernick
Copy link
Member

lbernick commented Feb 4, 2022

This issue is for discussing how to implement conversion between v1beta1 and v1 CRDs, as proposed in TEP-0096. This conversation came up in tektoncd/community#587.

One question we will need to answer is how to handle the "resources" field of Task/TaskRun/Pipeline/PipelineRun, since these will be removed in v1.

Copying over a comment from @sbwsg:

We serialize the Finally section of a v1beta1 Pipeline into an annotation if a client requests the v1alpha1 version. When a client submits a v1alpha1 version of a Pipeline with that annotation present the process reverses and the v1beta1 object is recreated with the Finally section in-tact:

// serializeFinally serializes a list of Finally Tasks to the annotations
// of an object's metadata section. This can then be used to re-instantiate
// the Finally Tasks when converting back up to v1beta1 and beyond.
func serializeFinally(meta *metav1.ObjectMeta, finally []v1beta1.PipelineTask) error {
if len(finally) != 0 {
b, err := json.Marshal(finally)
if err != nil {
return err
}
if meta.Annotations == nil {
meta.Annotations = make(map[string]string)
}
meta.Annotations[finallyAnnotationKey] = string(b)
}
return nil
}
// deserializeFinally populates a PipelineSpec's Finally list
// from an annotation found on resources that have been previously
// converted down from v1beta1 to v1alpha1.
func deserializeFinally(meta *metav1.ObjectMeta, spec *v1beta1.PipelineSpec) error {
if meta.Annotations != nil {
if str, ok := meta.Annotations[finallyAnnotationKey]; ok {
finally := []v1beta1.PipelineTask{}
if err := json.Unmarshal([]byte(str), &finally); err != nil {
return fmt.Errorf("error converting finally annotation into beta field: %w", err)
}
delete(meta.Annotations, finallyAnnotationKey)
if len(meta.Annotations) == 0 {
meta.Annotations = nil
}
spec.Finally = finally
}
}
return nil
}

We don't do this for any other v1beta1-only fields. So, for example, when a v1alpha1 client requests a v1beta1 Pipeline with When Expressions, we simply drop them on the floor. If that client then re-applies the v1alpha1 version the stored v1beta1 version will be updated and the when expressions will be gone.

For a while we were working on an approach where we take the entire v1beta1 object and serialize it into an annotation when clients request v1alpha1, but it proved to be a complex and imperfect workaround.

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 5, 2022
@vdemeester
Copy link
Member

/lifecycle frozen

@tekton-robot tekton-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 5, 2022
@abayer
Copy link
Contributor

abayer commented Jun 23, 2022

Should this be folded into #4983?

@vdemeester
Copy link
Member

probably 🙃

@lbernick
Copy link
Member Author

Added it to the linked issues in that epic! I think this still has value remaining open as a subtask, to discuss what we want to do when we update the stored version to v1 and need to implement conversion from v1beta1 to v1.

@lbernick lbernick changed the title Conversion between v1beta1 and v1 CRDs Handle deprecated fields during conversion from v1beta1 to v1 Jul 29, 2022
@lbernick
Copy link
Member Author

@afrittoli if you have any context about past pitfalls we ran into when supporting deprecated fields on conversion from v1alpha1 -> v1beta1 this issue is probably the best place for it!

@vdemeester
Copy link
Member

We don't do this for any other v1beta1-only fields. So, for example, when a v1alpha1 client requests a v1beta1 Pipeline with When Expressions, we simply drop them on the floor. If that client then re-applies the v1alpha1 version the stored v1beta1 version will be updated and the when expressions will be gone.

Note that this can happen no matter what we do 🙃. If the user / tool cleans the annotation(s), the situation is the same 👼🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants