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

Unify IPPool subnet and ExternalIPPool definitions #6036

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

mengdie-song
Copy link
Contributor

@mengdie-song mengdie-song commented Feb 29, 2024

Unify IPPool subnet and ExternalIPPool definitions

  1. Make IPPoolSpec of IPPool use the same way to define IP ranges
    and subnet as ExternalIPPool. With this change, one IPPool can
    have multiple IP Ranges, but can only have one SubnetInfo.

  2. Deprecated IPVersion field in IPPool definition.

  3. Change IPPool VLAN from uint16 to int32 to be consisitent
    with ExternalIPPool definition.

  4. Promote IPPool CRD to v1beta1

  5. Add conversion webhook to IPPool for compatibility.

Fixes:#5961

Signed-off-by: Mengdie Song songm@vmware.com

@mengdie-song mengdie-song force-pushed the unify-ippool-def branch 3 times, most recently from aeac2de to 5f7342e Compare March 1, 2024 09:25
pkg/apis/crd/v1beta1/types.go Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
pkg/apis/crd/v1beta1/types.go Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
pkg/apis/crd/v1beta1/types.go Show resolved Hide resolved
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.

Please check and fix unit test failures.

build/charts/antrea/crds/ippool.yaml Show resolved Hide resolved
pkg/agent/ipassigner/ip_assigner_linux.go Outdated Show resolved Hide resolved
pkg/apis/crd/v1beta1/types.go Outdated Show resolved Hide resolved
pkg/apis/crd/v1beta1/types.go Show resolved Hide resolved
@XinShuYang XinShuYang force-pushed the unify-ippool-def branch 2 times, most recently from 060ebf8 to e63b677 Compare March 20, 2024 10:06
@luolanzone
Copy link
Contributor

@mengdie-song please resolve the code conflicts.

@XinShuYang XinShuYang force-pushed the unify-ippool-def branch 2 times, most recently from b23e128 to c2393dd Compare March 21, 2024 06:30
@mengdie-song mengdie-song force-pushed the unify-ippool-def branch 3 times, most recently from 3d5ee0a to 0661bf0 Compare March 22, 2024 08:09
.github/workflows/kind.yml Outdated Show resolved Hide resolved
pkg/agent/cniserver/ipam/ipam_service.go Outdated Show resolved Hide resolved
pkg/agent/interfacestore/types.go Outdated Show resolved Hide resolved
pkg/controller/ipam/convert.go Outdated Show resolved Hide resolved
pkg/controller/ipam/convert.go Outdated Show resolved Hide resolved
test/e2e/antreaipam_test.go Outdated Show resolved Hide resolved
.github/workflows/kind.yml Outdated Show resolved Hide resolved
pkg/agent/cniserver/ipam/antrea_ipam_controller.go Outdated Show resolved Hide resolved
pkg/controller/ipam/convert.go Outdated Show resolved Hide resolved
pkg/util/ip/ip.go Outdated Show resolved Hide resolved
@XinShuYang XinShuYang force-pushed the unify-ippool-def branch 2 times, most recently from 26925bf to a380cca Compare April 11, 2024 09:58
@luolanzone luolanzone added this to the Antrea v2.0 release milestone Apr 16, 2024
@mengdie-song mengdie-song force-pushed the unify-ippool-def branch 2 times, most recently from 18213f2 to 95cd0aa Compare April 18, 2024 07:51
@mengdie-song
Copy link
Contributor Author

/test-flexible-ipam-e2e

docs/antrea-ipam.md Show resolved Hide resolved
build/charts/antrea/crds/ippool.yaml Show resolved Hide resolved
pkg/controller/ipam/convert.go Show resolved Hide resolved
docs/antrea-ipam.md Outdated Show resolved Hide resolved
luolanzone
luolanzone previously approved these changes Apr 19, 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, @antoninbas can you take a look again? Thanks.

build/charts/antrea/crds/ippool.yaml Show resolved Hide resolved
docs/antrea-ipam.md Outdated Show resolved Hide resolved
pkg/agent/cniserver/ipam/antrea_ipam_controller.go Outdated Show resolved Hide resolved
pkg/apis/crd/v1beta1/types.go Outdated Show resolved Hide resolved
pkg/apis/crd/v1beta1/types.go Outdated Show resolved Hide resolved
pkg/apiserver/apiserver.go Outdated Show resolved Hide resolved
pkg/controller/ipam/convert.go Show resolved Hide resolved
pkg/controller/ipam/convert_test.go Outdated Show resolved Hide resolved
pkg/controller/ipam/convert_test.go Show resolved Hide resolved
pkg/ipam/poolallocator/allocator.go Outdated Show resolved Hide resolved
1. Make IPPoolSpec of IPPool use the same way to define IP ranges
and subnet as ExternalIPPool.With this change, one IPPool can have
multiple IP Ranges, but it can only have one SubnetInfo, so this change
also adjusts the related tests, especially the ones for antrea ipam validation.

2. Deprecated IPVersion field in IPPool definition.

3. Change IPPool VLAN from uint16 to int32 to be consisitent
with ExternalIPPool definition.

4. Promote IPPool CRD to v1beta1

5. Add conversion webhook to IPPool for compatibility.

Fixes:antrea-io#5961

Signed-off-by: Mengdie Song <songm@vmware.com>
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 antoninbas added the action/release-note Indicates a PR that should be included in release notes. label Apr 24, 2024
@antoninbas
Copy link
Contributor

/test-all

@antoninbas
Copy link
Contributor

/test-conformance

@antoninbas antoninbas merged commit 9a9db6f into antrea-io:main Apr 25, 2024
53 of 57 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.

6 participants