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 deprecated v1alpha2 ClusterGroups #6049

Merged
merged 1 commit into from Mar 15, 2024
Merged

Remove deprecated v1alpha2 ClusterGroups #6049

merged 1 commit into from Mar 15, 2024

Conversation

luolanzone
Copy link
Contributor

No description provided.

antoninbas
antoninbas reviewed Mar 4, 2024
View reviewed changes
docs/api.md Outdated Show resolved Hide resolved
@luolanzone luolanzone force-pushed the remove-v1alpha2-clustergroups branch from 689c7b8 to 3527855 Compare March 6, 2024 01:54
@luolanzone luolanzone force-pushed the remove-v1alpha2-clustergroups branch from 3527855 to 8ea6b40 Compare March 11, 2024 02:02
antoninbas
antoninbas previously approved these changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/apis/crd/v1alpha2/webhook.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this PR, but I cannot figure what we are using this code for?

In particular, I cannot find any call to the RegisterWebhook function below.

@tnqn do you know if this is some unused legacy code?

Copy link
Member

Choose a reason for hiding this comment

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

I had the same question when it's pushed: #775 (comment) and Su explained it. I think it was supposed to be called from another probject that imports the package, and it may be no longer needed? Besides, controller-runtime has deprecated this pattern: kubernetes-sigs/controller-runtime#2639

Copy link
Contributor

Choose a reason for hiding this comment

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

@luolanzone I see that you deleted the file. Maybe we should do it in a separate PR to be on the safe side (and because it's unrelated to this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I will create another PR to remove this file.

@luolanzone luolanzone force-pushed the remove-v1alpha2-clustergroups branch 2 times, most recently from 3675d42 to d985f57 Compare March 14, 2024 06:29
@luolanzone luolanzone mentioned this pull request Mar 14, 2024
Merged
tnqn
tnqn previously approved these changes Mar 14, 2024
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented Mar 14, 2024

There is a conflict

@luolanzone
Remove deprecated v1alpha2 ClusterGroups
646d556 
Signed-off-by: Lan Luo <luola@vmware.com>
@luolanzone luolanzone force-pushed the remove-v1alpha2-clustergroups branch from d985f57 to 646d556 Compare March 15, 2024 02:35
@luolanzone
Copy link
Contributor Author

Conflicts resolved.

tnqn
tnqn approved these changes Mar 15, 2024
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas
Copy link
Contributor

/test-all

@antoninbas antoninbas merged commit bc54253 into antrea-io:main Mar 15, 2024
51 of 55 checks passed
@antoninbas antoninbas added this to the Antrea v2.0 release milestone Mar 15, 2024
@antoninbas antoninbas added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API. action/release-note Indicates a PR that should be included in release notes. labels Mar 15, 2024
antoninbas pushed a commit to antoninbas/antrea that referenced this pull request Mar 20, 2024
@luolanzone @antoninbas
Remove deprecated v1alpha2 ClusterGroup API (antrea-io#6049)
c9e722d 
Signed-off-by: Lan Luo <luola@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoninbas antoninbas antoninbas left review comments

@tnqn tnqn tnqn approved these changes

@GraysonWu GraysonWu Awaiting requested review from GraysonWu

@Dyanngg Dyanngg Awaiting requested review from Dyanngg

Assignees
No one assigned
Labels
action/release-note Indicates a PR that should be included in release notes. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API.
Projects
None yet
Milestone
Antrea v2.0 release
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@luolanzone @tnqn @antoninbas @Dyanngg