Skip to content

Update CRD definition to use apiextensions v1 #3236

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 22, 2020
Merged

Update CRD definition to use apiextensions v1 #3236

merged 1 commit into from
Sep 22, 2020

Conversation

vdemeester
Copy link
Member

@vdemeester vdemeester commented Sep 15, 2020 •
edited
Loading

Changes

Using apiextensions.k8s.io/v1 for the Custom Resource Definition,
which updates the webhook conversion.

Signed-off-by: Vincent Demeester [email protected]

I wonder if this would help fixing #3206 (because of conversionReviewVersions: ["v1beta1"]), but it's still a bit dark magic to me.

/cc @tektoncd/core-maintainers

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes tests (if functionality changed/added)
  • Includes docs (if user facing)
  • Commit messages follow commit message best practices
  • Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

Update CRD to use apiextensions.k8s.io/v1 instead of v1beta1

/kind feature

@tekton-robot tekton-robot requested a review from a team September 15, 2020 08:24
@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 15, 2020
@vdemeester
Copy link
Member Author

/retest

@dibyom
Copy link
Member

dibyom commented Sep 16, 2020

unknown field "JSONPath" in io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.CustomResourceColumnDefinition,

Looks like we'd have to update some CRD definitions

@vdemeester
Copy link
Member Author

@dibyom oh the case is not the right one, will fix.

@vdemeester vdemeester force-pushed the apiextensions-v1 branch 2 times, most recently from 5aa27f3 to 4e1ddc3 Compare September 17, 2020 13:18
@vdemeester
Update CRD definition to use apiextensions v1
7530c4f 
Using `apiextensions.k8s.io/v1` for the Custom Resource Definition,
which updates the webhook conversion.

Signed-off-by: Vincent Demeester <[email protected]>
@dibyom
Copy link
Member

dibyom commented Sep 17, 2020

Release Notes for CRD version upgrade to v1: kubernetes/kubernetes#79604

dibyom
dibyom reviewed Sep 17, 2020
View reviewed changes
@dibyom
Copy link
Member

dibyom commented Sep 17, 2020

LGTM -- just one question about using the more "advanced" YAML syntax 😛

@dibyom
Copy link
Member

dibyom commented Sep 18, 2020

/lgtm

Fairly minor but it does make the configs a bit harder to read since you have to be familiar with the YAML << operator as well as anchors.

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 18, 2020
@vdemeester
Copy link
Member Author

Fairly minor but it does make the configs a bit harder to read since you have to be familiar with the YAML << operator as well as anchors.

@dibyom to be fair, it's gonna get worse when we generate this from code and have the schema filled 😝.

@afrittoli
Copy link
Member

/cc @tektoncd/core-maintainers

Fancy :D

afrittoli
afrittoli reviewed Sep 22, 2020
View reviewed changes
Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this.
v1beta1 has been deprecated in v1.16 in favor of apiextensions.k8s.io/v1, so definitely had to update the version :)

@afrittoli
Copy link
Member

/approve

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 22, 2020
@vdemeester
Copy link
Member Author

/test pull-tekton-pipeline-integration-tests

@vdemeester
Copy link
Member Author 

    --- FAIL: TestExamples/v1beta1/taskruns/cloud-event (0.03s)

/test pull-tekton-pipeline-integration-tests

@tekton-robot tekton-robot merged commit d59d594 into tektoncd:master Sep 22, 2020
@vdemeester vdemeester deleted the apiextensions-v1 branch September 22, 2020 13:03
@bobcatfish
Copy link
Collaborator

@vdemeester does this have any implications for folks when they upgrade?

@vdemeester
Copy link
Member Author

@vdemeester does this have any implications for folks when they upgrade?

I don't think there is any impact on upgrade (mainly because it get auto converted to v1) but this is something I need to make sure of.

@vdemeester
Copy link
Member Author

@bobcatfish just tried it, it just go smoothly 👼

@imjasonh imjasonh mentioned this pull request Mar 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afrittoli afrittoli afrittoli left review comments

@dibyom dibyom dibyom left review comments

Assignees

@dibyom dibyom

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vdemeester @dibyom @afrittoli @tekton-robot @bobcatfish