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

cmd/netsetgo: Enforce pid argument #2

Merged
merged 1 commit into from Jun 3, 2018
Merged

cmd/netsetgo: Enforce pid argument #2

merged 1 commit into from Jun 3, 2018

Conversation

marcosps
Copy link
Contributor

@marcosps marcosps commented Jun 1, 2018

If we call netsetgo without arguments, it returns an error:
ERROR - no such process

It fails because Veth.MoveToNetworkNamespace calls
netlink.LinkSetNsPid, which needs a pid that belongs to another netns in
order to set a veth interface inside the new netns.

So, before doing any setup, check if a pid was passed as argument of
netsetgo.

Signed-off-by: Marcos Paulo de Souza marcos.souza.org@gmail.com

@teddyking
Copy link
Owner

Thanks @marcosps!

Any chance you'd be able to add a test for this as well?
Maybe something to https://github.com/teddyking/netsetgo/blob/master/integration_test.go along the lines of

Context("when an invalid pid is provided", func() { 
    It("exits -1", func() {  
        ... 
    })
})

@marcosps
Copy link
Contributor Author

marcosps commented Jun 1, 2018

Sure, will try to do so in a few hours!

@marcosps
Copy link
Contributor Author

marcosps commented Jun 2, 2018

@teddyking I'm struggling to install this concourse ci in my Ubuntu, but don't worry, it won't take the whole weekend (that's what I expect at least...)

@teddyking
Copy link
Owner

Oh yeah that can be a bit tricky. You should also be able to just run the tests by installing ginkgo and running ginkgo -r from the root of the netsetgo repo. Although note that you'll need to be the root user and this will actually add/remove network interfaces from your machine (should be fairly safe but just FYI).

No worries if you can't get it working, I can add a test in later.

@marcosps
Copy link
Contributor Author

marcosps commented Jun 2, 2018

@teddyking tricky yes, but I made it work :)

I hope this now fulfills your expectations!

If we call netsetgo without arguments, it returns an error:
ERROR - no such process

It fails because Veth.MoveToNetworkNamespace calls
netlink.LinkSetNsPid, which needs a pid that belongs to another netns in
order to set a veth interface inside the new netns.

So, before doing any setup, check if a pid was passed as argument of
netsetgo.

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
@teddyking teddyking merged commit 211d904 into teddyking:master Jun 3, 2018
@teddyking
Copy link
Owner

Awesome, looks good to me.

Thanks!
Ed

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.

None yet

2 participants