aledbf / kube-keepalived-vip

Kubernetes Virtual IP address/es using keepalived
Apache License 2.0
190 stars 75 forks source link

Extended configMap #80

Open mshaverdo opened 5 years ago

mshaverdo commented 5 years ago

In our usecase we met several problems:

  1. we have several services on the same IP, but it is impossible to specify them in the keepalived-vip configmap due to CM key is justs IP
  2. services defined on several different interfaces, so we need to run several instances of keepalived-vip, one per interface

To solve this problems, we've extended configmap semantics: configmap key now is [OPTIONAL_INDEX-]IP_ADDRESS[@OPTIONAL_INTERFACE], e.g.: 01-10.0.0.1@eth0 or 01-10.0.0.1 or 10.0.0.1@eth0 or just 10.0.0.1. It allows to specify several services on single IPs and explicitly specify interface name. If interface is not specified in configmap, it uses interface specified via --iface flag

It fixes #12 and fixes #77

CLAassistant commented 5 years ago

CLA assistant check
All committers have signed the CLA.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+9.2%) to 24.254% when pulling 718a65d676b60572c4cff72daef06d33ffb9cfca on mshaverdo:extended_configmap into f3b9e294329fda472a46391b9e01e7c81f977d74 on aledbf:master.

steven-sheehy commented 5 years ago

Can you mark this as fixes #12 and fixes #77?

I'm not a fan of the new format, but it is at least backwards compatible. If we break backwards compatibility we can redesign it as a list instead of map. Let's see what @aledbf thinks.

aledbf commented 5 years ago

@mshaverdo @steven-sheehy I don't like the new format. Right now, using a configmap makes no sense with all the features we are trying to define in a key-value string definition. I think is time to evolve using a no namespaced CRD

mshaverdo commented 5 years ago

@mshaverdo @steven-sheehy Maybe, it's a good idea to summarize desired features?

steven-sheehy commented 5 years ago

Maybe IPv6 support #64?

Looks like multiple nics will also fix #73.

panpan0000 commented 5 years ago

@mshaverdo @steven-sheehy I don't like the new format. Right now, using a configmap makes no sense with all the features we are trying to define in a key-value string definition. I think is time to evolve using a no namespaced CRD

👍 concur on CRD I met the same situation. external IP is quite valuable, reuse the same external IP in NAT mode, map different backend service to different host port is a tradeoff.

panpan0000 commented 5 years ago

another major comment for this implementation: image there's two lines in the configMap, one says using "interface:eth0", another says "interface:eth1".

But interface is a property of vrrp_instance . There's single vrrp_instance(named vips ) generated in the /etc/keepalived/keepalived.conf. For different items in the configMap, each item will have their own virtual_server, but belongs to the same vrrp_instance . So in above case, I believe the eth1 will override the eth0 line in configMap.( I did not try )

so I think the resolution for diff NIC interface is : a global config item in CRD...

panpan0000 commented 5 years ago

another major comment for this implementation: image there's two lines in the configMap, one says using "interface:eth0", another says "interface:eth1".

But interface is a property of vrrp_instance . There's single vrrp_instance(named vips ) generated in the /etc/keepalived/keepalived.conf. For different items in the configMap, each item will have their own virtual_server, but belongs to the same vrrp_instance . So in above case, I believe the eth1 will override the eth0 line in configMap.( I did not try )

so I think the resolution for diff NIC interface is : a global config item in CRD...

forgive my ignorance. iface can be VIP granularity .

  virtual_ipaddress {
    1.2.3.4 dev ens192
    1.2.3.5 dev ens224
  }