feat: Move creation of 'ext' HNS network into cse for Windows #2450
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2450 +/- ##
==========================================
+ Coverage 72.3% 72.57% +0.27%
==========================================
Files 130 130
Lines 23648 23916 +268
==========================================
+ Hits 17098 17358 +260
- Misses 5524 5530 +6
- Partials 1026 1028 +2 |
|
/cc @pradipd |
parts/k8s/kuberneteswindowssetup.ps1
Outdated
| @@ -275,12 +279,14 @@ try | |||
| -IdentitySystem "{{ GetIdentitySystem }}" | |||
| } | |||
|
|
|||
| } elseif ($global:NetworkPlugin -eq "kubenet") { | |||
| New-ExternalHnsNetwork | |||
There was a problem hiding this comment.
just to clarify - is New-ExternalHnsNetwork not required for kubenet?
There was a problem hiding this comment.
It isn't being created for kubenet today
There was a problem hiding this comment.
So i assume it isn't
There was a problem hiding this comment.
The removal is in the code block for if ($NetworkPlugin -eq "azure") { }
There was a problem hiding this comment.
Maybe @pradipd or @dineshgovindasamy can comment on what scenarios we should create the ext HNS network?
There was a problem hiding this comment.
It should be created if you want to prevent that brief network disconnect on reboot
There was a problem hiding this comment.
I updated to create the network for all network plugings.
| } | ||
|
|
||
| # If there is more than one adapter, use the first adapter. | ||
| $managementIP = (Get-NetIPAddress -ifIndex $na[0].ifIndex -AddressFamily IPv4).IPAddress |
There was a problem hiding this comment.
@pradipd - should this be smarter? @dineshgovindasamy said there may be cases where a NIC doesn't have an IP and we should choose another?
There was a problem hiding this comment.
Yes. Actually, the (Get-NetAdapter -Physical) on L261 needs to be different for acceleratedNetworkingEnabledWindows.
I just haven't had a chance to figure out what it should be.
There was a problem hiding this comment.
Yes. Actually, the (Get-NetAdapter -Physical) on L261 needs to be different for acceleratedNetworkingEnabledWindows.
I just haven't had a chance to figure out what it should be.
There was a problem hiding this comment.
@pradipd ok - we'll consider that for a separate PR
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marosset, PatrickLang 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 |
Reason for Change:
Issue Fixed:
Fixes #2432
Requirements:
Notes: