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.

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

Increase the minimum supported Kubernetes version to v1.19 #6089

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

hjiajing
Copy link
Contributor

@hjiajing hjiajing commented Mar 8, 2024

Since Kubernetes v1.19, some APIs are deprecated. Add deprecated comment to those APIs and update documents.

Reference: https://kubernetes.io/docs/reference/using-api/deprecation-guide/

README.md Show resolved Hide resolved
@@ -44,6 +44,9 @@ spec:
externalNode:
type: string
- name: v1alpha1
deprecated: true
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a few more CRDs with two or more served versions, I think we need to add this field for all old versions. @antoninbas what's your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we have more deprecated CRD versions according to https://github.com/antrea-io/antrea/blob/main/docs/api.md#crds-in-crdantreaio

Looking at this document, I have 2 questions though:

  • why is v1alpha1/ExternalEntity not listed in the doc?
  • the doc mentions that v1alpha1/Tier is supposed to be removed in Antrea v2.0. Are we still planning to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a few more CRDs with two or more served versions, I think we need to add this field for all old versions. @antoninbas what's your thoughts?

Yes, that is what I want to ask. So, if one CRD has more than one served version, we just keep the latest one and add deprecated: true and deprecation warning to other versions?.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. https://github.com/antrea-io/antrea/blob/main/docs/api.md#crds-in-crdantreaio should also be up-to-date in that regard (see Deprecated in column).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

The PR is submitted to remove the v1alpha1/Tier #6162
@hjiajing you need to rebase after the merge. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the reminder, I will rebase after that PR is merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, looks like we can simply remove v1alpha1/ExternalEntity. It has not been served for 2+ years. But let's do it in a separate 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.

Hi @antoninbas , I created a PR to remove this CRD #6177

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. I assume this PR will need to be rebased after we merge #6177

@hjiajing hjiajing force-pushed the increase-min-k8s-support branch 3 times, most recently from a8c0dda to ff490d0 Compare March 28, 2024 02:06
@luolanzone luolanzone added the action/release-note Indicates a PR that should be included in release notes. label Mar 28, 2024
Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

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

LGTM overall, @hjiajing could you check the comment of PR6177, so we can merge it and move forward for this PR? Thanks.

@hjiajing
Copy link
Contributor Author

hjiajing commented Apr 9, 2024

LGTM overall, @hjiajing could you check the comment of PR6177, so we can merge it and move forward for this PR? Thanks.

Sure, I have addressed the comment of #6177 , I will move forward after that PR is merged.

@hjiajing
Copy link
Contributor Author

Hi @luolanzone @antoninbas , I have rebased to the latest main branch, could you please help to check this PR, thanks.

Comment on lines 13 to 14
deprecated: true
deprecationWarning: "crd.antrea.io/v1alpha1 ClusterNetworkPolicy is deprecated; use crd.antrea.io/v1beta1 ClusterNetworkPolicy"
Copy link
Contributor

Choose a reason for hiding this comment

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

this API (and some other ones) were supposed to be removed in #6162
I think @luolanzone may have missed some files when updating her PR
I will open a PR to address this

Copy link
Contributor

Choose a reason for hiding this comment

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

If #6238 and #6239 are merged, this is likely to be just a documentation PR, as all deprecated APIs will have been removed.
However, we will be able to use these attributes (deprecated / deprecationWarning) in the future, which is a good thing.

@luolanzone
Copy link
Contributor

@hjiajing please update the PR message to resolve issue #5879

Since Kubernetes v1.19, some APIs are deprecated. Add deprecated comment
to those APIs and update documents.

Reference: https://kubernetes.io/docs/reference/using-api/deprecation-guide/

Signed-off-by: hjiajing <hjiajing@vmware.com>
@antoninbas
Copy link
Contributor

/skip-all

@antoninbas antoninbas merged commit 42e7cfa into antrea-io:main Apr 24, 2024
49 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants