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

synchronize clusterIP CIDR from serviceCIDR to support k8s 1.29 #3132

Merged

Conversation

cyclinder
Copy link
Collaborator

@cyclinder cyclinder commented Jan 25, 2024

it could be sync service cidr to spidercoordinator from serviceCIDR in k8s 1.29

Thanks for contributing!

What type of PR is this?

  • release/feature

What this PR does / why we need it:

Refer to https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/1880-multiple-service-cidrs/README.md, kubernetes can allocate service ips from serviceCIDR resources in 1.29.


➜  ~ kubectl get servicecidr kubernetes -o yaml
apiVersion: networking.k8s.io/v1alpha1
kind: ServiceCIDR
metadata:
  creationTimestamp: "2024-01-25T08:36:00Z"
  finalizers:
  - networking.k8s.io/service-cidr-finalizer
  name: kubernetes
  resourceVersion: "504422"
  uid: 72461b7d-fddd-409d-bdf2-83d1a2c067ca
spec:
  cidrs:
  - 10.233.0.0/18
  - fd00:10:233::/116
status:
  conditions:
  - lastTransitionTime: "2024-01-28T06:38:55Z"
    message: Kubernetes Service CIDR is ready
    reason: ""
    status: "True"
    type: Ready

➜  ~ kubectl get servicecidr kubernetes -o yaml
apiVersion: networking.k8s.io/v1alpha1
kind: ServiceCIDR
metadata:
  creationTimestamp: "2024-01-25T08:36:00Z"
  finalizers:
  - networking.k8s.io/service-cidr-finalizer
  name: kubernetes
  resourceVersion: "504422"
  uid: 72461b7d-fddd-409d-bdf2-83d1a2c067ca
spec:
  cidrs:
  - 10.233.0.0/18
  - fd00:10:233::/116
status:
  conditions:
  - lastTransitionTime: "2024-01-28T06:38:55Z"
    message: Kubernetes Service CIDR is ready
    reason: ""
    status: "True"
    type: Ready

This PR synchronizes the service cidr from the serviceCIDR resource to the spidercoordinator. If the resource cannot be found or there are any errors, then we will fall back to the default behavior (from kubeadm-config and kube-controller-manager).

➜  test git:(3e40ed05) ✗ kubectl get servicecidr
NAME         CIDRS                             AGE
kubernetes   10.233.0.0/18,fd00:10:233::/116   5h41m
test1        10.234.0.0/18,fd00:10:234::/116   53m
➜  test git:(3e40ed05) ✗ kubectl get spidercoordinators.spiderpool.spidernet.io default -o yaml
apiVersion: spiderpool.spidernet.io/v2beta1
kind: SpiderCoordinator
metadata:
  creationTimestamp: "2024-01-25T08:41:50Z"
  finalizers:
  - spiderpool.spidernet.io
  generation: 1
  name: default
  resourceVersion: "41645"
  uid: d1e095db-d6e8-4413-b60e-fcf31ad2bf5e
spec:
  detectGateway: false
  detectIPConflict: false
  hijackCIDR:
  - 10.244.64.0/18
  - fd00:10:244::/112
  hostRPFilter: 0
  hostRuleTable: 500
  mode: auto
  podCIDRType: auto
  podDefaultRouteNIC: ""
  podMACPrefix: ""
  tunePodRoutes: true
  txQueueLen: 0
status:
  phase: Synced
  serviceCIDR:
  - 10.234.0.0/18
  - fd00:10:234::/116
  - 10.233.0.0/18
  - fd00:10:233::/116
  
➜  test kubectl delete servicecidr test1
servicecidr.networking.k8s.io "test1" deleted

➜  test git:(3e40ed05) ✗
➜  test git:(3e40ed05) ✗ kubectl get spidercoordinators.spiderpool.spidernet.io default -o yaml
apiVersion: spiderpool.spidernet.io/v2beta1
kind: SpiderCoordinator
metadata:
  creationTimestamp: "2024-01-25T08:41:50Z"
  finalizers:
  - spiderpool.spidernet.io
  generation: 1
  name: default
  resourceVersion: "46709"
  uid: d1e095db-d6e8-4413-b60e-fcf31ad2bf5e
spec:
  detectGateway: false
  detectIPConflict: false
  hijackCIDR:
  - 10.244.64.0/18
  - fd00:10:244::/112
  hostRPFilter: 0
  hostRuleTable: 500
  mode: auto
  podCIDRType: auto
  podDefaultRouteNIC: ""
  podMACPrefix: ""
  tunePodRoutes: true
  txQueueLen: 0
status:
  phase: Synced
  serviceCIDR:
  - 10.233.0.0/18
  - fd00:10:233::/116

Which issue(s) this PR fixes:

Fixes #2905

Special notes for your reviewer:

Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (077e85d) 81.07% compared to head (0628e63) 81.13%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3132      +/-   ##
==========================================
+ Coverage   81.07%   81.13%   +0.05%     
==========================================
  Files          50       50              
  Lines        5358     5358              
==========================================
+ Hits         4344     4347       +3     
+ Misses        856      854       -2     
+ Partials      158      157       -1     
Flag Coverage Δ
unittests 81.13% <87.50%> (+0.05%) ⬆️

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

Files Coverage Δ
...pplicationcontroller/applicationinformers/utils.go 85.75% <100.00%> (ø)
pkg/ippoolmanager/ippool_informer.go 55.81% <100.00%> (ø)
pkg/ippoolmanager/ippool_mutate.go 97.05% <100.00%> (ø)
pkg/subnetmanager/subnet_manager.go 41.48% <66.66%> (ø)

... and 1 file with indirect coverage changes

@cyclinder cyclinder force-pushed the spidercoorinator/k8s_1.29_servicecidr branch from 820f5f4 to 44c200b Compare January 25, 2024 15:12
@weizhoublue
Copy link
Collaborator

in the PR description, could you attach a yaml example of servicecidr to show what it looks like for reference

@weizhoublue weizhoublue changed the title it could be sync service cidr from serviceCIDR in k8s 1.29 synchronize clusterIP CIDR from serviceCIDR to support k8s 1.29 Jan 26, 2024
@weizhoublue
Copy link
Collaborator

for a new feature, it should add some explanation.
(1) doc/develop/roadmap
(2) doc/usage/coordinator.md

@cyclinder cyclinder force-pushed the spidercoorinator/k8s_1.29_servicecidr branch 2 times, most recently from 0bfa8e5 to 2aed06a Compare January 26, 2024 04:53
@cyclinder cyclinder force-pushed the spidercoorinator/k8s_1.29_servicecidr branch 6 times, most recently from 5081b9e to d782a64 Compare January 28, 2024 09:18
@cyclinder cyclinder force-pushed the spidercoorinator/k8s_1.29_servicecidr branch 4 times, most recently from 5cec15f to e687fb8 Compare January 29, 2024 02:23
@cyclinder cyclinder force-pushed the spidercoorinator/k8s_1.29_servicecidr branch 2 times, most recently from c59c3eb to cc91f6a Compare January 29, 2024 02:56
it could be sync service cidr to spidercoordinator from serviceCIDR in k8s 1.29

Signed-off-by: cyclinder <qifeng.guo@daocloud.io>
@cyclinder cyclinder force-pushed the spidercoorinator/k8s_1.29_servicecidr branch 2 times, most recently from 92a703b to b5890bb Compare January 29, 2024 03:45
@ty-dc
Copy link
Collaborator

ty-dc commented Jan 29, 2024

该 PR 似乎会导致 k8s 矩阵(1.22、1.23、1.24等版本)失败 ?是否只能在 1.29 版本的 K8S 上运行?

@cyclinder
Copy link
Collaborator Author

该 PR 似乎会导致 k8s 矩阵(1.22、1.23、1.24等版本)失败 ?是否只能在 1.29 版本的 K8S 上运行?

是的,e2e 只能在 1.29 以上跑

Signed-off-by: cyclinder <qifeng.guo@daocloud.io>
@cyclinder cyclinder force-pushed the spidercoorinator/k8s_1.29_servicecidr branch from b5890bb to 0628e63 Compare January 29, 2024 06:32
@cyclinder cyclinder merged commit bc64d7d into spidernet-io:main Jan 29, 2024
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature release/feature-new release note for new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

k8s 1.29: clusterIP cidr
4 participants