fix: enable Accelerated Networking settings and pick up the right adapter in windows azure function #4585

bingbing8 · 2021-07-16T07:34:00Z

fix: enable Accelerated Networking settings and pick up the right adapter in windows azure function

Reason for Change:
Enable Accelerated Networking settings and pick up the right adapter in windows azure function

Issue Fixed:

currently acceleratedNetworkingEnabledWindows only take false as value; remove the restriction to it is allowed to set to true
windowsazurefunction sometime chose Mellanox to retrieve ipaddress, but Mellanox sits beneath the virtual ethernet adapter that it’s associated with and has no ipaddress

Credit Where Due:

Does this change contain code from or inspired by another project?

No
Yes

If "Yes," did you notify that project's maintainers and provide attribution?

No
Yes

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Notes:

welcome · 2021-07-16T07:34:02Z

💖 Thanks for opening your first pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix. Examples of commit messages with semantic prefixes: - fix: change azure disk cachingMode to ReadOnly - feat: make maximumLoadBalancerRuleCount configurable - docs: add note on AKS Engine and AKS relationship
Make sure to check out the developer guide for guidance on testing your change.

pkg/api/vlabs/validate.go

jsturtevant · 2021-07-16T15:16:57Z

Thanks for testing this and getting it working! Will this work as is today, or are there new base vm images that are needed?

Can you also update the doc where acceleratedNetworkingEnabledWindows is mentioned: https://github.com/jackfrancis/aks-engine/blob/master/docs/topics/clusterdefinitions.md#agentpoolprofiles

jsturtevant · 2021-07-16T15:18:49Z

fixes: #2281

pkg/api/vlabs/validate.go

jadarsie · 2021-07-16T15:28:06Z

parts/k8s/windowsazurecnifunc.ps1

+    # If there is more than one adapter, use the first adapter that is assigned an ipaddress.
+    foreach($na in $nas)
+    {
+        $netIP = Get-NetIPAddress -ifIndex $na.ifIndex -AddressFamily IPv4 -ErrorAction SilentlyContinue -ErrorVariable a


nit, err or netIPErr may be more descriptive names

parts/k8s/windowsazurecnifunc.ps1

bingbing8 · 2021-07-16T19:27:43Z

Thanks for testing this and getting it working! Will this work as is today, or are there new base vm images that are needed?

Can you also update the doc where acceleratedNetworkingEnabledWindows is mentioned: https://github.com/jackfrancis/aks-engine/blob/master/docs/topics/clusterdefinitions.md#agentpoolprofiles

@jsturtevant, it requires the right drivers are installed on the vm images to support accelerated Networking if it is set to true. Updated the doc. Please review. Thanks!

jsturtevant · 2021-07-16T20:18:50Z

it requires the right drivers are installed

Where do folks get the right drivers? What versions are needed? Will the eventually be in the base Azure Market place images?

jsturtevant · 2021-07-16T20:33:54Z

/azp test pr-e2e

azure-pipelines · 2021-07-16T20:33:59Z

Command 'test' is not supported by Azure Pipelines. Supported commands help: Get descriptions, examples and documentation about supported commands Example: help "command_name" list: List all pipelines for this repository using a comment. Example: "list" run: Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run. Example: "run" or "run pipeline_name, pipeline_name, pipeline_name" where: Report back the Azure DevOps orgs that are related to this repository and org Example: "where" See additional documentation.

jsturtevant · 2021-07-16T20:34:24Z

/azp run pr-e2e

azure-pipelines · 2021-07-16T20:34:34Z

Azure Pipelines successfully started running 1 pipeline(s).

jsturtevant · 2021-07-20T17:19:52Z

@bingbing8 it looks like make generate and a new push, is needed to pass tests

bingbing8 · 2021-07-20T18:11:39Z

@bingbing8 it looks like make generate and a new push, is needed to pass tests

I will work on it later this week.

bingbing8 · 2021-07-26T18:53:55Z

it requires the right drivers are installed

Where do folks get the right drivers? What versions are needed? Will the eventually be in the base Azure Market place images?
Mellanox updated VF drivers to supported nested virtualization in the following driver versions:
CX3: WinOF driver package 5.50.53000 (actual .sys file version 5.50.14740)
CX4: WinOF-2 driver package 2.40.50000 (actual.sys file version 2.40.22511)

The drivers are at Mellanox OFED for Windows - WinOF / WinOF-2.
I am not sure if the drivers will be eventually in the base Azure Market place images, at least it will be in the AKS vhd

chewong · 2021-07-26T18:59:26Z

/azp run pr-e2e

azure-pipelines · 2021-07-26T18:59:41Z

Azure Pipelines successfully started running 1 pipeline(s).

bingbing8 · 2021-07-26T19:30:57Z

/azp run pr-e2e

azure-pipelines · 2021-07-26T19:31:02Z

Commenter does not have sufficient privileges for PR 4585 in repo Azure/aks-engine

chewong · 2021-07-26T19:32:41Z

/azp run pr-e2e

azure-pipelines · 2021-07-26T19:32:51Z

Azure Pipelines successfully started running 1 pipeline(s).

chewong · 2021-07-26T22:09:04Z

/azp run pr-e2e

azure-pipelines · 2021-07-26T22:09:15Z

Azure Pipelines successfully started running 1 pipeline(s).

mainred

Looks good, thank you. Let upstream folks decide

marosset · 2021-09-29T19:01:37Z

/azp pr-e2e

marosset · 2021-09-29T19:02:34Z

@bingbing8 can you rebase your changes on top of master and push changes to your PR branch again?

azure-pipelines · 2021-09-29T19:03:15Z

Command 'pr-e2e' is not supported by Azure Pipelines. Supported commands help: Get descriptions, examples and documentation about supported commands Example: help "command_name" list: List all pipelines for this repository using a comment. Example: "list" run: Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run. Example: "run" or "run pipeline_name, pipeline_name, pipeline_name" where: Report back the Azure DevOps orgs that are related to this repository and org Example: "where" See additional documentation.

marosset · 2021-09-29T19:03:43Z

/azp run pr-e2e

azure-pipelines · 2021-09-29T19:04:23Z

Azure Pipelines successfully started running 1 pipeline(s).

bingbing8 · 2021-10-06T18:34:16Z

@marosset, I am working on it

… changeing var name and report error before print out netiperror. 3. update doc for the ac setting

jsturtevant · 2021-10-06T19:43:10Z

/azp run pr-e2e

azure-pipelines · 2021-10-06T19:43:21Z

Azure Pipelines successfully started running 1 pipeline(s).

bingbing8 · 2021-10-06T20:58:07Z

Using the new binary built from the rebased code, I deployed a cluster with below windows profile:
"windowsProfile": {
"adminUsername": "azureuser",
"adminPassword": "replacepassword1234$",
"sshEnabled": true,
"windowsPublisher": "microsoft-aks",
"windowsOffer": "aks-windows",
"windowsSku": "2019-datacenter-core-smalldisk-2109",
"imageVersion": "17763.2213.210927"
}

I can see Mellanox driver show the windows node and traffic through the adapter
PS C:\Users\azureuser> Get-NetAdapter

Name InterfaceDescription ifIndex Status MacAddress LinkSpeed

vEthernet (nat) Hyper-V Virtual Ethernet Adapter 14 Up 00-15-5D-28-3F-1C 10 Gbps
Ethernet 4 Mellanox ConnectX-3 Virtual Function... 9 Up 00-0D-3A-C4-83-38 40 Gbps
Ethernet 3 Microsoft Hyper-V Network Adapter #3 6 Up 00-0D-3A-C4-83-38 40 Gbps
vEthernet (Ethernet 3) Hyper-V Virtual Ethernet Adapter #2 5 Up 00-0D-3A-C4-83-38 40 Gbps

PS C:\Users\azureuser> Get-NetAdapterStatistics

Name ReceivedBytes ReceivedUnicastPackets SentBytes SentUnicastPackets

vEthernet (nat) 0 0 5341 0
Ethernet 4 9399662 7900 1792747 3678
Ethernet 3 9875021 9078 1731265 2942
vEthernet (Ethernet 3) 9875105 9080 1735089 2943

marosset

/lgtm

jsturtevant · 2021-10-07T16:31:09Z

/lgtm

jsturtevant · 2021-10-07T16:39:36Z

The 1.18 tests have been removed. The rest of the tests have passed going to merge manually.

welcome · 2021-10-07T16:39:44Z

Congrats on merging your first pull request! 🎉🎉🎉

jsturtevant suggested changes Jul 16, 2021

View reviewed changes

pkg/api/vlabs/validate.go Show resolved Hide resolved

pkg/api/vlabs/validate.go Show resolved Hide resolved

jadarsie reviewed Jul 16, 2021

View reviewed changes

jadarsie previously approved these changes Jul 20, 2021

View reviewed changes

bingbing8 dismissed jadarsie’s stale review via 1ea93d0 July 26, 2021 19:30

mainred previously approved these changes Jul 27, 2021

View reviewed changes

jsturtevant previously approved these changes Jul 27, 2021

View reviewed changes

jadarsie previously approved these changes Jul 27, 2021

View reviewed changes

jsturtevant mentioned this pull request Sep 28, 2021

chore: New Windows VHD with 9C patches #4677

Merged

8 tasks

bingbing8 added 10 commits October 6, 2021 11:40

remove validation for accelerated networking

ee8a8be

update to select the right adapter

b816a02

revert lines to disable netadapterRsc

cd0a983

update to generic way to find the adapter with IP address

4590b0b

log error if fail to get the netIP

dc75bfc

minor change to print out the variable

2b5ac18

revert all changes in k8swinsetup

849b3f0

1. revert the chagnes for IsAzureStackCloud and vmsize validation. 2.…

ba131b4

… changeing var name and report error before print out netiperror. 3. update doc for the ac setting

new tempaltes fixing accelerated networking related issue

3f68f0e

update tests for allow enabling AcceleratedNetworkingEnabledWindows

8bb0ea3

bingbing8 dismissed stale reviews from jadarsie, jsturtevant, and mainred via 8bb0ea3 October 6, 2021 18:40

bingbing8 force-pushed the master branch from 8828cab to 8bb0ea3 Compare October 6, 2021 18:40

include true inside `

0272bd2

marosset approved these changes Oct 7, 2021

View reviewed changes

jsturtevant approved these changes Oct 7, 2021

View reviewed changes

jsturtevant merged commit b1cdd1e into Azure:master Oct 7, 2021

bridgetkromhout mentioned this pull request Feb 24, 2022

kubelet failed to start on windows nodes with accelerated networking #2281

Closed

fix: enable Accelerated Networking settings and pick up the right adapter in windows azure function #4585

fix: enable Accelerated Networking settings and pick up the right adapter in windows azure function #4585

Conversation

bingbing8 commented Jul 16, 2021

welcome bot commented Jul 16, 2021

jsturtevant commented Jul 16, 2021

jsturtevant commented Jul 16, 2021

jadarsie Jul 16, 2021

Choose a reason for hiding this comment

bingbing8 Jul 16, 2021

Choose a reason for hiding this comment

bingbing8 commented Jul 16, 2021 • edited Loading

jsturtevant commented Jul 16, 2021

jsturtevant commented Jul 16, 2021

azure-pipelines bot commented Jul 16, 2021

jsturtevant commented Jul 16, 2021

azure-pipelines bot commented Jul 16, 2021

jsturtevant commented Jul 20, 2021

bingbing8 commented Jul 20, 2021

bingbing8 commented Jul 26, 2021 • edited Loading

chewong commented Jul 26, 2021

azure-pipelines bot commented Jul 26, 2021

bingbing8 commented Jul 26, 2021

azure-pipelines bot commented Jul 26, 2021

chewong commented Jul 26, 2021

azure-pipelines bot commented Jul 26, 2021

chewong commented Jul 26, 2021

azure-pipelines bot commented Jul 26, 2021

mainred left a comment

Choose a reason for hiding this comment

marosset commented Sep 29, 2021

marosset commented Sep 29, 2021

azure-pipelines bot commented Sep 29, 2021

marosset commented Sep 29, 2021

azure-pipelines bot commented Sep 29, 2021

bingbing8 commented Oct 6, 2021

jsturtevant commented Oct 6, 2021

azure-pipelines bot commented Oct 6, 2021

bingbing8 commented Oct 6, 2021

marosset left a comment

Choose a reason for hiding this comment

jsturtevant commented Oct 7, 2021

jsturtevant commented Oct 7, 2021

welcome bot commented Oct 7, 2021

bingbing8 commented Jul 16, 2021 •

edited

Loading

bingbing8 commented Jul 26, 2021 •

edited

Loading