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

Spidercoordinator: It able to get CIDR from kubeadm-config #3062

Merged

Conversation

cyclinder
Copy link
Collaborator

If the kube-controller-manager Pod is running as systemd process rather than Pod, In this case, We can't get the CIDR from the KCM Pod. We can get the CIDR from the kubeadm-config configMap.

Thanks for contributing!

What type of PR is this?

  • release/bug

What this PR does / why we need it:

Kube-controller-pod is running as systemd process rather than a pod, We can't get the cluster CIDR in this case.

  1. We get the clusterCIDR from kubeadm-config ConfigMap first.
  2. If kubeadm-config ConfigMap does exist in cluster, we can get it from kube-controller-manager.

Which issue(s) this PR fixes:

Fixes #3054

Special notes for your reviewer:

Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e247e6d) 81.05% compared to head (712f10c) 81.10%.
Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3062      +/-   ##
==========================================
+ Coverage   81.05%   81.10%   +0.05%     
==========================================
  Files          49       49              
  Lines        5351     5351              
==========================================
+ Hits         4337     4340       +3     
+ Misses        856      854       -2     
+ Partials      158      157       -1     
Flag Coverage Δ
unittests 81.10% <ø> (+0.05%) ⬆️

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

see 1 file with indirect coverage changes

@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 4 times, most recently from 87b44fa to 5533503 Compare January 5, 2024 08:01
@weizhoublue
Copy link
Collaborator

there are so many cases already and it is import . Is it possible to design relevant E2E cases to long term test ?

and it could finish #2905 as well recently in another PR

@cyclinder
Copy link
Collaborator Author

#2927 is some E2E cases for spidercoordiantor, we can also add a case for these changes: delete the kubeadm-config configMap and see if works, but we can't delete kube-controller-manager pod, it's so dangerous. It can be finished in #2905.

@weizhoublue
Copy link
Collaborator

#2927 is some E2E cases for spidercoordiantor, we can also add a case for these changes: delete the kubeadm-config configMap and see if works, but we can't delete kube-controller-manager pod, it's so dangerous. It can be finished in #2905.

do you already have two cases to get CIDR from configmap and pod yaml separately ?

@cyclinder
Copy link
Collaborator Author

we can also add a case for these changes: delete the kubeadm-config configMap and see if works, but we can't delete kube-controller-manager pod, it's so dangerous. It can be finished in #2905.

Not yet. It can be finished in this PR Or #2905

@weizhoublue
Copy link
Collaborator

I suggest to finish the E2E together for verification

@weizhoublue
Copy link
Collaborator

I suggest to finish the E2E together for verification

is there any update

@cyclinder
Copy link
Collaborator Author

is there any update

Still working on this

@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch from 5533503 to a4b5bfd Compare January 17, 2024 10:32
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 8 times, most recently from 54a2fcf to a462243 Compare January 18, 2024 03:27
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch from a462243 to df9ce4a Compare January 18, 2024 05:52
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 3 times, most recently from cfb3856 to ef2f046 Compare January 18, 2024 07:28
ty-dc
ty-dc previously approved these changes Jan 18, 2024
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 5 times, most recently from e907c33 to 83be433 Compare January 22, 2024 13:15
If the kube-controller-manager Pod is running as systemd precess rather than Pod, In this case, We can't get the CIDR from the KCM Pod. We can get the CIDR from the kubeadm-config configMap.

Signed-off-by: cyclinder <[email protected]>
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch from 83be433 to 712f10c Compare January 22, 2024 14:31
@cyclinder
Copy link
Collaborator Author

/cc @weizhoublue

@ty-dc ty-dc merged commit 421bfb0 into spidernet-io:main Jan 24, 2024
43 checks passed
@ty-dc ty-dc mentioned this pull request Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spiderpool does not support binary installation of k8s?
3 participants