qmsk / clusterf

Clustered IPVS load balancer control plane with Docker integration
MIT License
35 stars 4 forks source link

clusterf-ipvs: apply wildcard configs only to objects from that config source #7

Closed SpComb closed 8 years ago

SpComb commented 8 years ago

If we have multiple config sources, such as -config-path and etcd, then we can get multiple top-level ConfigService and ConfigRoute events. The current Services.config logic assumes that any such event applies to all services/routes, but it should only apply to those services/routes from that specific config source.

This breaks things like etcdctl rm -recursive /clusterf/routes, which will also delete any locally-configured routes, but also breaks any local routes on startup:

A Route is first configured from the local source:

    clusterf-ipvs[9875]: 2015/12/21 12:40:43 config:Files.Open: /srv/clusterf/config
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 config:Files.Scan: 8 configs
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 clusterf: config new &config.ConfigRoute{RouteName:"", Route:config.Route{Prefix4:"", Gateway4:"", IpvsMethod:""}}
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 clusterf: config new &config.ConfigRoute{RouteName:"docker-hosts", Route:config.Route{Prefix4:"10.109.109.0/24", Gateway4:"", IpvsMethod:"droute"}}
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 clusterf:Route docker-hosts: new &{RouteName:docker-hosts Route:{Prefix4:10.109.109.0/24 Gateway4: IpvsMethod:droute}}
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 clusterf:Route docker-hosts: &{Name:docker-hosts Prefix4:10.109.109.0/24 Gateway4:<nil> ipvs_fwdMethod:0xc82011c100 ipvs_filter:false}

Then the /clusterf/route directory is scanned from etcd:

    clusterf-ipvs[9875]: 2015/12/21 12:40:43 config:Etcd.Scan: 22 configs
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 clusterf: config new &config.ConfigRoute{RouteName:"", Route:config.Route{Prefix4:"", Gateway4:"", IpvsMethod:""}}
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 clusterf:Route docker-hosts: new &{RouteName: Route:{Prefix4: Gateway4: IpvsMethod:}}
    clusterf-ipvs[9875]: 2015/12/21 12:40:43 clusterf:Route docker-hosts: &{Name:docker-hosts Prefix4:<nil> Gateway4:<nil> ipvs_fwdMethod:<nil> ipvs_filter:false}

This is NewConfig event is distributed to the existing Route, and breaks the Route's configuration.

Quickfix would be to only distribute DelConfig events to all objects. To fix recursive removes, objects would need to be tagged with their config source, and then recursive DelConfig events only distributed to objects for that source.

SpComb commented 8 years ago

I added a testcase for this

go test -v github.com/qmsk/clusterf/
=== RUN   TestNewConfigRoute
2015/12/22 14:47:23 clusterf: config new &config.ConfigRoute{RouteName:"", Route:config.Route{Prefix4:"", Gateway4:"", IpvsMethod:""}}
2015/12/22 14:47:23 clusterf: config new &config.ConfigRoute{RouteName:"test", Route:config.Route{Prefix4:"10.0.0.0/24", Gateway4:"", IpvsMethod:"droute"}}
2015/12/22 14:47:23 clusterf:Route test: new &{RouteName:test Route:{Prefix4:10.0.0.0/24 Gateway4: IpvsMethod:droute}}
2015/12/22 14:47:23 clusterf:Route test: &{Name:test Prefix4:10.0.0.0/24 Gateway4:<nil> ipvs_fwdMethod:0xc8201086e0 ipvs_filter:false}
2015/12/22 14:47:23 clusterf: config new &config.ConfigRoute{RouteName:"", Route:config.Route{Prefix4:"", Gateway4:"", IpvsMethod:""}}
2015/12/22 14:47:23 clusterf:Route test: new &{RouteName: Route:{Prefix4: Gateway4: IpvsMethod:}}
2015/12/22 14:47:23 clusterf:Route test: &{Name:test Prefix4:<nil> Gateway4:<nil> ipvs_fwdMethod:<nil> ipvs_filter:false}
--- FAIL: TestNewConfigRoute (0.00s)
    route_test.go:38: test route mis-configured: &clusterf.Route{Name:"test", Prefix4:(*net.IPNet)(nil), Gateway4:net.IP(nil), ipvs_fwdMethod:(*ipvs.FwdMethod)(nil), ipvs_filter:false}
FAIL
exit status 1
FAIL    github.com/qmsk/clusterf        0.013s
SpComb commented 8 years ago

Fixed for routes, but something similar also applies to Services. Might even consider some merging of Backends from multiple config sources... or just stick to a single ConfigSource per Service.

SpComb commented 8 years ago

The new config rewrite implements merging of Routes and ServiceBackends from multiple sources with limited recursive delete, but the code is untested and somewhat fragile.

We should either forbid mixed-source Services, or make sure we deal with them sensibly. Main weirdness if if one source defines the ServiceFrontend, and other sources also define ServiceBackends.

SpComb commented 8 years ago

Commit b9b6520e replaces the in-place merging/recursive-delete logic with explicit merging of multiple per-Source Configs. The Route/ServiceFrontend/ServiceBackend nodes from later --config-source's override any earlier conflicting ones.