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.

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

Beta resources: Setting storage version to v1beta1, not serving v1alpha2 #1348

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Beta resources: Setting storage version to v1beta1, not serving v1alpha2 #1348

merged 1 commit into from
Sep 1, 2022

Conversation

robscott
Copy link
Member

What type of PR is this?
/kind cleanup
/kind deprecation

What this PR does / why we need it:
This affects resources that graduated to v1beta1 in the v0.5.0 release.
Although that specific change is relatively small, it required a lot of
related work:

  • Restructuring examples to remove v1alpha2 examples where they were no
    longer useful and their installation would fail
  • Updating conformance tests to use v1beta1 where possible
  • Some related fixes to v1alpha2 blog post to account for removal of
    v1alpha2 example

This is closely related to #1347

Does this PR introduce a user-facing change?:

GatewayClass, Gateway, and HTTPRoute are now only supported with the v1beta1 version of the API. The v1alpha2 API versions of these resources will be fully removed in a future release.

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 20, 2022
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 20, 2022
@robscott
Copy link
Member Author

This is a big change. Adding a hold for consensus.

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 20, 2022
@robscott robscott added this to the v0.6.0 milestone Aug 20, 2022
@shaneutt shaneutt self-requested a review August 22, 2022 16:43
youngnick
youngnick reviewed Aug 29, 2022
View reviewed changes
Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

Wow, this was a surprisingly big one. Most of the changes are pretty straightforward renames, but this probably needs someone to do another visual check through the docs and make sure they make sense (I am going a bit crosseyed).

/lgtm

hack/verify-examples-kind.sh
@@ -106,6 +105,9 @@ for CHANNEL in experimental standard; do
echo Examples failed as expected
done

# Undo workaround from earlier
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh, this is a good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

"Temporary workaround for release" sounds like something that should just be removed? If it's needed permanently, maybe we need to update the comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're right, this probably shouldn't exist at all. There was a time where we wanted the manifests installed from the main branch to be at least somewhat useful and refer to the latest released image instead of the latest image build from the main branch. All of our installation instructions clearly refer to a versioned release so this is likely entirely unnecessary, will revert this change altogether.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually as I was digging into this more, it looks more complicated than initially expected and I don't want to expand this PR any further. I've created #1366 to track this and added it to the v0.6.0 milestone.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 29, 2022
@markmc
Copy link
Contributor

markmc commented Aug 29, 2022

This needs updating in admission_webhook.yaml doesn't it?

webhooks:
  - name: validate.gateway.networking.k8s.io
    matchPolicy: Equivalent
    rules:
      - operations: [ "CREATE" , "UPDATE" ]
        apiGroups: [ "gateway.networking.k8s.io" ]
        apiVersions: [ "v1alpha2" ]
        resources: [ "gateways", "gatewayclasses", "httproutes" ]

@robscott
Copy link
Member Author

Good catch @markmc! Should be fixed by #1365

@robscott robscott mentioned this pull request Aug 30, 2022
Closed
@markmc
Copy link
Contributor

markmc commented Aug 30, 2022

/lgtm

shaneutt
shaneutt approved these changes Aug 30, 2022
View reviewed changes
Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: robscott, shaneutt

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

@robscott
Copy link
Member Author

Fixes #1372

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 31, 2022
@robscott robscott linked an issue Aug 31, 2022 that may be closed by this pull request
Conformance tests using v1beta1 #1372
Closed
@robscott
Beta resources: Setting storage version to v1beta1, not serving v1alpha2
5f296d8 
This affects resources that graduated to v1beta1 in the v0.5.0 release.
Although that specific change is relatively small, it required a lot of
related work:

* Restructuring examples to remove v1alpha2 examples where they were no
  longer useful and their installation would fail
* Updating conformance tests to use v1beta1 where possible
* Some related fixes to v1alpha2 blog post to account for removal of
  v1alpha2 example
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 1, 2022
@robscott
Copy link
Member Author

robscott commented Sep 1, 2022

Missed updating one conformance test to use v1beta1, PTAL.

youngnick
youngnick reviewed Sep 1, 2022
View reviewed changes
Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 1, 2022
@k8s-ci-robot k8s-ci-robot merged commit 1afb35a into kubernetes-sigs:main Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmc markmc markmc left review comments

@youngnick youngnick youngnick left review comments

@shaneutt shaneutt shaneutt approved these changes

@bowei bowei Awaiting requested review from bowei

Assignees

@markmc markmc

@shaneutt shaneutt

@youngnick youngnick

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v0.6.0
Development

Successfully merging this pull request may close these issues.

Conformance tests using v1beta1
5 participants
@robscott @markmc @k8s-ci-robot @shaneutt @youngnick