Feature/476 multiple nics #757
Conversation
Codecov Report
@@ Coverage Diff @@
## master #757 +/- ##
==========================================
- Coverage 72.12% 71.91% -0.21%
==========================================
Files 131 131
Lines 9621 9653 +32
==========================================
+ Hits 6939 6942 +3
- Misses 2268 2295 +27
- Partials 414 416 +2
Continue to review full report at Codecov.
|
js/runner.go
Outdated
| // Select given interface | ||
| targetIf, err := net.InterfaceByName(ifaceName) | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
This type of error suppression throughout the whole function probably isn't the best way to go about things. If some user gives a wrong interface name, then I'd expect k6 to emit an error, not silently continue working without using the specified interface
js/runner.go
Outdated
| localTCPAddr := net.TCPAddr{ | ||
| IP: a.IP, | ||
| } | ||
| r.BaseDialer.LocalAddr = &localTCPAddr |
There was a problem hiding this comment.
This seems problematic... Take a look at the k6 architecture - the Runner (r here) is the thing that spawns VUs, so if you change the LocalAddr of its BaseDialer every time, you're just changing the BaseDialer all VUs would use. I think that with the current code all VUs will end up having the same LocalAddr configured, instead the thing that should be changed is probably the VU-local Dialer that's configured a few lines below this.
|
Hi na, I agree this contribution is much more a proof of concept than a finished implementation . I'll clean up the code and PR again as soon as possible. Thanks Olivier Fauchon |
- Support for multiple interfaces list (comma separated) - Use of Local Vu Dialer instead of BaseDialer
| dialer := &netext.Dialer{ | ||
| Dialer: r.BaseDialer, | ||
| Resolver: r.Resolver, | ||
| Blacklist: r.Bundle.Options.BlacklistIPs, | ||
| Hosts: r.Bundle.Options.Hosts, | ||
| } | ||
| if r.Bundle.Options.Nic.Valid { | ||
| randAddr, err := getRandomIP(r.Bundle.Options.Nic.ValueOrZero()) |
There was a problem hiding this comment.
declaration of "err" shadows declaration at js/runner.go:157 (from govet)
| @@ -113,6 +115,43 @@ func (r *Runner) NewVU(samplesOut chan<- stats.SampleContainer) (lib.VU, error) | |||
| return lib.VU(vu), nil | |||
| } | |||
|
|
|||
| func getRandomIP(ifacesList string) (*net.IPAddr, error) { | |||
|
|
|||
| var addresses []net.Addr | |||
There was a problem hiding this comment.
Consider preallocating addresses (from prealloc)
|
Hey, @ofauchon, if you're picking this up again, you might want to merge |
|
|
|
Hi, In september 2018, I published the initial multi-nic patch. Thanks to @srguglielmo contributions, more enhancement and fixes were done. But I'm surprised this PR could not be merged since Sep. 2018. Do you think it would be possible to spend more time @k6 to help contributors finish these zombies PR. Thanks in advance. Olivier |
|
@ofauchon, hi, and sorry for the long delays here 😞 IIRC this PR was a victim of our focus on #1007... 😞 And it probably should have been closed when #1293 was opened, so I'm going to do so now. The current consensus is that k6 shouldn't accept |
This branch enables a new option "--nic" option
When using this flag eg: --nic eth0" , all the ips of the given interface will be used randomly when running injection scripts.
I have tested this feature with interfaces having 100+ IPv4 addresses.
IPv6 needs more testing though.
Support for multiple nics ( --nic eth0, eth1,eth2...etc) will come on a second time