kvm: transform flannel network to allow teardown #2647
Conversation
|
LGTM. |
| @@ -569,6 +569,10 @@ extend Networking struct with methods to clean up kvm specific network configura | |||
| // removing tuntap interface and releasing its ip from IPAM plugin | |||
| func (n *Networking) teardownKvmNets() { | |||
| for _, an := range n.nets { | |||
| if an.conf.Type == "flannel" { | |||
| kvmTransformFlannelNetwork(&an) | |||
There was a problem hiding this comment.
yyy, where is error handling?
|
Ups, this LGTM was too fast. Missing error checking stands there for -1. |
|
@jellonek Done |
|
Now really LGTM ;) |
| @@ -569,6 +569,13 @@ extend Networking struct with methods to clean up kvm specific network configura | |||
| // removing tuntap interface and releasing its ip from IPAM plugin | |||
| func (n *Networking) teardownKvmNets() { | |||
| for _, an := range n.nets { | |||
| if an.conf.Type == "flannel" { | |||
There was a problem hiding this comment.
can you move this within the switch statement that immediately follows?
There was a problem hiding this comment.
@jonboulle I can't. At first, I transform flannel network and then pass to the switch statement. We use similar check here:
https://github.com/coreos/rkt/blob/master/networking/kvm.go#L431
There was a problem hiding this comment.
ah, I see, thanks for clarifying
| @@ -569,6 +569,13 @@ extend Networking struct with methods to clean up kvm specific network configura | |||
| // removing tuntap interface and releasing its ip from IPAM plugin | |||
| func (n *Networking) teardownKvmNets() { | |||
| for _, an := range n.nets { | |||
| if an.conf.Type == "flannel" { | |||
| if err := kvmTransformFlannelNetwork(&an); err != nil { | |||
| stderr.PrintE("error transforming flannel network", err) | |||
There was a problem hiding this comment.
does it make sense to add here something more from an.runtime to the error print?
There was a problem hiding this comment.
@woodbor I don't think so, It's simple check and clear message, similar to "unsupported network type" below and error handling in seting up the network. But if you think there's some useful information to include here, point it :)
|
LGTM |
Fixes #2636