Skip to content

Conversation

@aegal
Copy link
Contributor

@aegal aegal commented Sep 16, 2021

Reason for Change:

Issue Fixed:

Requirements:

Notes:

rbtr and others added 8 commits September 14, 2021 13:00
* fix debug commands

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* fix: update cns client

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* add ctx to debug calls

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* repackage cns client

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* add ctx to all methods and preinit all route urls

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* down-scope cns client interface and move to consumer packages

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* no unkeyed struct literals

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* trace updated client method signatures out through windows paths

* delint

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* fix windows build

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* delint

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
* Initial pass at Netlink interface

* changing some netlink and epc

* Resolcing all dependencies on netlink package

* first pass at adding a netlinkinterface

* windows working now

* feat: update cns client (#992)

* fix debug commands

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* fix: update cns client

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* add ctx to debug calls

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* repackage cns client

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* add ctx to all methods and preinit all route urls

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* down-scope cns client interface and move to consumer packages

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* no unkeyed struct literals

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* trace updated client method signatures out through windows paths

* delint

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* fix windows build

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* delint

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* windows working now

* Some golints checks

* commenting a flaky NPM UT and adding some golint checks

* renaming fakenetlink to mocknetlink

* removing a mock netlink usage

* fixing more golints and a test fix

* fixing more go lints

* Adding in netlink from higher level as input

* adding netlinkinterface to windows endpoint impl

* removing netlink name confusion

Co-authored-by: Evan Baker <rbtr@users.noreply.github.com>
Copy link
Contributor

@vakalapa vakalapa left a comment

Choose a reason for hiding this comment

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

Some initial questions

"github.com/Microsoft/hcsshim/hcn"
)

var hnsv2 hnswrapper.HnsV2WrapperInterface = hnswrapper.Hnsv2wrapper{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of initializing it here, can you pass this in for each needed call. And let this be part of NetworkManager. We can initialize this in NewnetworkManager. That way we can control what gets passed on early on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to avoid passing around os specific code such as HNS in platform agnostic code. By passing from the top down, its my opinion that it muddies the code. Happy to chat more about it though.

@aegal aegal requested a review from vakalapa September 16, 2021 18:43
@rbtr rbtr changed the base branch from dev/utcoverage to master September 20, 2021 19:08
shriroopjoshi
shriroopjoshi previously approved these changes Sep 20, 2021
@aegal aegal requested a review from shriroopjoshi September 20, 2021 20:59
@aegal aegal merged commit 62172a4 into master Sep 21, 2021
@rbtr rbtr deleted the alegal/hns_tests branch September 21, 2021 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants