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

Disable accelerated network for AvSet clusters #100

Merged

Conversation

vpnachev
Copy link
Member

How to categorize this PR?

/area networking
/kind bug
/priority critical
/platform azure

What this PR does / why we need it:
Disable accelerated networking for AvSet clusters. This is necessary because for legacy clusters it is not possible to create new VMs. The creation calls are failing with

ExistingAvailabilitySetWasNotDeployedOnAcceleratedNetworkingEnabledCluster" Message="Cannot add a virtual machine (/subscriptions/...) with accelerated-networking-enabled network interface(s) on an existing availability set (/subscriptions/...) with VM(s) without accelerated-networking-enabled network interfaces.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

See MicrosoftDocs/azure-docs#10536 for more details.

Release note:

Accelerated Networking feature has been disabled for AvSet clusters. It is still available for zoned clusters. 

@vpnachev vpnachev requested review from a team as code owners May 28, 2020 16:41
@ghost ghost added area/networking Networking related kind/bug Bug platform/azure Microsoft Azure platform/infrastructure priority/critical Needs to be resolved soon, because it impacts users negatively labels May 28, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 28, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 28, 2020
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice PR, lgtm, I would just enhance our user-facing docs. :)

docs/usage-as-end-user.md Outdated Show resolved Hide resolved
@vpnachev vpnachev force-pushed the disable-acc-network-for-avset-clusters branch from 2fe5fb4 to 3d2de40 Compare May 28, 2020 17:18
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 28, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 28, 2020
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@ghost ghost added the reviewed/lgtm Has approval for merging label May 28, 2020
@vpnachev vpnachev merged commit be1ffc3 into gardener:master May 28, 2020
@vpnachev vpnachev deleted the disable-acc-network-for-avset-clusters branch May 28, 2020 17:56
@dkistner
Copy link
Member

Oh thats a pity. Once more on case which was not part of the test matrix... To many deployment scenarios on Azure...

So we can reenable it when we have an own avset for each workerpool. These avset contain then only machines of same type and configuration (e.g. operating system).

@vpnachev vpnachev changed the title Diable accelerated network for AvSet clusters Disable accelerated network for AvSet clusters May 29, 2020
@vpnachev
Copy link
Member Author

So we can reenable it when we have an own avset for each workerpool. These avset contain then only machines of same type and configuration (e.g. operating system).

I am not sure - if the worker pool is rolled from disabled accelerated network to enabled, we will face the same issue. So we have to keep status of the AvSet and disallow accelerated networking if it still has NICs with disabled accelerated networking. Which means we will have to list all NICs in the AvSet which is one more operation contributing to the Azure API calls.

@dkistner
Copy link
Member

Hmm. I rather thinking in the direction of creating a new avset in case the machineclass spec is changing. The rolled machines (with acc networking) would then be part of a new avset and the not rolled ones would be part of the old avset.

@vpnachev
Copy link
Member Author

Valid point, I've opened #101 to summarize the topic and keep track on it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking Networking related kind/bug Bug needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/azure Microsoft Azure platform/infrastructure priority/critical Needs to be resolved soon, because it impacts users negatively reviewed/lgtm Has approval for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants