Prepare for Antrea IPAM StatefulSet support #3141

annakhm · 2021-12-15T01:19:16Z

Support continuous IP range allocation in ip allocator
Support pe-allocating continuous range of IPs from an IP
Pool for StatefulSet. This functionality will be used by
StatefulSets that do not have a dedicated IP Pool annotated.

Signed-off-by: Anna Khmelnitsky akhmelnitsky@vmware.com

codecov-commenter · 2021-12-15T01:27:07Z

Codecov Report

Merging #3141 (0884d2c) into main (2ee6ad1) will decrease coverage by 12.68%.
The diff coverage is 73.83%.

@@             Coverage Diff             @@
##             main    #3141       +/-   ##
===========================================
- Coverage   60.59%   47.90%   -12.69%     
===========================================
  Files         297      491      +194     
  Lines       25434    43453    +18019     
===========================================
+ Hits        15411    20818     +5407     
- Misses       8345    20476    +12131     
- Partials     1678     2159      +481

Flag	Coverage Δ
integration-tests	`29.53% <ø> (?)`
kind-e2e-tests	`46.66% <0.00%> (-0.32%)`	⬇️
unit-tests	`40.59% <73.83%> (+0.24%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/util/k8s/name.go	`6.25% <0.00%> (-2.09%)`	⬇️
pkg/ipam/poolallocator/allocator.go	`50.00% <58.49%> (+1.18%)`	⬆️
pkg/controller/ipam/antrea_ipam_controller.go	`80.28% <80.28%> (ø)`
pkg/ipam/ipallocator/allocator.go	`86.50% <87.80%> (+2.29%)`	⬆️
pkg/agent/cniserver/ipam/antrea_ipam_controller.go	`79.76% <100.00%> (ø)`
...ver/registry/controlplane/nodestatssummary/rest.go	`50.00% <0.00%> (-50.00%)`	⬇️
pkg/agent/metrics/prometheus.go	`24.32% <0.00%> (-5.41%)`	⬇️
pkg/controller/traceflow/controller.go	`74.07% <0.00%> (-2.12%)`	⬇️
pkg/monitor/controller.go	`27.61% <0.00%> (-1.50%)`	⬇️
... and 204 more

annakhm · 2021-12-15T02:01:56Z

/test-all

pkg/ipam/poolallocator/allocator.go

annakhm · 2021-12-17T05:17:48Z

I will be adding workers to the controller, since the delete operation is heavy.

pkg/controller/ipam/antrea_ipam_controller.go

annakhm · 2021-12-22T02:05:52Z

/test-all

gran-vmv · 2021-12-23T03:13:46Z

@annakhm Could you integrate commit from #3046 into this PR? Otherwise we'll get many conflicts when merge.

annakhm · 2021-12-24T01:31:27Z

@annakhm Could you integrate commit from #3046 into this PR? Otherwise we'll get many conflicts when merge.

Done

gran-vmv · 2021-12-24T04:02:46Z

e2e failed. Permission issue. Please add rbac into this PR.

I1224 02:38:41.556860       1 antrea_ipam_controller.go:164] "Processing delete notification" StatefulSet="sts-test-8xye4uy9"
I1224 02:38:41.556926       1 antrea_ipam_controller.go:171] "Clearing reserved IP addresses" StatefulSet="sts-test-8xye4uy9" IP Pools=[test-ippool-ipv4-0]
W1224 02:38:41.558171       1 allocator.go:515] IP Pool test-ippool-ipv4-0 update failed: ippools.crd.antrea.io "test-ippool-ipv4-0" is forbidden: User "system:serviceaccount:kube-system:antrea-controller" cannot update resource "ippools/status" in API group "crd.antrea.io" at the cluster scope
E1224 02:38:41.558210       1 allocator.go:524] Failed to release IP addresses for Stateful Set:antrea-ipam-test/sts-test-8xye4uy9 from pool test-ippool-ipv4-0: ippools.crd.antrea.io "test-ippool-ipv4-0" is forbidden: User "system:serviceaccount:kube-system:antrea-controller" cannot update resource "ippools/status" in API group "crd.antrea.io" at the cluster scope
E1224 02:38:41.558662       1 antrea_ipam_controller.go:186] Failed to clean IP allocations for StatefulSet sts-test-8xye4uy9 in Pool test-ippool-ipv4-0
E1224 02:38:41.558699       1 antrea_ipam_controller.go:218] Failed to clean IP Pool for StatefulSet 229ef5fc-9e9e-40b0-83ce-dd463964f4ed: ippools.crd.antrea.io "test-ippool-ipv4-0" is forbidden: User "system:serviceaccount:kube-system:antrea-controller" cannot update resource "ippools/status" in API group "crd.antrea.io" at the cluster scope

gran-vmv · 2022-01-06T01:28:56Z

Hi Anna,
Could you fix the permission issue and rebase to latest main branch?

annakhm · 2022-01-06T20:32:01Z

/test-all

annakhm · 2022-01-06T22:31:45Z

/test-all

annakhm · 2022-01-06T23:21:14Z

Fixed permissions and rebased

vicky-liu · 2022-01-07T00:25:52Z

cc @tnqn @jianjuns for more review.

pkg/controller/ipam/antrea_ipam_controller.go

pkg/controller/ipam/antrea_ipam_controller_test.go

tnqn · 2022-01-07T13:12:46Z

pkg/ipam/poolallocator/allocator.go

+// This functionality is useful when StatefulSet does not have a dedicated IP Pool assigned.
+// It returns error if such range is not available. In this case IPs for the StatefulSet will
+// be allocated on the fly, and there is no guarantee for continuous IPs.
+func (a *IPPoolAllocator) AllocateStatefulSet(namespace, name string, size int) error {


Where is it called? If it's just a preparation for another feature, could you move it and all the callees it introduces to the PR that implements the feature? Otherwise it's hard to say whether the methods can really fit that feature without any caller and may introduce unnecessary changes.
This PR could focus on StatefulSet Pod IP cleanup which is scoped in 1.5?

This is for future feature, and not used in code as of today, but used in tests. I'd prefer to include this in the PR for future use rather than copying this functionality in testing.

gran-vmv · 2022-01-10T01:34:57Z

Hi Anna,
We should check the PodTemplate's annotation on StatefulSet, because the Pods will be deleted before StatefulSet deleted.
Could you also check Quan's comment above? He provided a better method to solve this.

Below case is using a dedicated IPPool for StatefulSet and the name is test-ippool-ipv4-1, but we found this IPPool is not cleaned up and the controller log said no IP allocations found in test-ippool-ipv4-0 which is annotated to Namespace.

    antreaipam_test.go:386: IPPool status isn't correct [{"ipAddress":"192.168.240.130","phase":"Reserved","owner":{"statefulSet":{"name":"sts-test-ecxchkuw","namespace":"antrea-ipam-test","index":0}}},{"ipAddress":"192.168.240.131","phase":"Reserved","owner":{"statefulSet":{"name":"sts-test-ecxchkuw","namespace":"antrea-ipam-test","index":1}}},{"ipAddress":"192.168.240.132","phase":"Allocated","owner":{"pod":{"name":"test-standalone-pod-sfdsvncz","namespace":"antrea-ipam-test","containerID":"c61f4b067978fbcc5f2197e328f8ed8319d8a4f5d19a16f4e8227c56f8b57373"}}},{"ipAddress":"192.168.240.133","phase":"Reserved","owner":{"statefulSet":{"name":"sts-test-ecxchkuw","namespace":"antrea-ipam-test","index":2}}}]
    antreaipam_test.go:390: 
        	Error Trace:	antreaipam_test.go:390
        	            				antreaipam_test.go:307
        	            				antreaipam_test.go:155
        	Error:      	Expected nil, but got: &errors.errorString{s:"timed out waiting for the condition"}
        	Test:       	TestAntreaIPAM/testAntreaIPAMStatefulSetDedicated

I0107 12:58:52.514229       1 antrea_ipam_controller.go:164] "Processing delete notification" StatefulSet="sts-test-ecxchkuw"
I0107 12:58:52.514286       1 antrea_ipam_controller.go:171] "Clearing reserved IP addresses" StatefulSet="sts-test-ecxchkuw" IP Pools=[test-ippool-ipv4-0]
I0107 12:58:52.514305       1 allocator.go:506] "No reserved IPs found" pool="test-ippool-ipv4-0" Namespace="antrea-ipam-test" StatefulSet="sts-test-ecxchkuw"

annakhm · 2022-01-11T02:21:56Z

/test-all

gran-vmv

flexible-ipam-e2e passed. Thanks.

pkg/controller/ipam/antrea_ipam_controller.go

pkg/controller/ipam/antrea_ipam_controller_test.go

pkg/ipam/poolallocator/allocator.go

annakhm · 2022-01-11T23:11:25Z

/test-all

gran-vmv

flexible-ipam-e2e passed

* Support continuous IP range allocation in IP allocator * Support pe-allocating continuous range of IPs from an IP Pool for StatefulSet. This functionality will be used by StatefulSets that do not have a dedicated IP Pool annotated. * Support releasing all IP address entries associated with a StatefulSet in IP Pool * Implement Antrea Controller responsible for clearing IP Pool allocations when StatefulSet is deleted Signed-off-by: Anna Khmelnitsky <akhmelnitsky@vmware.com>

tnqn

LGTM overall

tnqn · 2022-01-12T06:46:26Z

/test-flexible-ipam-e2e

gran-vmv

flexible-ipam-e2e passed with #3146 , which means this PR should also pass this pipeline.

tnqn · 2022-01-12T10:15:27Z

/skip-e2e
/skip-integration
/skip-networkpolicy
/skip-conformance
I have run the above tests manually.

* Support continuous IP range allocation in IP allocator * Support pre-allocating continuous range of IPs from an IP Pool for StatefulSet. This functionality will be used by StatefulSets that do not have a dedicated IP Pool annotated. * Support releasing all IP address entries associated with a StatefulSet in IP Pool * Implement Antrea Controller responsible for clearing IP Pool allocations when StatefulSet is deleted Signed-off-by: Anna Khmelnitsky <akhmelnitsky@vmware.com>

annakhm mentioned this pull request Dec 15, 2021

[flexible-ipam] Prepare for Antrea IPAM StatefulSet support #3065

Closed

annakhm requested review from tnqn and gran-vmv December 15, 2021 04:15

gran-vmv reviewed Dec 15, 2021

View reviewed changes

pkg/ipam/poolallocator/allocator.go Show resolved Hide resolved

annakhm requested a review from salv-orlando December 16, 2021 17:47

annakhm force-pushed the allocator-range branch from d190235 to e4b500c Compare December 17, 2021 04:55

annakhm marked this pull request as draft December 17, 2021 05:17

annakhm marked this pull request as ready for review December 18, 2021 01:24

gran-vmv reviewed Dec 20, 2021

View reviewed changes

pkg/controller/ipam/antrea_ipam_controller.go Show resolved Hide resolved

gran-vmv self-requested a review December 20, 2021 10:04

annakhm force-pushed the allocator-range branch from 4b0c436 to 2b45d2b Compare December 24, 2021 01:29

gran-vmv added this to the Antrea v1.5 release milestone Jan 4, 2022

annakhm force-pushed the allocator-range branch from 2b45d2b to 8d57597 Compare January 6, 2022 18:14

annakhm force-pushed the allocator-range branch from 8d57597 to 901c1d7 Compare January 6, 2022 20:59

jianjuns reviewed Jan 7, 2022

View reviewed changes

pkg/controller/ipam/antrea_ipam_controller.go Outdated Show resolved Hide resolved

pkg/controller/ipam/antrea_ipam_controller.go Outdated Show resolved Hide resolved

pkg/controller/ipam/antrea_ipam_controller.go Outdated Show resolved Hide resolved

tnqn reviewed Jan 7, 2022

View reviewed changes

gran-vmv mentioned this pull request Jan 10, 2022

[flexible-ipam] Add e2e for StatefulSet #3146

Merged

annakhm force-pushed the allocator-range branch 3 times, most recently from 4b44f60 to a575be5 Compare January 11, 2022 01:38

gran-vmv previously approved these changes Jan 11, 2022

View reviewed changes

tnqn reviewed Jan 11, 2022

View reviewed changes

annakhm dismissed gran-vmv’s stale review via 50565ba January 11, 2022 22:22

annakhm force-pushed the allocator-range branch from a575be5 to 50565ba Compare January 11, 2022 22:22

gran-vmv self-requested a review January 12, 2022 03:33

gran-vmv previously approved these changes Jan 12, 2022

View reviewed changes

annakhm dismissed gran-vmv’s stale review via 8093a95 January 12, 2022 04:29

annakhm force-pushed the allocator-range branch from 50565ba to 8093a95 Compare January 12, 2022 04:29

tnqn previously approved these changes Jan 12, 2022

View reviewed changes

annakhm dismissed tnqn’s stale review via 0884d2c January 12, 2022 04:44

annakhm force-pushed the allocator-range branch from 8093a95 to 0884d2c Compare January 12, 2022 04:44

tnqn approved these changes Jan 12, 2022

View reviewed changes

gran-vmv self-requested a review January 12, 2022 09:20

gran-vmv approved these changes Jan 12, 2022

View reviewed changes

tnqn merged commit d691c62 into antrea-io:main Jan 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare for Antrea IPAM StatefulSet support #3141

Prepare for Antrea IPAM StatefulSet support #3141

annakhm commented Dec 15, 2021

codecov-commenter commented Dec 15, 2021 •

edited

Loading

annakhm commented Dec 15, 2021

annakhm commented Dec 17, 2021

annakhm commented Dec 22, 2021

gran-vmv commented Dec 23, 2021

annakhm commented Dec 24, 2021

gran-vmv commented Dec 24, 2021 •

edited

Loading

gran-vmv commented Jan 6, 2022

annakhm commented Jan 6, 2022

annakhm commented Jan 6, 2022

annakhm commented Jan 6, 2022

vicky-liu commented Jan 7, 2022

tnqn Jan 7, 2022

annakhm Jan 11, 2022

gran-vmv commented Jan 10, 2022 •

edited

Loading

annakhm commented Jan 11, 2022

gran-vmv left a comment

annakhm commented Jan 11, 2022

gran-vmv left a comment

tnqn left a comment

tnqn commented Jan 12, 2022

gran-vmv left a comment

tnqn commented Jan 12, 2022

Prepare for Antrea IPAM StatefulSet support #3141

Prepare for Antrea IPAM StatefulSet support #3141

Conversation

annakhm commented Dec 15, 2021

codecov-commenter commented Dec 15, 2021 • edited Loading

Codecov Report

annakhm commented Dec 15, 2021

annakhm commented Dec 17, 2021

annakhm commented Dec 22, 2021

gran-vmv commented Dec 23, 2021

annakhm commented Dec 24, 2021

gran-vmv commented Dec 24, 2021 • edited Loading

gran-vmv commented Jan 6, 2022

annakhm commented Jan 6, 2022

annakhm commented Jan 6, 2022

annakhm commented Jan 6, 2022

vicky-liu commented Jan 7, 2022

tnqn Jan 7, 2022

Choose a reason for hiding this comment

annakhm Jan 11, 2022

Choose a reason for hiding this comment

gran-vmv commented Jan 10, 2022 • edited Loading

annakhm commented Jan 11, 2022

gran-vmv left a comment

Choose a reason for hiding this comment

annakhm commented Jan 11, 2022

gran-vmv left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Jan 12, 2022

gran-vmv left a comment

Choose a reason for hiding this comment

tnqn commented Jan 12, 2022

codecov-commenter commented Dec 15, 2021 •

edited

Loading

gran-vmv commented Dec 24, 2021 •

edited

Loading

gran-vmv commented Jan 10, 2022 •

edited

Loading