WIP: Replace configmap with CRD #85
Conversation
|
@steven-sheehy @mshaverdo ping. Any feedback or suggestion/feature is welcome :) |
| ) | ||
|
|
||
| // VirtualIP is a keepalived CRD specificiation for virtual IP addresses | ||
| type VirtualIP struct { |
There was a problem hiding this comment.
Is the intent with this CRD that every IP + interface -> Service:port mapping would have a separate definition? That would require quite a bit duplication by the user if they had multiple services on one IP. What about a way to select which services apply by labels?
There was a problem hiding this comment.
Is the intent with this CRD that every IP + interface -> Service:port mapping would have a separate definition?
I changed the definition to a list of serviceReferences
There was a problem hiding this comment.
What about instead of binding the virtual IP to services in a centralized spot in the CRD, we decentralize it to be an annotation on the service? The annotation would be reference back the CRD's metadata.name. For example:
apiVersion: v1
kind: Service
metadata:
name: myservice
annotations:
keepalived/vip: myvipname
spec:
externalIPs:
- 10.0.22.170There was a problem hiding this comment.
It'd be even nicer if it could populate the externalIPs on the service object. MetalLB does something similar to what I'm suggesting using annotations and modifying service IP:
There was a problem hiding this comment.
I am aware of the externalIPs field but using that also makes validation harder (I am thinking to add a webhook to avoid invalid configuration/duplicated IPs). Also using the service could lead to someone ask for externalTrafficPolicy support
There was a problem hiding this comment.
I think people should be aware that externalTrafficPolicy does not apply to externalIPs. We can document it in the README if need be or point to kubernetes docs.
If populating externalIPs is difficult/error prone, what do you think about just the annotation idea to do service binding?
| LVSScheduler *LVSScheduler `json:"lvsScheduler,omitempty"` | ||
| // LVSMethod default LVS forwarding method (NAT|DR) | ||
| // Default: NAT | ||
| LVSMethod *string `json:"lvsMethod,omitempty"` |
There was a problem hiding this comment.
I think this one also should be an enum.
| // LVSScheduler LVS scheduler (rr|wrr|lc|wlc|lblc|sh|mh|dh|fo|ovf|lblcr|sed|nq) | ||
| type LVSScheduler string | ||
|
|
||
| const ( |
There was a problem hiding this comment.
seems keepalived supports moer than 4
https://www.keepalived.org/doc/configuration_synopsis.html#virtual-server-definitions-synopsis
it says lb_algo can be rr|wrr|lc|wlc|sh|dh|lblc ?
|
Anyway, it has been more than one month, so looking forward to this CRD feature coming to be merged :-) |
|
|
||
| // Interface interface where the virtual IP should be configured | ||
| // The default value is eth0 | ||
| Interface *string `json:"interface,omitempty"` |
There was a problem hiding this comment.
May be better to detect by CIDR on machine with keepalived controller?
And then may be webhook, to check if interface exists?
|
it seems this proposal has been forgotten ? ^_^ |
Unfortunately, I don't have enough free time to continue with this. that said, this is still in my radar. |
Goals:
fixes #80