Add cluster context to config resource to allow multi-cluster setup#66
Add cluster context to config resource to allow multi-cluster setup#66
Conversation
| log.Errorf("Failed to get ConfigMap object") | ||
| return | ||
| } | ||
| func (c *ConfigMapWatcher) add(cluster string) func(obj interface{}) { |
There was a problem hiding this comment.
without knowing much about the code base and spending some time checking it, why are we now returning a func?
There was a problem hiding this comment.
add was used as the event handler directly further above. Since I needed to customize it, I generate the function dynamically via this function now. I could rename it to addFuncFactory or something like that if that's more clear.
There was a problem hiding this comment.
it was mainly to learn here and understand what I'm approving 😄
I think your suggestion to rename the function make it easier to understand, I would rename the 3 functions to mention it's a factory.
There was a problem hiding this comment.
I changed it to use a separate struct for the event handler (parameterized by the cluster) so the functions are less obscure. How about this?
| clients := map[string]kubernetes.Interface{} | ||
| for _, master := range cfg.Masters { | ||
| clients = append(clients, newKubeClient(cfg, master, kubeconfig)) | ||
| clients[master] = newKubeClient(cfg, master, kubeconfig) |
There was a problem hiding this comment.
what exactly is master? is it the URL for the API?
There was a problem hiding this comment.
Yes, plain URL for the master endpoint, like https://kube-1...
There was a problem hiding this comment.
would not be better we call it apiServerURL or something similar?
There was a problem hiding this comment.
Sure, but that's existing code (including the --master) flag. I don't really want to change that here.
linki
force-pushed
the
cluster-context
branch
from
6d4df4c to
8904006
Compare
2 participants
Attempt to implement https://github.com/zalando-build/cloud-platform/issues/7743#issuecomment-3843594776
Since it's using the plain struct as the key to the map, just adding another field holding the cluster should create two entries if the same egress config is used in two different clusters:
kube-static-egress-controller/controller/controller.go
Line 36 in 99985a7
This way removing one config in one cluster will leave another entry in the cache which will lead to the rule not being deleted from the CF stack prematurely.
The Cluster field can be anything that identifies the cluster. Previously I used the cluster alias but I figured that using the master URL is just as fine and we can keep the CLI semantic unchanged (i.e. it's backwards compatible).