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.

Sign up for GitHub

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

Jump to bottom

Remove bridgeName field from CNI windows conflist files #3135

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Remove bridgeName field from CNI windows conflist files #3135

merged 1 commit into from
Nov 14, 2024

Conversation

paulyufan2
Copy link
Contributor

@paulyufan2 paulyufan2 commented Nov 12, 2024 •
edited
Loading

Reason for Change:

This PR is to cleanup windows cni config file to remove bridgeName fields

Issue Fixed:

Requirements:

Notes:

@paulyufan2 paulyufan2 added the cni Related to CNI. label Nov 12, 2024
@paulyufan2 paulyufan2 requested a review from a team as a code owner November 12, 2024 15:31
@paulyufan2 paulyufan2 force-pushed the cleanupcniwindowsconfig branch 2 times, most recently from e91aaf7 to 72a8bd7 Compare November 12, 2024 22:14
QxBytes
QxBytes reviewed Nov 12, 2024
View reviewed changes
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.

So just confirming that "bridge" (the field) isn't used in windows, and mode will default to bridge if it is not specified? Could you also run the full pipeline with this change?

@paulyufan2
Copy link
Contributor Author

@QxBytes pipeline passes

@paulyufan2 paulyufan2 force-pushed the cleanupcniwindowsconfig branch 2 times, most recently from a1c4831 to 69e6239 Compare November 13, 2024 18:42
QxBytes
QxBytes approved these changes Nov 13, 2024
View reviewed changes
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.

On windows, a reference search for the bridgeName field of endpoint info does not reveal any usages. it seems like you're only modifying windows conflists so there shouldn't be any effect.

@paulyufan2
Copy link
Contributor Author

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@paulyufan2 paulyufan2 added this pull request to the merge queue Nov 14, 2024
Merged via the queue into master with commit 5d4c4dc Nov 14, 2024
14 checks passed
@paulyufan2 paulyufan2 deleted the cleanupcniwindowsconfig branch November 14, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QxBytes QxBytes QxBytes approved these changes

@rbtr rbtr Awaiting requested review from rbtr rbtr is a code owner automatically assigned from Azure/acn-cni-reviewers

Assignees
No one assigned
Labels
cni Related to CNI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@paulyufan2 @QxBytes