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

Introduce Azure Accelerated Networking #65

Closed
wants to merge 1 commit into from
Closed

Conversation

dkistner
Copy link
Member

@dkistner dkistner commented Apr 1, 2020

What this PR does / why we need it:
Enable automatically Azure Accelerated Networking for all compatible machines configurations. The machine type and os image version need to be compatible to make use of Azure Accelerated Networking.

Azure Accelerated Networking provide better networking performance, see here.

A list of compatible machine types can be maintained in the CloudProfile via .spec.providerConfig.acceleratedNetworingMachineTypes[].
The os image versions which are compatible can be maintained in the CloudProfile via .spec.providerConfig.machineImages[].versions[].acceleratedNetworking.

ℹ️ This PR require a mcm release which include gardener/machine-controller-manager#438. This is already integrated with #60.

Which issue(s) this PR fixes:
Fixes #55

Special notes for your reviewer:
Create a cluster with compatible machine type and os image version. Ssh to machine and exec instructions here.

Release note:

Azure Accelerated Networking is now automatically enabled if the machine type and operating system + version is compatible.
Azure Accelerated Networking can now be used for machines which use a compatible machine type and operating system + version. Operators need to maintain a list of compatible machine types and label compatible os image version in the Azure CloudProfile.

@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) 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 Apr 1, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 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 Apr 7, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 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 Apr 7, 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 Apr 7, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 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 Apr 7, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has 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 Apr 14, 2020
@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 Apr 14, 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 Apr 14, 2020
@dkistner dkistner marked this pull request as ready for review April 14, 2020 08:19
@dkistner dkistner requested review from a team as code owners April 14, 2020 08:19
docs/usage-as-operator.md Outdated Show resolved Hide resolved
docs/usage-as-operator.md Outdated Show resolved Hide resolved
pkg/apis/azure/types_cloudprofile.go Outdated Show resolved Hide resolved
pkg/apis/azure/v1alpha1/types_cloudprofile.go Outdated Show resolved Hide resolved
pkg/apis/azure/v1alpha1/types_cloudprofile.go Outdated Show resolved Hide resolved
pkg/controller/worker/machines.go Outdated Show resolved Hide resolved
pkg/controller/worker/machines.go Outdated Show resolved Hide resolved
pkg/controller/worker/machines.go Outdated Show resolved Hide resolved
pkg/controller/worker/machines.go Outdated Show resolved Hide resolved
@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 Apr 16, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 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 Apr 16, 2020
@dkistner
Copy link
Member Author

Updated the PR. The requested test is missing. Need some more time to restructure the tests..

pkg/apis/azure/types_cloudprofile.go Outdated Show resolved Hide resolved
pkg/apis/azure/v1alpha1/types_cloudprofile.go Show resolved Hide resolved
pkg/apis/azure/v1alpha1/types_cloudprofile.go Outdated Show resolved Hide resolved
pkg/controller/worker/machines.go Outdated Show resolved Hide resolved
@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 Apr 17, 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 Apr 17, 2020
@dkistner
Copy link
Member Author

Thanks @rfranzke for the review.
I have addressed the changes and extended the tests to cover accelerated networking configuration for the workers .

@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 Apr 17, 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 Apr 17, 2020
for machine type/os image combinations which support it.
@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 Apr 21, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 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 Apr 21, 2020
@rfranzke
Copy link
Member

@dkistner I want to merge, but it tells

This branch has conflicts that must be resolved

without showing them. Can you check? Maybe it's related to the GitHub issues yesterday evening, can you force-push?

rfranzke added a commit to rfranzke/gardener-extension-provider-azure that referenced this pull request Apr 22, 2020
Introduce Azure Accelerated Networking
@rfranzke
Copy link
Member

OK, it seems that this PR was already merged and pushed to master as of 357a30d0377385e0bd6f65bd57ee8c0287101d26, but GitHub didn't mark the PR as merged. Unfortunate.. 👎 I'll then close the PR manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accelerated Network Enablement
5 participants