Merge pull request #49018 from luxas/kubeadm_cleanup_kubernetesdir

Automatic merge from submit-queue (batch tested with PRs 49019, 48919, 49040, 49018, 48874)

kubeadm: Remove the old KubernetesDir envparam

**What this PR does / why we need it**:

The last piece of https://github.com/kubernetes/kubernetes/issues/48053 for moving the env params into the API, and the KubernetesDir into a constant.

After this pretty mechanical change, we might offer short-hand functions in constants like `GetStaticPodManifestDir()` etc easily in order to centralize the `filepath.Join` logic into one place.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
fixes https://github.com/kubernetes/kubeadm/issues/326

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
cc @kubernetes/sig-cluster-lifecycle-pr-reviews @fabriziopandini

cmd/kubeadm/app/apis/kubeadm/BUILD

@@ -11,7 +11,6 @@ go_library(
     name = "go_default_library",
     srcs = [
         "doc.go",
-        "env.go",
         "register.go",
         "types.go",
     ],

cmd/kubeadm/app/apis/kubeadm/env.go

@@ -1,43 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package kubeadm
-
-import (
-	"fmt"
-	"os"
-	"path"
-	"strings"
-)
-
-var GlobalEnvParams = SetEnvParams()
-
-func SetEnvParams() *EnvParams {
-
-	envParams := map[string]string{
-		"kubernetes_dir": "/etc/kubernetes",
-	}
-
-	for k := range envParams {
-		if v := strings.TrimSpace(os.Getenv(fmt.Sprintf("KUBE_%s", strings.ToUpper(k)))); v != "" {
-			envParams[k] = v
-		}
-	}
-
-	return &EnvParams{
-		KubernetesDir: path.Clean(envParams["kubernetes_dir"]),
-	}
-}

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -22,10 +22,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-type EnvParams struct {
-	KubernetesDir string
-}
-
 type MasterConfiguration struct {
 	metav1.TypeMeta
 

cmd/kubeadm/app/cmd/init.go

@@ -230,7 +230,7 @@ func (i *Init) Run(out io.Writer) error {
 	// PHASE 2: Generate kubeconfig files for the admin and the kubelet
 
 	masterEndpoint := fmt.Sprintf("https://%s:%d", i.cfg.API.AdvertiseAddress, i.cfg.API.BindPort)
-	err = kubeconfigphase.CreateInitKubeConfigFiles(masterEndpoint, i.cfg.CertificatesDir, kubeadmapi.GlobalEnvParams.KubernetesDir, i.cfg.NodeName)
+	err = kubeconfigphase.CreateInitKubeConfigFiles(masterEndpoint, i.cfg.CertificatesDir, kubeadmconstants.KubernetesDir, i.cfg.NodeName)
 	if err != nil {
 		return err
 	}
@@ -240,7 +240,7 @@ func (i *Init) Run(out io.Writer) error {
 		return err
 	}
 
-	adminKubeConfigPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AdminKubeConfigFileName)
+	adminKubeConfigPath := filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.AdminKubeConfigFileName)
 	client, err := kubeadmutil.CreateClientAndWaitForAPI(adminKubeConfigPath)
 	if err != nil {
 		return err
@@ -297,7 +297,7 @@ func (i *Init) Run(out io.Writer) error {
 	}
 
 	ctx := map[string]string{
-		"KubeConfigPath": filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AdminKubeConfigFileName),
+		"KubeConfigPath": filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.AdminKubeConfigFileName),
 		"KubeConfigName": kubeadmconstants.AdminKubeConfigFileName,
 		"Token":          i.cfg.Token,
 		"MasterIP":       i.cfg.API.AdvertiseAddress,

cmd/kubeadm/app/cmd/join.go

@@ -191,7 +191,7 @@ func (j *Join) Run(out io.Writer) error {
 		return err
 	}
 
-	kubeconfigFile := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.KubeletKubeConfigFileName)
+	kubeconfigFile := filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.KubeletKubeConfigFileName)
 	if err := kubeconfigutil.WriteToDisk(kubeconfigFile, cfg); err != nil {
 		return err
 	}

cmd/kubeadm/app/cmd/reset.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/spf13/cobra"
 
-	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
@@ -118,7 +117,7 @@ func (r *Reset) Run(out io.Writer) error {
 
 	// Only clear etcd data when the etcd manifest is found. In case it is not found, we must assume that the user
 	// provided external etcd endpoints. In that case, it is his own responsibility to reset etcd
-	etcdManifestPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ManifestsSubDirName, "etcd.yaml")
+	etcdManifestPath := filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName, "etcd.yaml")
 	if _, err := os.Stat(etcdManifestPath); err == nil {
 		dirsToClean = append(dirsToClean, "/var/lib/etcd")
 	} else {
@@ -132,7 +131,7 @@ func (r *Reset) Run(out io.Writer) error {
 	}
 
 	// Remove contents from the config and pki directories
-	resetConfigDir(kubeadmapi.GlobalEnvParams.KubernetesDir, r.certsDir)
+	resetConfigDir(kubeadmconstants.KubernetesDir, r.certsDir)
 
 	return nil
 }

cmd/kubeadm/app/constants/constants.go

@@ -24,10 +24,11 @@ import (
 	"k8s.io/kubernetes/pkg/util/version"
 )
 
-const (
+// KubernetesDir is the directory kubernetes owns for storing various configuration files
-	// KubernetesDir is the directory kubernetes owns for storing various configuration files
+// This semi-constant MUST NOT be modified during runtime. It's a variable solely for use in unit testing.
-	KubernetesDir = "/etc/kubernetes"
+var KubernetesDir = "/etc/kubernetes"
 
+const (
 	ManifestsSubDirName = "manifests"
 
 	CACertAndKeyBaseName = "ca"

cmd/kubeadm/app/phases/controlplane/manifests.go

@@ -129,7 +129,7 @@ func WriteStaticPodManifests(cfg *kubeadmapi.MasterConfiguration) error {
 		staticPodSpecs[etcd] = etcdPod
 	}
 
-	manifestsPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ManifestsSubDirName)
+	manifestsPath := filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName)
 	if err := os.MkdirAll(manifestsPath, 0700); err != nil {
 		return fmt.Errorf("failed to create directory %q [%v]", manifestsPath, err)
 	}
@@ -233,7 +233,7 @@ func k8sVolume() v1.Volume {
 	return v1.Volume{
 		Name: "k8s",
 		VolumeSource: v1.VolumeSource{
-			HostPath: &v1.HostPathVolumeSource{Path: kubeadmapi.GlobalEnvParams.KubernetesDir},
+			HostPath: &v1.HostPathVolumeSource{Path: kubeadmconstants.KubernetesDir},
 		},
 	}
 }
@@ -241,7 +241,7 @@ func k8sVolume() v1.Volume {
 func k8sVolumeMount() v1.VolumeMount {
 	return v1.VolumeMount{
 		Name:      "k8s",
-		MountPath: kubeadmapi.GlobalEnvParams.KubernetesDir,
+		MountPath: kubeadmconstants.KubernetesDir,
 		ReadOnly:  true,
 	}
 }
@@ -371,7 +371,7 @@ func getControllerManagerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted
 	defaultArguments := map[string]string{
 		"address":                          "127.0.0.1",
 		"leader-elect":                     "true",
-		"kubeconfig":                       filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ControllerManagerKubeConfigFileName),
+		"kubeconfig":                       filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ControllerManagerKubeConfigFileName),
 		"root-ca-file":                     filepath.Join(cfg.CertificatesDir, kubeadmconstants.CACertName),
 		"service-account-private-key-file": filepath.Join(cfg.CertificatesDir, kubeadmconstants.ServiceAccountPrivateKeyName),
 		"cluster-signing-cert-file":        filepath.Join(cfg.CertificatesDir, kubeadmconstants.CACertName),
@@ -404,7 +404,7 @@ func getSchedulerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted bool) [
 	defaultArguments := map[string]string{
 		"address":      "127.0.0.1",
 		"leader-elect": "true",
-		"kubeconfig":   filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.SchedulerKubeConfigFileName),
+		"kubeconfig":   filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.SchedulerKubeConfigFileName),
 	}
 
 	command := []string{"kube-scheduler"}

cmd/kubeadm/app/phases/controlplane/manifests_test.go

@@ -46,10 +46,8 @@ func TestWriteStaticPodManifests(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	// set up tmp GlobalEnvParams values for testing
+	// set up tmp KubernetesDir for testing
-	oldEnv := kubeadmapi.GlobalEnvParams
+	kubeadmconstants.KubernetesDir = fmt.Sprintf("%s/etc/kubernetes", tmpdir)
-	kubeadmapi.GlobalEnvParams.KubernetesDir = fmt.Sprintf("%s/etc/kubernetes", tmpdir)
-	defer func() { kubeadmapi.GlobalEnvParams = oldEnv }()
 
 	var tests = []struct {
 		cfg                  *kubeadmapi.MasterConfiguration
@@ -86,7 +84,7 @@ func TestWriteStaticPodManifests(t *testing.T) {
 
 		// Below is dead code.
 		if rt.expectedAPIProbePort != 0 {
-			manifest, err := os.Open(filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ManifestsSubDirName, "kube-apiserver.yaml"))
+			manifest, err := os.Open(filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName, "kube-apiserver.yaml"))
 			if err != nil {
 				t.Errorf("WriteStaticPodManifests: %v", err)
 				continue
@@ -340,7 +338,7 @@ func TestK8sVolume(t *testing.T) {
 				Name: "k8s",
 				VolumeSource: v1.VolumeSource{
 					HostPath: &v1.HostPathVolumeSource{
-						Path: kubeadmapi.GlobalEnvParams.KubernetesDir},
+						Path: kubeadmconstants.KubernetesDir},
 				}},
 		},
 	}
@@ -371,7 +369,7 @@ func TestK8sVolumeMount(t *testing.T) {
 		{
 			expected: v1.VolumeMount{
 				Name:      "k8s",
-				MountPath: kubeadmapi.GlobalEnvParams.KubernetesDir,
+				MountPath: kubeadmconstants.KubernetesDir,
 				ReadOnly:  true,
 			},
 		},
@@ -688,7 +686,7 @@ func TestGetControllerManagerCommand(t *testing.T) {
 				"kube-controller-manager",
 				"--address=127.0.0.1",
 				"--leader-elect=true",
-				"--kubeconfig=" + kubeadmapi.GlobalEnvParams.KubernetesDir + "/controller-manager.conf",
+				"--kubeconfig=" + kubeadmconstants.KubernetesDir + "/controller-manager.conf",
 				"--root-ca-file=" + testCertsDir + "/ca.crt",
 				"--service-account-private-key-file=" + testCertsDir + "/sa.key",
 				"--cluster-signing-cert-file=" + testCertsDir + "/ca.crt",
@@ -707,7 +705,7 @@ func TestGetControllerManagerCommand(t *testing.T) {
 				"kube-controller-manager",
 				"--address=127.0.0.1",
 				"--leader-elect=true",
-				"--kubeconfig=" + kubeadmapi.GlobalEnvParams.KubernetesDir + "/controller-manager.conf",
+				"--kubeconfig=" + kubeadmconstants.KubernetesDir + "/controller-manager.conf",
 				"--root-ca-file=" + testCertsDir + "/ca.crt",
 				"--service-account-private-key-file=" + testCertsDir + "/sa.key",
 				"--cluster-signing-cert-file=" + testCertsDir + "/ca.crt",
@@ -727,7 +725,7 @@ func TestGetControllerManagerCommand(t *testing.T) {
 				"kube-controller-manager",
 				"--address=127.0.0.1",
 				"--leader-elect=true",
-				"--kubeconfig=" + kubeadmapi.GlobalEnvParams.KubernetesDir + "/controller-manager.conf",
+				"--kubeconfig=" + kubeadmconstants.KubernetesDir + "/controller-manager.conf",
 				"--root-ca-file=" + testCertsDir + "/ca.crt",
 				"--service-account-private-key-file=" + testCertsDir + "/sa.key",
 				"--cluster-signing-cert-file=" + testCertsDir + "/ca.crt",
@@ -817,7 +815,7 @@ func TestGetSchedulerCommand(t *testing.T) {
 				"kube-scheduler",
 				"--address=127.0.0.1",
 				"--leader-elect=true",
-				"--kubeconfig=" + kubeadmapi.GlobalEnvParams.KubernetesDir + "/scheduler.conf",
+				"--kubeconfig=" + kubeadmconstants.KubernetesDir + "/scheduler.conf",
 			},
 		},
 	}

cmd/kubeadm/app/phases/selfhosting/selfhosting_volumes.go

@@ -37,7 +37,7 @@ type tlsKeyPair struct {
 func k8sSelfHostedVolumeMount() v1.VolumeMount {
 	return v1.VolumeMount{
 		Name:      "k8s",
-		MountPath: kubeadmapi.GlobalEnvParams.KubernetesDir,
+		MountPath: kubeadmconstants.KubernetesDir,
 		ReadOnly:  true,
 	}
 }
@@ -248,7 +248,7 @@ func createOpaqueSecrets(cfg *kubeadmapi.MasterConfiguration, client *clientset.
 	for _, file := range files {
 		secret, err := createOpaqueSecretFromFile(
 			file,
-			path.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, file),
+			path.Join(kubeadmconstants.KubernetesDir, file),
 		)
 		if err != nil {
 			return err

cmd/kubeadm/app/preflight/checks.go

@@ -545,7 +545,7 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 		PortOpenCheck{port: 10251},
 		PortOpenCheck{port: 10252},
 		HTTPProxyCheck{Proto: "https", Host: cfg.API.AdvertiseAddress, Port: int(cfg.API.BindPort)},
-		DirAvailableCheck{Path: filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},
+		DirAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},
 		DirAvailableCheck{Path: "/var/lib/kubelet"},
 		FileContentCheck{Path: bridgenf, Content: []byte{'1'}},
 		InPathCheck{executable: "ip", mandatory: true},
@@ -603,10 +603,10 @@ func RunJoinNodeChecks(cfg *kubeadmapi.NodeConfiguration) error {
 		ServiceCheck{Service: "kubelet", CheckIfActive: false},
 		ServiceCheck{Service: "docker", CheckIfActive: true},
 		PortOpenCheck{port: 10250},
-		DirAvailableCheck{Path: filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},
+		DirAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},
 		DirAvailableCheck{Path: "/var/lib/kubelet"},
 		FileAvailableCheck{Path: cfg.CACertPath},
-		FileAvailableCheck{Path: filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.KubeletKubeConfigFileName)},
+		FileAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.KubeletKubeConfigFileName)},
 		FileContentCheck{Path: bridgenf, Content: []byte{'1'}},
 		InPathCheck{executable: "ip", mandatory: true},
 		InPathCheck{executable: "iptables", mandatory: true},

cmd/kubeadm/app/util/kubeconfig/BUILD

@@ -13,7 +13,6 @@ go_test(
     srcs = ["kubeconfig_test.go"],
     library = ":go_default_library",
     tags = ["automanaged"],
-    deps = ["//cmd/kubeadm/app/apis/kubeadm:go_default_library"],
 )
 
 go_library(

cmd/kubeadm/app/util/kubeconfig/kubeconfig_test.go

@@ -21,10 +21,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path/filepath"
 	"testing"
-
-	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
 
 const (
@@ -144,12 +141,6 @@ func TestWriteKubeconfigToDisk(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	// set up tmp GlobalEnvParams values for testing
-	oldEnv := kubeadmapi.GlobalEnvParams
-	kubeadmapi.GlobalEnvParams = kubeadmapi.SetEnvParams()
-	kubeadmapi.GlobalEnvParams.KubernetesDir = fmt.Sprintf("%s/etc/kubernetes", tmpdir)
-	defer func() { kubeadmapi.GlobalEnvParams = oldEnv }()
-
 	var writeConfig = []struct {
 		name        string
 		cc          configClient
@@ -168,7 +159,7 @@ func TestWriteKubeconfigToDisk(t *testing.T) {
 			rt.cc.caCert,
 			rt.ccWithToken.token,
 		)
-		configPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, fmt.Sprintf("%s.conf", rt.name))
+		configPath := fmt.Sprintf("%s/etc/kubernetes/%s.conf", tmpdir, rt.name)
 		err := WriteToDisk(configPath, c)
 		if err != rt.expected {
 			t.Errorf(