Add conversion for remote tasks and pipelines to support v1 #6254
Conversation
|
Skipping CI for Draft Pull Request. |
tekton-robot
added
release-note-none
tekton-robot
added
the
size/L
Yongxuanzhang
force-pushed
the
resolution-v1
branch
from
2bff54f to
5ecee69
Compare
tekton-robot
added
release-note
tekton-robot
removed
the
do-not-merge/work-in-progress
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
/lgtm |
There was a problem hiding this comment.
Thanks @Yongxuanzhang
This lgtm. Just a small comment if we would want to also include some invalid pipeline/task conversion
I'm trying to find a case where we can return the error, but it looks like we will always have errors before this one, |
Yongxuanzhang
force-pushed
the
resolution-v1
branch
from
5ecee69 to
b67c9e7
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
There was a problem hiding this comment.
Thanks @Yongxuanzhang! We could also leave a todo comment here to convert to v1 once the migration is completed, not a blocker and
/lgtm
|
@QuanZhang-William: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/assign |
I think it might be necessary to support v1beta1 till it deprecates? |
Oh thanks for the reminder! I will add a todo there |
| if err := t.ConvertFrom(ctx, obj); err != nil { | ||
| return nil, err | ||
| } | ||
| return t, nil | ||
| } | ||
|
|
||
| return nil, errors.New("resource is not a pipeline") |
There was a problem hiding this comment.
can you please add a test for this case? (and same for taskref)
There was a problem hiding this comment.
I don't think we could add a test for this, the error will be caught by code before this one
tekton-robot
added
the
approved
This commit adds conversion for remote tasks and pipelines after they are resolved by resolvers to support v1 types. Before this commit v1 tasks and pipelines are not supported in remote resolution since we use v1beta1 as the storage version and in reconciler we will convert v1 crd into v1beta. But this conversion is missing for remote resources. After we change the storage version to v1 we should update the code to support v1beta. Signed-off-by: Yongxuan Zhang [email protected]
Yongxuanzhang
force-pushed
the
resolution-v1
branch
from
b67c9e7 to
9676f17
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
Note that the coverage drop of these 2 files are inevitable, the error case is not possible in current funciton |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JeromeJu, lbernick, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
/test pull-tekton-pipeline-go-coverage-df |
|
@Yongxuanzhang: The specified target(s) for
The following commands are available to trigger optional jobs:
Use In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retest |
|
The following is the coverage report on the affected files.
|
Changes
This commit adds conversion for remote tasks and pipelines after they are resolved by resolvers to support v1 types. Before this commit v1 tasks and pipelines are not supported in remote resolution since we use v1beta1 as the storage version and in reconciler we will convert v1 crd into v1beta. But this conversion is missing for remote resources. After we change the storage version to v1 we should update the code to support v1beta.
Closes #6141
/kind bug
Signed-off-by: Yongxuan Zhang [email protected]
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes