Add multiple egress IP support #1
Conversation
|
I was very impressed with the concept of kube-egress and hoped that this will support multiple egress IP. |
There was a problem hiding this comment.
Thanks for the PR, I provided some feedback. I think to support this in a dynamic environment where pods come and go kube-egress itself would need to watch the kubernetes API for pod/service/endpoint changes and update accordingly. This would probably be tricky to do in a bash program, but not sure.
README.md
Outdated
|
|
||
| ```shell | ||
| kubectl apply -f daemonset.yaml | ||
| ``` | ||
|
|
||
| Use egress_mgr.sh script to automatically update podip-vip-mappings configmap. |
There was a problem hiding this comment.
I don't think egress_mgr.sh is necessary. The user should construct the configmap with the mappings themselves. We can consider adding a helm chart that helps with this in the future.
config.txt
Outdated
| @@ -0,0 +1,2 @@ | |||
| default pod1 192.168.1.1 | |||
There was a problem hiding this comment.
This file and config directory shouldn't be checked in as they're examples. We should instead document format with an example in the README.md.
egress.sh
Outdated
| POD_SUBNET="10.32.0.0/12" | ||
| SERVICE_SUBNET="10.96.0.0/12" | ||
| PODIP_VIP_MAPPING_DIR="config/podip_vip_mapping/" |
There was a problem hiding this comment.
Should be an absolute path. Recommend to be in /etc/kube-egress/.
| name: podip-vip-mappings | ||
| namespace: default | ||
| data: | ||
| 10.244.2.2: 192.168.122.222 |
There was a problem hiding this comment.
Pod IPs are usually dynamic unless using a specific ClusterIP, so I'm not sure the value of hardcoding them here. Using pod names would have the same problem. It would be better to map to VIP based upon some higher level workload like a service or CRD. You'd need to watch for endpoint changes to update iptables appropriately. Not sure how feasible that is in bash.
There was a problem hiding this comment.
Thank you for your feedback.
Pod IPs are usually dynamic unless using a specific ClusterIP, so I'm not sure the value of hardcoding them here.
To dynamically update current Pod IP, I meant to introduce egress_mgr.sh, as an example.
This implementation could be replaced with operator to watch the change in Pod, instead of polling by using shell script. (This should reduce the lag.) And it might be better to define map to service, instead of mapping to pod directly, as you suggested.
(podip-vip-mappings configmap should be automatically created/updated from above tools, in a real usecase. And it won't be created manually. Sorry for making you confuse. I'll update README and delete pod_ip.yaml)
The basic idea is that kube-egress can keep simple in a way that it's role is just forwarding packets from Pod IP as packets from VIP, if such an external component keep updating mappings for Pod and Pod IP. So, the each script's role is like below:
- egress_mgr.sh: regularly read user's configuration for pod and VIP mapping, check the current Pod IP, and update the configmap for pod IP and VIP mapping.
- kube-egress.sh: regularly read configmap for pod IP and VIP mapping and update iptables and routing table in all nodes.
Of course, kube-egress.sh can do the same work to egress_mgr.sh, but it won't be necessary to be done in all nodes, as it is running as daemonset.
| name: vip-routeid-mappings | ||
| namespace: default | ||
| data: | ||
| 192.168.122.222: "64" |
There was a problem hiding this comment.
This is similar to the problem that kube-keepalived-vip is running into with encoding too much information into a string mapping, so he's considering switching to a CRD. We don't have to do a CRD, but we should combine both mappings into a single yaml-based configmap. Yaml is hard to parse in bash, but much easier for end users.
apiVersion: v1
kind: ConfigMap
metadata:
name: kube-egress
data:
mapping.yaml:
- name: "foo"
ip: "192.168.122.222"
route: 64
interface: "eth0"
interval: 60
services:
- name: "myservice"
namespace: "default"
- name: "myservice2"
namespace: "default"
- name: "bar"
ip: "192.168.122.223"
...
daemonset.yaml
Outdated
| @@ -21,14 +21,18 @@ spec: | |||
| terminationGracePeriodSeconds: 10 | |||
| containers: | |||
| - name: egress | |||
| image: ssheehy/kube-egress:0.3.0 | |||
| image: ssheehy/kube-egress:0.3.1 | |||
There was a problem hiding this comment.
Would be a minor bump: 0.4.0
|
Sorry for not responding for a long time. Also, I've fixed as you suggested in the latest commit. PTAL I hope that README files for kube-egress and egress-mapper explain enough for your review. |
|
Thanks for this @mkimuram. I'll be on vacation for a week and will dig into this once I'm back. |
|
Also, I've been working on adding Calico support. Since it's a layer 3 CNI it's a bit different on how it configures the egress. Hopefully I can make it backwards compatible so it supports both. Just FYI in case it impacts your PR. |
|
Thank you for your comment. Enjoy your vacation!
|
This PR add support for multiple egress IPs to kube-egress.
This allows users to specify mappings for pods and egress IPs, then the egress IPs for the pods will be kept as users specified. (For detailed usage, please see updated README.)
To achieve this, this PR changes below:
This is still a PoC level code, that doesn't handle all error cases. Also, how to interact between components should be improved. (egress_mgr.sh would be better to be operator that manages both kube-egress ds and keepalived-vip ds, and it should read configuration from crd that supports not only pod, but also rc, ds, etc ...) However, I would like to start discussion from this concept level, so any feedback is welcomed.