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

Add add route func in container #59

Merged
merged 2 commits into from
Jul 23, 2018
Merged

Add add route func in container #59

merged 2 commits into from
Jul 23, 2018

Conversation

sufuf3
Copy link
Contributor

@sufuf3 sufuf3 commented Jul 18, 2018

No description provided.

@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch from 384593e to 19f0b7b Compare July 18, 2018 12:04
@codecov
Copy link

codecov bot commented Jul 18, 2018 •
edited
Loading

Codecov Report

Merging #59 into master will increase coverage by 0.56%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   87.17%   87.74%   +0.56%     
==========================================
  Files           7        7              
  Lines         195      204       +9     
==========================================
+ Hits          170      179       +9     
  Misses         16       16              
  Partials        9        9
Impacted Files Coverage Δ
nl/link.go 71.69% <100%> (+5.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e336840...b4ee81c. Read the comment docs.

John-Lin
John-Lin suggested changes Jul 18, 2018
View reviewed changes
nl/link.go Outdated
func AddRoute(ipn *net.IPNet, dev string) error {
link, err := netlink.LinkByName(dev)
if err != nil {
fmt.Print(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

should return error

@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch from 19f0b7b to de69bc0 Compare July 18, 2018 12:09
John-Lin
John-Lin reviewed Jul 18, 2018
View reviewed changes
messages/messages.proto Outdated

message ConfigureRouteRequest {
string path = 1;
string dstIPNet = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

use dstCIDR

@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch from de69bc0 to b567d1d Compare July 18, 2018 16:22
@John-Lin
Copy link
Contributor

rebase and add some test cases

@sufuf3
Copy link
Contributor Author

sufuf3 commented Jul 19, 2018

Got it.
I will do this tonight. :)
Thank you.

@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch 3 times, most recently from 3b541e3 to 59d0ab4 Compare July 19, 2018 15:57
@sufuf3
Copy link
Contributor Author

sufuf3 commented Jul 19, 2018 •
edited
Loading

I rebase to the latest master and used https://github.com/cweill/gotests to generate the go test.

@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch 4 times, most recently from 73a7a0a to f2bc4a7 Compare July 20, 2018 03:15
@John-Lin
Copy link
Contributor

現在有考慮要把 gw加回來 下週我們討論一下再改

@sufuf3
Copy link
Contributor Author

sufuf3 commented Jul 20, 2018

好唷
Thanks

@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch from f2bc4a7 to f63395b Compare July 23, 2018 05:18
@sufuf3 sufuf3 changed the title Add config route func in container Add add route func in container Jul 23, 2018
@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch from f63395b to ea4ad48 Compare July 23, 2018 05:39
@sufuf3
Copy link
Contributor Author

sufuf3 commented Jul 23, 2018

@John-Lin I finished this PR.
would you please help me review this PR?
Many thanks

@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch 3 times, most recently from f649fa1 to c6815c7 Compare July 23, 2018 06:39
@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch from c6815c7 to e1d0b17 Compare July 23, 2018 06:59
@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch 3 times, most recently from 35a944b to 2f262ac Compare July 23, 2018 07:34
@sufuf3 sufuf3 force-pushed the sufuf3/add-route branch from 2f262ac to b4ee81c Compare July 23, 2018 07:35
@John-Lin John-Lin merged commit 32f5f34 into master Jul 23, 2018
@hwchiu hwchiu deleted the sufuf3/add-route branch July 23, 2018 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hwchiu hwchiu hwchiu approved these changes

@John-Lin John-Lin John-Lin approved these changes

@chenyunchen chenyunchen chenyunchen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sufuf3 @John-Lin @hwchiu @chenyunchen