traffic shaping: take configuration via a runtimeConfig

[m1093782566]

To make it easier to supply bandwidth in a dynamic manner, the traffic shaping plugin should support a capability arg.

Fixes: #133

[m1093782566]

@squeed @caseydavenport @dcbw @rosenhouse

[m1093782566]

• Failure [0.796 seconds]
Basic PTP using cnitool when the bandwidth plugin is chained with a plugin that returns multiple adapters [Measurement] limits traffic only on the restricted bandwith veth device 
/home/travis/gopath/src/github.com/containernetworking/plugins/gopath/src/github.com/containernetworking/plugins/integration/integration_linux_test.go:151
  Expected
      <time.Duration>: 26698362
  to be >
      <time.Duration>: 1023954067
  /home/travis/gopath/src/github.com/containernetworking/plugins/gopath/src/github.com/containernetworking/plugins/integration/integration_linux_test.go:188

The failed test seems flaky, so how to re-trigger the test?

[dcbw]

@m1093782566 restarted...

[dcbw]

@m1093782566 I think it's useful to have both runtime config and static config; also allows limiting if the runtime doesn't yet support the dynamic modes. Other than that, looks OK... Maybe if the static config is specified, it overrides runtime config?

[Lion-Wei]

@dcbw By static config you mean config like this?

{
            "type": "bandwidth",
            "ingressRate": 800,
            "ingressBurst": 200,
            "egressRate": 800,
            "egressBurst": 200
}

[m1093782566]

@dcbw CI is green now.

[dcbw]

@Lion-Wei yeah, like that.

[squeed]

I agree with @dcbw - the plugin should take both static and runtime configuration

[Lion-Wei]

@dcbw @squeed , make sense. We can do that. : )

[squeed]

I suggest you add an IsZero method on the BandwidthEntry type - makes these comparisons simpler.

[squeed]

Also, if you make the nested BandwidthEntry struct in config a pointer, this can be simpler.

[squeed]

The nested struct can be a pointer, and the json deserializer will handle it correctly - makes it easy to track empty values.

[squeed]

We do something similar with the host-local IPAM, if you want to see it in action.

[Lion-Wei]

Good Point, thanks, Now wit's can be much simpler, PTAL. : )

[squeed]

Minor request: The "w" in "bandwidth" should be lowercase everywhere, including config.

[squeed]

A few minor things, but otherwise looks good! When you're all done, can you squash to a single commit (and force-push to your branch)?

[m1093782566]

@squeed I squashed the commits and it still have two commits: one is mine and the other one is my friend @Lion-Wei 's as I paired with him on this work :)

[m1093782566]

@squeed @dcbw

All comments are addressed now. PTAL. BTW, the work of having Kubernetes CNI network driver support traffic shaping still relies on this PR.

[squeed]

Looks good to me. Is there any reason you changed the bandwidth values in the integration test? Nice job, btw.

[m1093782566]

@squeed

Oops... It is changed back now. Please take a look again when you have a chance, thanks!

[squeed]

LGTM! Thanks! Now just need a second reviewer.

[m1093782566]

Thanks @squeed

@dcbw would you please take a look?

[rosenhouse]

back ref: kubernetes/kubernetes#63194