test-infra
test-infra copied to clipboard
kubernetes
Add support for tektoncd/pipeline v1beta1 api version
This is a continuation of https://github.com/kubernetes/test-infra/issues/26535
We've already added the support for tektoncd/pipeline v0.36.0 in https://github.com/kubernetes/test-infra/pull/26762
The next phase is to add support for v1beta1 API versions
Hi @qaifshaikh. Thanks for your PR.
I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
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.
It seems tektoncd/pipeline deprecated the support for conditions in v1beta1.
But recently they completely removed it before releasing support for v1
Here's the issue that discusses it: https://github.com/tektoncd/pipeline/issues/3377
And this is the commit that removes it: https://github.com/tektoncd/pipeline/commit/9d60d0adf41e74f78029a0eaab73140fa4f7206a
I wonder if it's worth resolving the references for the conditions in v1beta1 support for prow? Or does it make sense to just get rid of it overall to prepare for v1 support later?
cc @eddycharly @olemarkus for input 🙏🏻
/ok-to-test (I would personally remove it completely and prepare the jump to v1)
I'm also for completely removing conditions and upgrade pipeline to 0.37+.
I'm also for completely removing conditions and upgrade pipeline to 0.37+.
+1
@qaifshaikh: The /test command needs one or more targets.
The following commands are available to trigger required jobs:
-
/test pull-test-infra-gubernator -
/test pull-test-infra-integration -
/test pull-test-infra-prow-checkconfig -
/test pull-test-infra-prow-image-build-test -
/test pull-test-infra-unit-test -
/test pull-test-infra-verify-lint
The following commands are available to trigger optional jobs:
-
/test pull-test-infra-bazel
Use /test all to run the following jobs that were automatically triggered:
-
pull-test-infra-integration -
pull-test-infra-prow-image-build-test -
pull-test-infra-unit-test -
pull-test-infra-verify-lint
In response to this:
/test
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.
Hmm 🤔 Being a noob, I'm not sure what's wrong here! I'd appreciate a review or feedback, please :)
cc @eddycharly @olemarkus
@qaifshaikh are you trying to replace v1alpha1 with v1beta1 ? Or to support both ?
@qaifshaikh are you trying to replace v1alpha1 with v1beta1 ? Or to support both ?
Yes @eddycharly , replacing pipeline v1alpha1 with v1beta1 but we need to keep the support for resource v1alpha1 since that API has no beta support
Why not support both alpha and beta ? Entirely removing alpha is going to break for existing users right ?
v1alpha1 is gradually going away. Doing a lot of work to support it seems futile. It will be a breaking change, but this repo doesn't have any concept of stable release, so I fear it is just something users will have to deal with. As far as I can tell no jobs in this repo depend on any v1alpha1-only features.
Supporting multiple versions makes sense to me. At some point we’ll need to support v1 too.
Shouldn’t adding support for beta and keeping current alpha support be less work that replacing ?
It would if not for the commits to tekton pipeline removing stuff from the alpha API.
Well they just got rid of a bunch of resources in https://github.com/tektoncd/pipeline/releases/tag/v0.38.0
Action required: v1alpha1 versions of Task, ClusterTask, TaskRun, Pipeline, and PipelineRun have been removed. Please use v1beta1 versions of these CRDs instead.
These were deprecated in a previous release so yeah we can't keep the support for v1alpha1
Just my opinion, I think we should support multiple versions. IMHO we should have done it a long time ago.
I feel like replacing alpha1 with beta1 is a bit of a hack and we’ll be in trouble again when v1 is out.
It’s just my opinion though, whatever works today is ok.
v1 is currently out after v0.37.0 and it removes all dependencies with v1alpha. So we won't be needing it all. Especially since its been deprecated and removed from the upstream now
It looks like https://github.com/kubernetes/test-infra/blob/aa928caf31ab0f30c2c999b2543790fbfa08e095/prow/test/integration/test/pod-utils_test.go#L557 fails, but i don't know how to get the why.
Pure guessing, the pipeline is not valid and the pod never gets created ? How is tekton deployed on the test cluster ?
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: olemarkus, qaifshaikh
Once this PR has been reviewed and has the lgtm label, please assign ameukam for approval by writing /assign @ameukam in a comment. For more information see:The Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@alvaroaleman we used it in my previous job but I'm pretty sure they switched to something else now.
@alvaroaleman
v1 was released a few weeks ago and was not completely stable and tekton added 2 patches in the following week for bug fixes.
I'm planning on doing a couple more PRs to upgrade tekton to v0.38.0 and then add support for v1 as well as it gets more stable
@alvaroaleman i've removed the irrelevant changes. Are we good to merge now?