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

Remove l2bridge checker on windows and l2tunnel mode #3113

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

paulyufan2
Copy link
Contributor

Reason for Change:

This PR is to simplify windows l2bridge mode and remove l2tunnel mode from the system.

Issue Fixed:

Requirements:

Notes:

@paulyufan2 paulyufan2 added the cni Related to CNI. label Nov 7, 2024
@paulyufan2 paulyufan2 requested review from a team as code owners November 7, 2024 15:31
@paulyufan2 paulyufan2 requested a review from QxBytes November 7, 2024 21:21
Copy link
Contributor

@QxBytes QxBytes left a comment

Choose a reason for hiding this comment

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

could we add a unit test where we pass in some value to nwInfo (type: EndpointInfo) and then call configure endpoint, ensuring that the resulting hcn network type is always l2 bridge?

QxBytes
QxBytes previously approved these changes Nov 11, 2024
Copy link
Contributor

@QxBytes QxBytes left a comment

Choose a reason for hiding this comment

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

Confirmed with paul that l2tunnel is not used anywhere. azure-windows-multitenancy.conflist is not modified as it could not be readily tested. Only time networking mode is not bridge in windows is if the nic type is a particular value. Linux side should be unaffected.

@paulyufan2
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@paulyufan2 paulyufan2 enabled auto-merge November 11, 2024 18:20
@rbtr
Copy link
Contributor

rbtr commented Nov 11, 2024

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@rbtr rbtr left a comment

Choose a reason for hiding this comment

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

no objection to the CNS changes

@paulyufan2 paulyufan2 added this pull request to the merge queue Nov 11, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 11, 2024
* remove l2bridge checker and l2tunnel mode

* fix l2bridge type

* fix the comment to add an UT to ensure hcn network type is always l2bridge

* go format to fix the linter issue
Merged via the queue into master with commit 8e8a5fd Nov 12, 2024
14 checks passed
@paulyufan2 paulyufan2 deleted the simplifyCNIConflist branch November 12, 2024 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cni Related to CNI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants