tektoncd / results

Long term storage of execution results.
Apache License 2.0
73 stars 69 forks source link

Support Tekton v1 APIs #303

Open adambkaplan opened 1 year ago

adambkaplan commented 1 year ago

Feature request

With Tekton Pipelines v0.43.0, we have our first version of Tekton v1 APIs. Results should be able to support reconciliation of v1 objects.

Use case

Archive Tekton v1 versions of TaskRun and PipelineRun objects.

adambkaplan commented 1 year ago

It appears that dependabot's blind update of pipelines to 0.43.0 introduced breaking changes: #304.

I think we should hold on this until v0.5.0 is released.

khrm commented 1 year ago

/assign khrm

khrm commented 1 year ago

I think we should do a release of v0.5.0 with v1. I would raise the pr. on Monday. v1 has been out for a while and now it doesn't make sense to do a v0.5.0 without v1.

@vdemeester

vdemeester commented 1 year ago

/area roadmap

adambkaplan commented 1 year ago

@khrm something that we need to consider is backwards compatibility. Results can be backwards-compatible if we provide an option for v1beta1 reconciliation. That won't be the case if we only reconcile v1 APIs. We have a choice here:

  1. Keep it simple, only reconcile v1 APIs, and require Pipelines v0.43.0 or higher.
  2. Add the complexity of reconciling one of two apiVersions.

I'm inclined to favor option 1 after we cut a release of our current body of work.

khrm commented 1 year ago

@adambkaplan We decided to do a release based on v1beta1 APIs in the WG call. Afterward, we would upgrade.

drriguz commented 1 year ago

@adambkaplan We decided to do a release based on v1beta1 APIs in the WG call. Afterward, we would upgrade.

Does that means support for v1beta1 api will be EOL after we upgraded to v1? Pipeline v0.41.x is a LTS version, do we also consider to provider a lts version of results/cli/chains that supports it?

khrm commented 1 year ago

CLI should have LTS for v1beta1 pipelines, @tektoncd/cli-maintainers can confirm.

@tektoncd/chains-maintainers can clarify whether we would have lts release for Pipelines v1beta1 and chains.

Results is still alpha project. So it might not be feasible but you can use v1beta1 with v1 records. Fields unique to v1beta1 are available in annotations.

tekton-robot commented 1 year ago

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.

adambkaplan commented 1 year ago

/lifecycle frozen

:thinking: I wonder if we need a TEP to discuss this. We currently reconcile v1beta1 APIs across the board. We could employ similar techniques that k8s does if we switch the "storage" to v1 for PipelineRun and TaskRun data, but allow clients to specify if they want PipelineRun/TaskRuns with v1beta1 apiVersion.

@dibyom @alan-ghelardi @khrm @vdemeester thoughts?

khrm commented 11 months ago

v1beta1 is now deprecated. Previously, annotations were used to transfer information during conversion. @vdemeester, do we truly need to provide support for this? Wouldn't it be simpler to just merge #526 after rebase? The results is still in alpha, this change won't break anything. Old information will still exist.

vdemeester commented 11 months ago

@khrm So as long as tektoncd/pipeline publish v1beta1, it's ok for results to use it. And v1beta1 is there for at least a year (counting from ~21st of July). This gives time for results to decide how to handle this.

I think at some point, results should switch to using v1 objects by default (as "the thing that gets watched") while still being able to read older objects (v1beta1). And it could even be done gradually through a feature flag, that would initially default to v1beta1 and at some point default to v1 instead.

cc @tektoncd/chains-maintainers as this is very similar.

khrm commented 11 months ago

We store v1beta1 stuff as jsonb in postgres. And the field in the protobuff is also a Any. So we will be able to show v1beta1 even after we move to v1. New objects will be stored as v1 and shown as v1 when we fetch them. The old data will continue to show as v1beta1. So we can move to v1 for watching and storing now.

@vdemeester @dibyom @sayan-biswas

adambkaplan commented 11 months ago

This creates an interesting challenge for long-running instances. When we switch to reconcile with v1 objects, the database will have some information as v1, and others as v1beta1. This can be a challenge for clients (cli, web consoles) that currently expect v1beta1 objects.

What can we do here?

Note that many of the options above are not mutually exclusive.

sayan-biswas commented 11 months ago

A combination of the 2nd and 3rd option would solve the purpose, assuming that no clients will try to fetch the objects from the database directly. Most of the process would be transparent.

khrm commented 10 months ago

I have created a TEP here https://github.com/tektoncd/community/pull/1055 to discuss in WG API call and bring attention to wider community.

dibyom commented 10 months ago

Launch a job that does the object conversion in the background?

This sounds a bit like the storage version migrator but for the results db: https://github.com/kubernetes-sigs/kube-storage-version-migrator/blob/master/USER_GUIDE.md

adambkaplan commented 10 months ago

This sounds a bit like the storage version migrator but for the results db: https://github.com/kubernetes-sigs/kube-storage-version-migrator/blob/master/USER_GUIDE.md

Exactly!

manuelwallrapp commented 2 months ago

Is this issue planned to implement in some near future?

khrm commented 2 months ago

Yes. I will target it after Event storage.

manuelwallrapp commented 2 months ago

For the Event storage we are waiting too, so this gonna be a big release for us. Thank you @khrm.