feat: Dualstack support for Windows containers #3415
Conversation
tamilmani1989
changed the title
|
/assign @jackfrancis @mboersma |
|
/azp run pr-e2e |
1 similar comment
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@mboersma fixed nit issue. can you please start pipeline again? |
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov Report
@@ Coverage Diff @@
## master #3415 +/- ##
==========================================
- Coverage 73.17% 73.16% -0.01%
==========================================
Files 147 147
Lines 24923 25024 +101
==========================================
+ Hits 18238 18310 +72
- Misses 5559 5580 +21
- Partials 1126 1134 +8
Continue to review full report at Codecov.
|
parts/k8s/kubeproxystart.ps1
Outdated
| @@ -16,6 +17,12 @@ while (!$hnsNetwork) { | |||
| $hnsNetwork = Get-HnsNetwork | ? Name -EQ $KubeNetwork | |||
| } | |||
|
|
|||
| # add dualstack feature gate if dualstack enabled | |||
| $isDualStackEnabled = ("--feature-gates=IPv6DualStack=true" | ? { $Global:ClusterConfiguration.Kubernetes.Kubelet.ConfigArgs -match $_ }) -ne $null | |||
There was a problem hiding this comment.
Would this check fail if IPv6DualStack feature gate was not first in the list --feature-gates=CSIInlineVolume=true,IPv6DualStack=true?
There was a problem hiding this comment.
you're right, it will fail. let me change the match to "IPv6DualStack=true"
| $configJson.plugins[0].AdditionalArgs[1].Value.DestinationPrefix = $serviceCidr[0] | ||
| $valueObj = [PSCustomObject]@{ | ||
| Type = 'ROUTE' | ||
| DestinationPrefix = $serviceCidr[1] |
There was a problem hiding this comment.
What about the case where cluster is dual stack, but services are just single stack? The user could just have a single family CIDR for service.
There was a problem hiding this comment.
so if user didn't specify ipv6 service cidr, won't aks-e assign default service v6 cidr? May be im interpreting your ask wrongly
There was a problem hiding this comment.
aks-e default v6 CIDR only for ClusterCIDR. Having v4 and v6 CIDR for services isn't mandatory.
There was a problem hiding this comment.
i believe we should add default for v6 service cidr also here if user opted for dualstack.
aks-engine/pkg/api/defaults.go
Line 278 in 24e9029
There was a problem hiding this comment.
@tamilmani1989 Having dual stack CIDRs for services is not mandatory unlike ClusterCIDRs. The user can just define single stack v4 or v6 service CIDR in dual stack cluster. Thats the reason for not appending v6 service CIDR unless the user explicitly requests it.
There was a problem hiding this comment.
@aramase Yes you are right. Its not mandatory to create ipv6 svc but if user decided to have in future he has to recreate cluster. What's the user going to loose if we assign default svc cidr if its not explicitly specified?
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@aramase as per our discussion offline, addressed those changes |
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marosset, tamilmani1989 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 |
|
Thank you @tamilmani1989! |
|
yay! |
|
Thank you guys! |
Reason for Change:
This PR adds dual stack support for azure cni windows cluster as like linux cluster. V4 Ips will be in vnet and v6 IPs will be in non-vnet. Non lifting validation check for windows dualstack until windows team release OS support. This PR also fixes creating multiple v6 configs per nic as its not supported.
Issue Fixed:
Requirements:
Notes: