K3sContainer: expose kubeConfig for docker network access

[johnathana]

The getKubeConfigYaml method has been extended to expose the kubeConfig with a k3s container network alias as server URL. This would enable Kubernetes cluster access from other containers in the docker network.

[kiview]

Thanks for providing the PR @johnathana. Besides the comment, it looks good to me, but we need someone with more k3s experience (such as @rnorth) to have another look 🙂.

[bsideup]

To avoid confusion, IMO we should rename this method to something that will tell that the returned config is for internal communication, as otherwise someone may call getKubeConfigYaml("k3s") and then expect it to work from their machine (in addition to in-network usage)

[rnorth]

Yeah, my main thought with this PR is the risk of user confusion. This will be, I think, the first time in a module that we're outputting an address which is specifically not usable by the test host machine itself.

I think we need this method to be named to reflect that this returns a kubeconfig that is for use inside of the docker network.

We should also enhance the documentation here - both Javadocs and the md docs, so that people can understand when to use this. As @bsideup suggested, including the test into the docs would be a good way to demonstrate it.

[bsideup]

@rnorth BTW how do you feel about changing from containerIsStarted to "get file every time"? IMO getKubeConfigYaml should return pre-calculated value from containerIsStarted, while generateInternalKubeConfigYaml (or whatever the name is) can do it on demand (or maybe from the same file, just by changing the host)

[kiview]

I agree that getKubeConfigYaml being a side-effect-free simple getter would feel better and be the more intuitive API.

[rnorth]

Yeah, I think it would be nicer to keep the fetch of the kubeconfig file in containerIsStarted. It would make it clearer that the file is effectively immutable rather than something that k3s might be changing over time.

[johnathana]

I have updated the code. The documentation part is missing, I will include it on another commit.

[rnorth]

This is a nice PR - have just suggested a couple of trivial wording tweaks but am otherwise pretty happy with it.

[bsideup]

👍

outdated

[kiview]

Merged, thanks a lot for your contribution @johnathana 🙌