-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Calico: Add kube_service_addresses_ipv6 to serviceClusterIPs #7944
Calico: Add kube_service_addresses_ipv6 to serviceClusterIPs #7944
Conversation
…tes-sigs#7889) Add IPv6 Service Addresses to BGP advertisement when calico_advertise_cluster_ips is true.
Welcome @olemathias! |
Hi @olemathias. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -240,7 +240,8 @@ | |||
"logSeverityScreen": "Info", | |||
{% if not calico_no_global_as_num|default(false) %}"asNumber": {{ global_as_num }},{% endif %} | |||
"nodeToNodeMeshEnabled": {{ nodeToNodeMeshEnabled|default('true') }} , | |||
{% if calico_advertise_cluster_ips|default(false) %}"serviceClusterIPs": [{"cidr": "{{ kube_service_addresses }}" }],{% endif %} | |||
{% if calico_advertise_cluster_ips|default(false) %} | |||
"serviceClusterIPs": [{"cidr": "{{ kube_service_addresses }}" } {{ ',{"cidr":"' + kube_service_addresses_ipv6 + '"}' if enable_dual_stack_networks else '' }}],{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://docs.projectcalico.org/networking/advertise-service-ips#advertise-service-cluster-ip-addresses this option seems good.
In addition, this kube_service_addresses_ipv6 is available only if enable_dual_stack_networks is true on Kubespray side.
That also looks good for me because of the following description:
# Kubernetes internal network for IPv6 services, unused block of space.
# This is only used if enable_dual_stack_networks is set to true
# This provides 4096 IPv6 IPs
kube_service_addresses_ipv6: fd85:ee78:d8a6:8607::1000/116
/lgtm
forgot to put ok-to-test.. /ok-to-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@olemathias Thanks for the PR |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floryut, olemathias The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…tes-sigs#7889) (kubernetes-sigs#7944) Add IPv6 Service Addresses to BGP advertisement when calico_advertise_cluster_ips is true.
…tes-sigs#7889) (kubernetes-sigs#7944) Add IPv6 Service Addresses to BGP advertisement when calico_advertise_cluster_ips is true.
Add IPv6 Service Addresses to BGP advertisement when calico_advertise_cluster_ips is true.
What type of PR is this?
/kind feature
What this PR does / why we need it:
Currently when enabling
calico_advertise_cluster_ips
only IPv4kube_service_addresses
is added. This PR also adds IPv6kube_service_addresses_ipv6
whenenable_dual_stack_networks
is true.Which issue(s) this PR fixes:
Fixes #7889
Special notes for your reviewer:
Does this PR introduce a user-facing change?: