Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Init docker env before generate cluster config? #8883

Closed
lingsamuel opened this issue Jul 30, 2020 · 2 comments
Closed

Init docker env before generate cluster config? #8883

lingsamuel opened this issue Jul 30, 2020 · 2 comments
Labels
kind/support Categorizes issue or PR as a support question. triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@lingsamuel
Copy link
Contributor

lingsamuel commented Jul 30, 2020

The function generateClusterConfig generates a new cluster config if existing == nil.
But it uses config.DockerEnv before initialize?

https://github.com/kubernetes/minikube/blob/master/cmd/minikube/cmd/start_flags.go#L294

https://github.com/kubernetes/minikube/blob/master/cmd/minikube/cmd/start_flags.go#L355

// generateClusterConfig generate a config.ClusterConfig based on flags or existing cluster config
func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k8sVersion string, drvName string) (config.ClusterConfig, config.Node, error) {
	var cc config.ClusterConfig
	if existing != nil {
		cc = updateExistingConfigFromFlags(cmd, existing)
	} else {
                // .....
		cc = config.ClusterConfig{
                        // ....
			DockerEnv:               config.DockerEnv,
                        // ....
			},
		}
                // ......
	}

        // .....
	// Feed Docker our host proxy environment by default, so that it can pull images
	// doing this for both new config and existing, in case proxy changed since previous start
	if _, ok := r.(*cruntime.Docker); ok {
		proxy.SetDockerEnv()
	}
        // .....

If add some logs to test TestGenerateCfgFromFlagsHTTPProxyHandling in cmd/minikube/cmd/start_test.go, the generated config has wrong docker env (I think the test is wrong by the way).

@priyawadhwa priyawadhwa added the kind/support Categorizes issue or PR as a support question. label Aug 12, 2020
@priyawadhwa
Copy link

Hey @lingsamuel so it seems like the default value for config.DockerEnv is nil:

startCmd.Flags().StringArrayVar(&config.DockerEnv, "docker-env", nil, "Environment variables to pass to the Docker daemon. (format: key=value)")

which seems correct to me for a brand new cluster. Do you still think there's a bug?

@priyawadhwa priyawadhwa added the triage/needs-information Indicates an issue needs more information in order to work on it. label Aug 12, 2020
@lingsamuel
Copy link
Contributor Author

My fault, I forget to close this. The problem I encountered is a bug in the test, which I already fixed it in #8885

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/support Categorizes issue or PR as a support question. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

No branches or pull requests

2 participants