romana / core

Romana core components - Micro services written in Go.
Apache License 2.0
47 stars 11 forks source link

Allocate address using network IP instead of plain IP from CIDR #42

Closed cgilmour closed 8 years ago

cgilmour commented 8 years ago

Before: romana_ip needs to be configured as network address (10.1.0.0/16) instead of IP and mask (10.1.0.1/16) Using a .1 address in this config would make IPAM allocate only even addresses. (It would take the full address 10.1.0.1 and add tenant/segment bits, instead of taking the network address 10.1.0.0). After: always calculate off network address.

Impact: k8s-listener is using romana_ip when it detects a new policy and tries to send instructions to the listener-agent to create rules.

jbrendel commented 8 years ago

Ok

cgilmour commented 8 years ago

Agent errors if romana_ip was configured as a host+mask instead of network address.

2016/02/25 02:13:49 Agent: ensuring interhost routes exist
2016/02/25 02:13:49 Acquiring mutex ensureInterhostRoutes
2016/02/25 02:13:49 Acquired mutex ensureInterhostRoutes
2016/02/25 02:13:49 otherHosts(): Our host is 0 out of 2, all hosts: [{1 ip-192-168-99-10 192.168.99.10 10.0.0.1/16 %!s(int=0) []} {2 ip-192-168-99-11 192.168.99.11 10.1.0.1/16 %!s(int=0) []}]
2016/02/25 02:13:49 Helper.Executor: executing command: /sbin/ip [ro show 10.1.0.1/16]
2016/02/25 02:13:49 Helper: creating route
2016/02/25 02:13:49 Helper.Executor: executing command: /sbin/ip [ro add 10.1.0.1/16 via 192.168.99.11]
2016/02/25 02:13:49 Releasing mutex ensureInterhostRoutes
2016/02/25 02:13:49 Unspecified error (Agent: Can't create IP route (target 10.1.0.1/16 -> 192.168.99.11: cause External command unsuccessful (/sbin/ip [ro add 10.1.0.1/16 via 192.168.99.11]: exit status 2)))

This didn't happen with Stas's installer because routes are created at installation time, so the agent never attempts to create inter-host routes itself.

cgilmour commented 8 years ago

http://play.golang.org/p/V55by7mY_l So added another commit (1de17cd) to handle that.

jbrendel commented 8 years ago

If the bug is fixed then we should go ahead and merge it. I'll create a card for rename/refactor IPv4ToInt().