feat: recognize Kubernetes NodePorts and LoadBalancer #1355
Conversation
| "/etc/rancher/k3s/k3s.yaml", | ||
| "/root/.kube/config", |
There was a problem hiding this comment.
A more reliable way is to add kubernetes related config into lima YAML, but I feel it a bit exaggerated.
| type Entry struct { | ||
| Protocol Protocol | ||
| IP net.IP | ||
| Port uint16 | ||
| } |
There was a problem hiding this comment.
I feel the need of unifying port detecting packages(prochttp, iptables, kubernetesservice) if this PR is merged. These packages should use the same Entry struct for instance.
|
I will squash commits after all reviews are done, for reviewers' convenience. |
lobshunter
requested review from
AkihiroSuda and
Nino-K
and removed request for
AkihiroSuda and
Nino-K
AkihiroSuda
requested review from
afbjorklund and
jandubois
and removed request for
Nino-K
Signed-off-by: pengyu <[email protected]>
lobshunter
force-pushed
the
k8s-service
branch
from
eeff040
to
e1ed721
Compare
|
I think this is fine as-is, but I would like to review this on Monday together with @Nino-K to make sure any remaining additional code we have in the Rancher Desktop version is only required for WSL support on Windows, and not relevant to Lima. |
| _, err := os.Stat(kubeconfig) | ||
| if err != nil { | ||
| if os.IsNotExist(err) { | ||
| continue | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("stat kubeconfig %s failed: %w", kubeconfig, err) | ||
| } | ||
|
|
||
| restConfig, err := clientcmd.BuildConfigFromFlags("", kubeconfig) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("build kubeconfig from %s failed: %w", kubeconfig, err) | ||
| } | ||
| u, err := url.Parse(restConfig.Host) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("parse kubeconfig host %s failed: %w", restConfig.Host, err) | ||
| } | ||
| if u.Hostname() != "127.0.0.1" { // might need to support IPv6 | ||
| // ensures the kubeconfig points to local k8s | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
What you have here seems fine, but, want to let you know that you can also leverage the library to do more work for you, something like:
func getClientConfig(configPath string) (*restclient.Config, error) {
loadingRules := clientcmd.ClientConfigLoadingRules{
ExplicitPath: configPath,
}
clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(&loadingRules, nil)
config, err := clientConfig.ClientConfig()
if err != nil {
return nil, fmt.Errorf("could not load Kubernetes client config from %s: %w", configPath, err)
}
return config, nil
}
There was a problem hiding this comment.
Nice one, I wasn't aware of this approach. Will update the code later today.
There was a problem hiding this comment.
Hmmm, after some trying I would like to stay at current implementation. Because
using Precedence makes error message kind of confusing
loadingRules := clientcmd.ClientConfigLoadingRules{
Precedence: []string{
"/etc/rancher/k3s/k3s.yaml",
"/root/.kube/config",
},
}using ExplicitPath needs the same amount of code, and it would rely on client-go to return os.ErrNotExist if configPath is missing.
loadingRules := clientcmd.ClientConfigLoadingRules{
ExplicitPath: configPath,
}
This PR closes #1021, recognizes kubernetes NodePorts and LoadBalancer for port-forwarding. Based on the idea of #1021 (comment)