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

Remove Kuryr types #1652

Merged
merged 1 commit into from Nov 30, 2023
Merged

Remove Kuryr types #1652

merged 1 commit into from Nov 30, 2023

Conversation

dulek
Copy link
Contributor

@dulek dulek commented Nov 7, 2023

Kuryr was deprecated since 4.12 and is being removed in 4.15. This commit gets rid of all the Kuryr stuff in the API.

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 7, 2023

Hello @dulek! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 7, 2023
@JoelSpeed
Copy link
Contributor

@dulek Can you prove to me that no one is still using Kuryr? What happens to some in 4.14 if they are still using it and try to upgrade to 4.15?

@dulek
Copy link
Contributor Author

dulek commented Nov 13, 2023

@dulek Can you prove to me that no one is still using Kuryr? What happens to some in 4.14 if they are still using it and try to upgrade to 4.15?

We aim to solve the upgrade issue at openshift/cluster-network-operator#2096. CNO already has no ability to deploy with Kuryr and openshift/installer#7675 should block it in the installer.

All that being said I'm not sure we're able to remove any fields, the verify-crd-schema is complaining about this patch.

@cuppett
Copy link
Member

cuppett commented Nov 14, 2023

Confirmed/approved on OCP side on this removal in 4.15 timeframe.

cc @abhat @deads2k @fepan @dhellmann

@deads2k
Copy link
Contributor

deads2k commented Nov 15, 2023

Confirmed/approved on OCP side on this removal in 4.15 timeframe.

cc @abhat @deads2k @fepan @dhellmann

Thanks.

@dulek the block on upgrade and install must merge before this PR. Then I can override the "cannot remove existing fields" failure. Please ping me on slack when it is time.

@dulek
Copy link
Contributor Author

dulek commented Nov 23, 2023 •
edited

@deads2k: openshift/release#45275, openshift/installer#7675 and openshift/cluster-network-operator#2096 are merged now.

@dulek
Copy link
Contributor Author

dulek commented Nov 27, 2023

@deads2k: And openshift/cincinnati-graph-data#4430 is in too!

@deads2k
Copy link
Contributor

deads2k commented Nov 27, 2023

/override ci/prow/verify-crd-schema

ack from Cuppett, edges blocked

/lgtm
/approve

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 27, 2023

@deads2k: Overrode contexts on behalf of deads2k: ci/prow/verify-crd-schema

In response to this:

/override ci/prow/verify-crd-schema

ack from Cuppett, edges blocked

/lgtm
/approve

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.

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 27, 2023
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 7955d3d and 2 for PR HEAD 16eb14d in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 6ceea94 and 1 for PR HEAD 16eb14d in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD d83ab5b and 0 for PR HEAD 16eb14d in total

@openshift-ci-robot
Copy link

/hold

Revision 16eb14d was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2023
@dulek
Copy link
Contributor Author

dulek commented Nov 28, 2023

/hold cancel
/test e2e-aws-serial-techpreview

@deads2k: Can you override verify-crd-schema again? Seems like it lost it on the retest.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2023
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD d83ab5b and 2 for PR HEAD 16eb14d in total

@JoelSpeed
Copy link
Contributor

/override ci/prow/verify-crd-schema

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 28, 2023

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema

In response to this:

/override ci/prow/verify-crd-schema

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.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD e1845c5 and 1 for PR HEAD 16eb14d in total

@dulek
Copy link
Contributor Author

dulek commented Nov 28, 2023

/retest-required

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2023
@dulek
Copy link
Contributor Author

dulek commented Nov 28, 2023

I needed to resolve a merge conflict.

@dulek
Remove Kuryr types
b303172 
Kuryr was deprecated since 4.12 and is being removed in 4.15. This
commit gets rid of all the Kuryr stuff in the API.
@dulek
Copy link
Contributor Author

dulek commented Nov 28, 2023

/retest-required

1 similar comment
@dulek
Copy link
Contributor Author

dulek commented Nov 29, 2023

/retest-required

@dulek
Copy link
Contributor Author

dulek commented Nov 29, 2023

@JoelSpeed: Would you care to override verify-crd-schema again?

@JoelSpeed
Copy link
Contributor

/override ci/prow/verify-crd-schema
/retest-required

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 29, 2023

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema

In response to this:

/override ci/prow/verify-crd-schema
/retest-required

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.

@pierreprinetti
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 29, 2023
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 29, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, dulek, pierreprinetti

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

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 03f577b and 2 for PR HEAD b303172 in total

@JoelSpeed
Copy link
Contributor

/override ci/prow/verify-crd-schema

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 29, 2023

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema

In response to this:

/override ci/prow/verify-crd-schema

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.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD a782d1c and 1 for PR HEAD b303172 in total

@dulek
Copy link
Contributor Author

dulek commented Nov 29, 2023

@JoelSpeed: Could you help once again?

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 7f2a17e and 0 for PR HEAD b303172 in total

@openshift-ci-robot
Copy link

/hold

Revision b303172 was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 29, 2023
@dulek
Copy link
Contributor Author

dulek commented Nov 30, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 30, 2023
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD be20875 and 2 for PR HEAD b303172 in total

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 30, 2023 •
edited

@dulek: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-azure b303172 link false /test e2e-azure

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@JoelSpeed
Copy link
Contributor

/override ci/prow/verify-crd-schema

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 30, 2023

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema

In response to this:

/override ci/prow/verify-crd-schema

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.

@openshift-merge-bot openshift-merge-bot bot merged commit f6ae107 into openshift:master Nov 30, 2023
15 of 16 checks passed
@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-config-api-container-v4.15.0-202311301610.p0.gf6ae107.assembly.stream for distgit ose-cluster-config-api.
All builds following this will include this PR.

@dulek dulek mentioned this pull request Jan 10, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

@mfojtik mfojtik Awaiting requested review from mfojtik

Assignees

@pierreprinetti pierreprinetti

@deads2k deads2k

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. 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.

None yet
7 participants
@dulek @JoelSpeed @cuppett @deads2k @openshift-ci-robot @pierreprinetti @openshift-bot