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

feat: Added NIC type and modified setRoutes function to support Windows #2679

Merged
merged 3 commits into from
Jun 8, 2024
Merged

feat: Added NIC type and modified setRoutes function to support Windows #2679

merged 3 commits into from
Jun 8, 2024

Conversation

bohuini
Copy link
Contributor

@bohuini bohuini commented Apr 8, 2024 •
edited
Loading

Reason for Change:

  1. Added Accelnet NIC type
  2. Modified setRoutes function to differentiate OS
  3. Modified the name of NIC type to align with the naming conventions in DNC.
  4. Modified CRD Pipeline
  5. Added unit test

Issue Fixed:

Requirements:

Notes:

@bohuini bohuini changed the title Added NIC type and modified setRoutes function to support Windows feat: Added NIC type and modified setRoutes function to support Windows Apr 8, 2024
timraymond
timraymond reviewed Apr 11, 2024
View reviewed changes
cns/middlewares/k8sSwiftV2.go Outdated Show resolved Hide resolved
cns/middlewares/k8sSwiftV2_linux.go Outdated Show resolved Hide resolved
cns/middlewares/k8sSwiftV2_linux.go Outdated Show resolved Hide resolved
cns/middlewares/k8sSwiftV2_linux.go Show resolved Hide resolved
cns/middlewares/k8sSwiftV2_linux.go Outdated Show resolved Hide resolved
cns/middlewares/k8sSwiftV2_windows.go Outdated Show resolved Hide resolved
estebancams
estebancams previously approved these changes Apr 11, 2024
View reviewed changes
@bohuini bohuini marked this pull request as ready for review April 12, 2024 21:15
@bohuini bohuini requested review from a team as code owners April 12, 2024 21:15
nddq
nddq reviewed Apr 16, 2024
View reviewed changes
Copy link
Contributor

@nddq nddq left a comment

Choose a reason for hiding this comment

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

lgtm, can you resolve the failing PR checks? and then ill approve it

@bohuini bohuini added the cns Related to CNS. label Apr 18, 2024
@bohuini bohuini requested a review from a team as a code owner April 22, 2024 22:18
@bohuini bohuini requested a review from jaer-tsun April 22, 2024 22:18
@rbtr
Copy link
Contributor

rbtr commented Apr 23, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bohuini bohuini added the go Pull requests that update Go code label Apr 26, 2024
@bohuini
Copy link
Contributor Author

bohuini commented Apr 26, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Azure Azure deleted a comment from azure-pipelines bot Apr 26, 2024
@Azure Azure deleted a comment from azure-pipelines bot Apr 26, 2024
@bohuini bohuini enabled auto-merge April 26, 2024 09:21
@bohuini bohuini disabled auto-merge April 26, 2024 19:02
nddq
nddq previously approved these changes Apr 26, 2024
View reviewed changes
Copy link
Contributor

@nddq nddq left a comment

Choose a reason for hiding this comment

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

lgtm, please seek reviews from cni-reviewers since you made some changes there as well.

@bohuini
Copy link
Contributor Author

bohuini commented Jun 4, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bohuini
Copy link
Contributor Author

bohuini commented Jun 5, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bohuini
Copy link
Contributor Author

bohuini commented Jun 5, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bohuini
Copy link
Contributor Author

bohuini commented Jun 5, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bohuini
Copy link
Contributor Author

bohuini commented Jun 5, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bohuini
Copy link
Contributor Author

bohuini commented Jun 5, 2024

/azp run Azure Container Networking PR

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bohuini bohuini enabled auto-merge June 6, 2024 00:33
@bohuini bohuini added this pull request to the merge queue Jun 7, 2024
Merged via the queue into Azure:master with commit 13f7037 Jun 8, 2024
11 checks passed
@bohuini bohuini deleted the bohui_supportAKSWindows branch June 8, 2024 01:03
kmurudi added a commit that referenced this pull request Aug 21, 2024
@kmurudi
Revert "feat: Added NIC type and modified setRoutes function to suppo…
e4fa55c 
…rt Windows (#2679)"

This reverts commit 13f7037.
github-merge-queue bot pushed a commit that referenced this pull request Aug 23, 2024
@kmurudi
Revert accelnet cns 1.5.32 (#2948)
9c20b43 
* Revert "fix: Modified the value of DelegatedVMNIC to resolve compatibility issue in Version 1.5 (#2808)"

This reverts commit 2b45998.

* Revert "feat: Added NIC type and modified setRoutes function to support Windows (#2679)"

This reverts commit 13f7037.
github-merge-queue bot pushed a commit that referenced this pull request Aug 23, 2024
@kmurudi
Revert accelnet cns 1.5.32 (#2948)
c07a4a2 
* Revert "fix: Modified the value of DelegatedVMNIC to resolve compatibility issue in Version 1.5 (#2808)"

This reverts commit 2b45998.

* Revert "feat: Added NIC type and modified setRoutes function to support Windows (#2679)"

This reverts commit 13f7037.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shreyashastantram shreyashastantram shreyashastantram left review comments

@kmurudi kmurudi kmurudi left review comments

@timraymond timraymond timraymond left review comments

@paulyufan2 paulyufan2 paulyufan2 approved these changes

@nddq nddq nddq approved these changes

@estebancams estebancams estebancams left review comments

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

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

Assignees
No one assigned
Labels
cns Related to CNS. go Pull requests that update Go code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@bohuini @rbtr @kmurudi @timraymond @jaer-tsun @nddq @estebancams @shreyashastantram @paulyufan2