caicloud / ciao

Kernel for Kubeflow in Jupyter Notebook
Apache License 2.0
67 stars 18 forks source link

Add support for ciao using Incluster k8s config #60

Closed yeya24 closed 5 years ago

yeya24 commented 5 years ago

FEATURE REQUEST: Add support for ciao running in k8s cluster. https://github.com/caicloud/ciao/blob/ebedd2a805d50bb56c782a8ae5a662cbdc5b59d1/cmd/kubeflow-kernel/command/run.go#L54

Here we just get the config file path from yaml. Can we add support for ciao using Incluster config?

/kind feature

gaocegege commented 5 years ago

Sure, actually, we will also run the kernel as a pod in kubernetes, thus we need to support incluster config. Thanks for the issue!

gaocegege commented 5 years ago

While, when you do not input a kubeconfig, the package will use inclusterconfig automatically. Thus we do not need to change the code, maybe we could add some instructions or help message for this

gaocegege commented 5 years ago

Code here: https://github.com/caicloud/ciao/blob/ebedd2a805d50bb56c782a8ae5a662cbdc5b59d1/cmd/kubeflow-kernel/command/run.go#L57

Then the function is like:

// BuildConfigFromFlags is a helper function that builds configs from a master
// url or a kubeconfig filepath. These are passed in as command line flags for cluster
// components. Warnings should reflect this usage. If neither masterUrl or kubeconfigPath
// are passed in we fallback to inClusterConfig. If inClusterConfig fails, we fallback
// to the default config.
func BuildConfigFromFlags(masterUrl, kubeconfigPath string) (*restclient.Config, error) {
    if kubeconfigPath == "" && masterUrl == "" {
        glog.Warningf("Neither --kubeconfig nor --master was specified.  Using the inClusterConfig.  This might not work.")
        kubeconfig, err := restclient.InClusterConfig()
        if err == nil {
            return kubeconfig, nil
        }
        glog.Warning("error creating inClusterConfig, falling back to default config: ", err)
    }
    return NewNonInteractiveDeferredLoadingClientConfig(
        &ClientConfigLoadingRules{ExplicitPath: kubeconfigPath},
        &ConfigOverrides{ClusterInfo: clientcmdapi.Cluster{Server: masterUrl}}).ClientConfig()
}
yeya24 commented 5 years ago

Oh. Thanks for clarifying that.Sorry for this miss.

gaocegege commented 5 years ago

No worry, thanks for your remind, actually I also forget checking this.

gaocegege commented 5 years ago

I am closing the issue, if you are interested in the development of ciao, welcome comments and other suggestions!

/close