fix initialize from kube pods flow #1194
Conversation
rbtr
force-pushed
the
cns/fix/init-from-pods
branch
from
53cc280 to
e42a739
Compare
| } | ||
| }() | ||
|
|
||
| clientset, err := kubernetes.NewForConfig(kubeConfig) |
There was a problem hiding this comment.
I still suggest to move this logic inside initCNS. It is more contained there as part of CNS initialization workflow.
There was a problem hiding this comment.
podInfoByProvider is kind of an internal state which we are passing to InitCNS func.
There was a problem hiding this comment.
moving the clientset/the rest of this initialization into the initCNS breaks IOC - we should provide that func all of the objects it needs as inputs. passing it ready-to-use implementations of the interfaces it needs is the better pattern
rbtr
force-pushed
the
cns/fix/init-from-pods
branch
from
f415b48 to
62b2b34
Compare
| } else { | ||
| logger.Printf("Initializing from Kubernetes") | ||
| podInfoByIPProvider = cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { | ||
| pods, err := clientset.CoreV1().Pods("").List(ctx, metav1.ListOptions{ //nolint:govet // ignore err shadow |
There was a problem hiding this comment.
I think I'm okay with the contents of this pr, but I do want to mention that hostnetworking pods on windows do require a hacky kubeconfig to be constructed externally and loaded via file as opposed to using an inclusterconfig, if running in hostnw on windows becomes a thing
There was a problem hiding this comment.
good to know, we would need to update how we source the kubeconfigs in a couple of places for that
Signed-off-by: Evan Baker <rbtr@users.noreply.github.
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
rbtr
force-pushed
the
cns/fix/init-from-pods
branch
from
62b2b34 to
673a4ee
Compare
3 participants
Signed-off-by: Evan Baker rbtr@users.noreply.github.
Reason for Change:
Issue Fixed:
Requirements:
Notes: