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

add GatewayConditionAccepted and GatewayReasonAccepted #1447

Merged
merged 2 commits into from
Oct 11, 2022
Merged

add GatewayConditionAccepted and GatewayReasonAccepted #1447

merged 2 commits into from
Oct 11, 2022

Conversation

mikemorris
Copy link
Contributor

@mikemorris mikemorris commented Oct 10, 2022 •
edited
Loading

What type of PR is this?
/kind cleanup
/kind documentation
/kind api-change
/kind deprecation

What this PR does / why we need it:
Implements the recommendation in GEP-1364 to replace GatewayConditionScheduled with GatewayConditionAccepted

Which issue(s) this PR fixes:

Refs #1111, #1364

Does this PR introduce a user-facing change?:

Adds GatewayConditionAccepted and GatewayReasonAccepted
Deprecates GatewayConditionScheduled and GatewayReasonScheduled

This status condition does not appear to be currently checked in conformance tests, which mostly rely on theReady condition instead. We could either add checks for this condition now, or in a subsequent pull request.

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/documentation Categorizes issue or PR as related to documentation. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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. labels Oct 10, 2022
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 10, 2022
@mikemorris
add GatewayConditionAccepted and GatewayReasonAccepted
990299c 
deprecate GatewayConditionScheduled and GatewayReasonScheduled
@mikemorris mikemorris mentioned this pull request Oct 10, 2022
Closed
7 tasks
@robscott robscott added this to the v0.6.0 milestone Oct 10, 2022
shaneutt
shaneutt reviewed Oct 11, 2022
View reviewed changes
apis/v1alpha2/gateway_types.go Outdated
// true.
// True. This reason is deprecated and GatewayReasonAccepted should be used
// with GatewayConditionAccepted instead.
// TODO: is there a need/way to add a kubebuilder annotation here?
Copy link
Member

Choose a reason for hiding this comment

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

I am only aware of the kubebuilder:deprecatedversion tag as per the kubebuilder CRD generation documentation, which does not apply here because it marks the entire version as deprecated. I am not aware of any way to call a specific field deprecated. I think we can just use the godoc you've already provided explaining that it's deprecated for now, maybe we can open/find an upstream issue to add something that automates this for us in kubebuilder later.

Copy link
Member

Choose a reason for hiding this comment

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

Something similar was proposed but ended up getting turned down: kubernetes/enhancements#3340.

@mikemorris
Copy link
Contributor Author

Removed TODOs and switched deprecation warnings from passive to active voice, think this should be ready to go now.

@robscott
Copy link
Member

Thanks!

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 11, 2022
@k8s-ci-robot k8s-ci-robot merged commit 295356b into kubernetes-sigs:main Oct 11, 2022
@mikemorris mikemorris deleted the gateway-condition-accepted branch October 12, 2022 17:18
@skriss skriss mentioned this pull request Oct 13, 2022
Closed
3 tasks
@howardjohn
Copy link
Contributor

Give v0.6.0 is not a new API version, what is the expected behavior of implementors here? Must set both?

@arkodg arkodg mentioned this pull request Dec 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robscott robscott robscott left review comments

@shaneutt shaneutt shaneutt left review comments

@howardjohn howardjohn Awaiting requested review from howardjohn

Assignees

@robscott robscott

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/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v0.6.0
Development

Successfully merging this pull request may close these issues.
5 participants
@mikemorris @robscott @k8s-ci-robot @howardjohn @shaneutt