From 51197e4393b10d9d77b636eea98953e1611b5a86 Mon Sep 17 00:00:00 2001 From: "Rostislav M. Georgiev" Date: Fri, 7 Dec 2018 12:15:51 +0200 Subject: [PATCH] kubeadm: Refactor InitConfiguration init APIs Currently ConfigFileAndDefaultsToInternalConfig and FetchConfigFromFileOrCluster are used to default and load InitConfiguration from file or cluster. These two APIs do a couple of completely separate things depending on how they were invoked. In the case of ConfigFileAndDefaultsToInternalConfig, an InitConfiguration could be either defaulted with external override parameters, or loaded from file. With FetchConfigFromFileOrCluster an InitConfiguration is either loaded from file or from the config map in the cluster. The two share both some functionality, but not enough code. They are also quite difficult to use and sometimes even error prone. To solve the issues, the following steps were taken: - Introduce DefaultedInitConfiguration which returns defaulted version agnostic InitConfiguration. The function takes InitConfiguration for overriding the defaults. - Introduce LoadInitConfigurationFromFile, which loads, converts, validates and defaults an InitConfiguration from file. - Introduce FetchInitConfigurationFromCluster that fetches InitConfiguration from the config map. - Reduce, when possible, the usage of ConfigFileAndDefaultsToInternalConfig by replacing it with DefaultedInitConfiguration or LoadInitConfigurationFromFile invocations. - Replace all usages of FetchConfigFromFileOrCluster with calls to LoadInitConfigurationFromFile or FetchInitConfigurationFromCluster. - Delete FetchConfigFromFileOrCluster as it's no longer used. - Rename ConfigFileAndDefaultsToInternalConfig to LoadOrDefaultInitConfiguration in order to better describe what the function is actually doing. Signed-off-by: Rostislav M. Georgiev --- cmd/kubeadm/app/cmd/alpha/certs.go | 2 +- cmd/kubeadm/app/cmd/alpha/kubeconfig.go | 4 +- cmd/kubeadm/app/cmd/alpha/selfhosting.go | 4 +- cmd/kubeadm/app/cmd/config.go | 47 +++++------ cmd/kubeadm/app/cmd/config_test.go | 2 +- cmd/kubeadm/app/cmd/init.go | 2 +- cmd/kubeadm/app/cmd/join.go | 2 +- cmd/kubeadm/app/cmd/reset.go | 4 +- cmd/kubeadm/app/cmd/token.go | 4 +- cmd/kubeadm/app/cmd/token_test.go | 2 +- cmd/kubeadm/app/cmd/upgrade/BUILD | 1 - cmd/kubeadm/app/cmd/upgrade/apply.go | 4 +- cmd/kubeadm/app/cmd/upgrade/common.go | 9 +- cmd/kubeadm/app/cmd/upgrade/diff.go | 4 +- cmd/kubeadm/app/cmd/upgrade/node.go | 2 +- cmd/kubeadm/app/cmd/upgrade/plan.go | 4 +- .../app/phases/addons/proxy/proxy_test.go | 2 +- .../phases/uploadconfig/uploadconfig_test.go | 2 +- cmd/kubeadm/app/util/config/cluster.go | 50 +++-------- cmd/kubeadm/app/util/config/cluster_test.go | 71 ---------------- cmd/kubeadm/app/util/config/common.go | 2 +- .../app/util/config/initconfiguration.go | 67 +++++++++------ .../app/util/config/initconfiguration_test.go | 82 ++++++++++++++++++- cmd/kubeadm/test/util.go | 2 +- 24 files changed, 180 insertions(+), 195 deletions(-) diff --git a/cmd/kubeadm/app/cmd/alpha/certs.go b/cmd/kubeadm/app/cmd/alpha/certs.go index dfba33209d1..0c6b65ed47a 100644 --- a/cmd/kubeadm/app/cmd/alpha/certs.go +++ b/cmd/kubeadm/app/cmd/alpha/certs.go @@ -135,7 +135,7 @@ func addFlags(cmd *cobra.Command, cfg *renewConfig) { func generateRenewalFunction(cert *certsphase.KubeadmCert, caCert *certsphase.KubeadmCert, cfg *renewConfig) func() { return func() { - internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(cfg.cfgPath, &cfg.cfg) + internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfg.cfgPath, &cfg.cfg) kubeadmutil.CheckErr(err) if cfg.useCSR { diff --git a/cmd/kubeadm/app/cmd/alpha/kubeconfig.go b/cmd/kubeadm/app/cmd/alpha/kubeconfig.go index 0c25c368a23..6d28a2d5387 100644 --- a/cmd/kubeadm/app/cmd/alpha/kubeconfig.go +++ b/cmd/kubeadm/app/cmd/alpha/kubeconfig.go @@ -79,8 +79,8 @@ func newCmdUserKubeConfig(out io.Writer) *cobra.Command { kubeadmutil.CheckErr(errors.New("missing required argument --client-name")) } - // This call returns the ready-to-use configuration based on the configuration file that might or might not exist and the default cfg populated by flags - internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig("", cfg) + // This call returns the ready-to-use configuration based on the default cfg populated by flags + internalcfg, err := configutil.DefaultedInitConfiguration(cfg) kubeadmutil.CheckErr(err) // if the kubeconfig file for an additional user has to use a token, use it diff --git a/cmd/kubeadm/app/cmd/alpha/selfhosting.go b/cmd/kubeadm/app/cmd/alpha/selfhosting.go index 31329bb4cb6..26ba71a42e8 100644 --- a/cmd/kubeadm/app/cmd/alpha/selfhosting.go +++ b/cmd/kubeadm/app/cmd/alpha/selfhosting.go @@ -124,11 +124,11 @@ func getSelfhostingSubCommand(in io.Reader) *cobra.Command { kubeadmutil.CheckErr(err) // KubernetesVersion is not used, but we set it explicitly to avoid the lookup - // of the version from the internet when executing ConfigFileAndDefaultsToInternalConfig + // of the version from the internet when executing LoadOrDefaultInitConfiguration phases.SetKubernetesVersion(&cfg.ClusterConfiguration) // This call returns the ready-to-use configuration based on the configuration file that might or might not exist and the default cfg populated by flags - internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(cfgPath, cfg) + internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, cfg) kubeadmutil.CheckErr(err) // Converts the Static Pod-hosted control plane into a self-hosted one diff --git a/cmd/kubeadm/app/cmd/config.go b/cmd/kubeadm/app/cmd/config.go index 2e583a097c3..d747eea9032 100644 --- a/cmd/kubeadm/app/cmd/config.go +++ b/cmd/kubeadm/app/cmd/config.go @@ -178,7 +178,7 @@ func getSupportedComponentConfigAPIObjects() []string { } func getDefaultedInitConfig() (*kubeadmapi.InitConfiguration, error) { - return configutil.ConfigFileAndDefaultsToInternalConfig("", &kubeadmapiv1beta1.InitConfiguration{ + return configutil.DefaultedInitConfiguration(&kubeadmapiv1beta1.InitConfiguration{ // TODO: Probably move to getDefaultedClusterConfig? LocalAPIEndpoint: kubeadmapiv1beta1.APIEndpoint{AdvertiseAddress: "1.2.3.4"}, ClusterConfiguration: kubeadmapiv1beta1.ClusterConfiguration{ @@ -320,12 +320,13 @@ func NewCmdConfigUploadFromFile(out io.Writer, kubeConfigFile *string) *cobra.Co client, err := kubeconfigutil.ClientSetFromFile(*kubeConfigFile) kubeadmutil.CheckErr(err) - // The default configuration is empty; everything should come from the file on disk - klog.V(1).Infoln("[config] creating empty default configuration") - defaultcfg := &kubeadmapiv1beta1.InitConfiguration{} - // Upload the configuration using the file; don't care about the defaultcfg really + // Default both statically and dynamically, convert to internal API type, and validate everything + internalcfg, err := configutil.LoadInitConfigurationFromFile(cfgPath) + kubeadmutil.CheckErr(err) + + // Upload the configuration using the file klog.V(1).Infof("[config] uploading configuration") - err = uploadConfiguration(client, cfgPath, defaultcfg) + err = uploadconfig.UploadConfiguration(internalcfg, client) kubeadmutil.CheckErr(err) }, } @@ -360,10 +361,18 @@ func NewCmdConfigUploadFromFlags(out io.Writer, kubeConfigFile *string) *cobra.C client, err := kubeconfigutil.ClientSetFromFile(*kubeConfigFile) kubeadmutil.CheckErr(err) + // KubernetesVersion is not used, but we set it explicitly to avoid the lookup + // of the version from the internet when executing DefaultedInitConfiguration + phaseutil.SetKubernetesVersion(&cfg.ClusterConfiguration) + // Default both statically and dynamically, convert to internal API type, and validate everything - // The cfgPath argument is unset here as we shouldn't load a config file from disk, just go with cfg + klog.V(1).Infoln("[config] converting to internal API type") + internalcfg, err := configutil.DefaultedInitConfiguration(cfg) + kubeadmutil.CheckErr(err) + + // Finally, upload the configuration klog.V(1).Infof("[config] uploading configuration") - err = uploadConfiguration(client, "", cfg) + err = uploadconfig.UploadConfiguration(internalcfg, client) kubeadmutil.CheckErr(err) }, } @@ -384,24 +393,6 @@ func RunConfigView(out io.Writer, client clientset.Interface) error { return nil } -// uploadConfiguration handles the uploading of the configuration internally -func uploadConfiguration(client clientset.Interface, cfgPath string, defaultcfg *kubeadmapiv1beta1.InitConfiguration) error { - // KubernetesVersion is not used, but we set it explicitly to avoid the lookup - // of the version from the internet when executing ConfigFileAndDefaultsToInternalConfig - phaseutil.SetKubernetesVersion(&defaultcfg.ClusterConfiguration) - - // Default both statically and dynamically, convert to internal API type, and validate everything - // First argument is unset here as we shouldn't load a config file from disk - klog.V(1).Infoln("[config] converting to internal API type") - internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(cfgPath, defaultcfg) - if err != nil { - return err - } - - // Then just call the uploadconfig phase to do the rest of the work - return uploadconfig.UploadConfiguration(internalcfg, client) -} - // NewCmdConfigImages returns the "kubeadm config images" command func NewCmdConfigImages(out io.Writer) *cobra.Command { cmd := &cobra.Command{ @@ -427,7 +418,7 @@ func NewCmdConfigImagesPull() *cobra.Command { Run: func(_ *cobra.Command, _ []string) { externalcfg.ClusterConfiguration.FeatureGates, err = features.NewFeatureGate(&features.InitFeatureGates, featureGatesString) kubeadmutil.CheckErr(err) - internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(cfgPath, externalcfg) + internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, externalcfg) kubeadmutil.CheckErr(err) containerRuntime, err := utilruntime.NewContainerRuntime(utilsexec.New(), internalcfg.GetCRISocket()) kubeadmutil.CheckErr(err) @@ -496,7 +487,7 @@ func NewCmdConfigImagesList(out io.Writer, mockK8sVersion *string) *cobra.Comman // NewImagesList returns the underlying struct for the "kubeadm config images list" command func NewImagesList(cfgPath string, cfg *kubeadmapiv1beta1.InitConfiguration) (*ImagesList, error) { - initcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(cfgPath, cfg) + initcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, cfg) if err != nil { return nil, errors.Wrap(err, "could not convert cfg to an internal cfg") } diff --git a/cmd/kubeadm/app/cmd/config_test.go b/cmd/kubeadm/app/cmd/config_test.go index 5fb5777c30a..7445b982f58 100644 --- a/cmd/kubeadm/app/cmd/config_test.go +++ b/cmd/kubeadm/app/cmd/config_test.go @@ -275,7 +275,7 @@ func TestMigrate(t *testing.T) { t.Fatalf("failed to set new-config flag") } command.Run(nil, nil) - if _, err := configutil.ConfigFileAndDefaultsToInternalConfig(newConfigPath, &kubeadmapiv1beta1.InitConfiguration{}); err != nil { + if _, err := configutil.LoadInitConfigurationFromFile(newConfigPath); err != nil { t.Fatalf("Could not read output back into internal type: %v", err) } } diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index b73df6179c5..70c0b8f07d7 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -302,7 +302,7 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io // Either use the config file if specified, or convert public kubeadm API to the internal InitConfiguration // and validates InitConfiguration - cfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(options.cfgPath, options.externalcfg) + cfg, err := configutil.LoadOrDefaultInitConfiguration(options.cfgPath, options.externalcfg) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/cmd/join.go b/cmd/kubeadm/app/cmd/join.go index 8c952a39822..30d76b94f72 100644 --- a/cmd/kubeadm/app/cmd/join.go +++ b/cmd/kubeadm/app/cmd/join.go @@ -530,7 +530,7 @@ func fetchInitConfiguration(tlsBootstrapCfg *clientcmdapi.Config) (*kubeadmapi.I } // Fetches the init configuration - initConfiguration, err := configutil.FetchConfigFromFileOrCluster(tlsClient, os.Stdout, "join", "", true) + initConfiguration, err := configutil.FetchInitConfigurationFromCluster(tlsClient, os.Stdout, "join", true) if err != nil { return nil, errors.Wrap(err, "unable to fetch the kubeadm-config ConfigMap") } diff --git a/cmd/kubeadm/app/cmd/reset.go b/cmd/kubeadm/app/cmd/reset.go index ebb780ef512..a5bfa609b8e 100644 --- a/cmd/kubeadm/app/cmd/reset.go +++ b/cmd/kubeadm/app/cmd/reset.go @@ -208,7 +208,7 @@ func getEtcdDataDir(manifestPath string, client clientset.Interface) (string, er var dataDir string if client != nil { - cfg, err := configutil.FetchConfigFromFileOrCluster(client, os.Stdout, "reset", "", false) + cfg, err := configutil.FetchInitConfigurationFromCluster(client, os.Stdout, "reset", false) if err == nil && cfg.Etcd.Local != nil { return cfg.Etcd.Local.DataDir, nil } @@ -299,7 +299,7 @@ func resetConfigDir(configPathDir, pkiPathDir string) { func resetDetectCRISocket(client clientset.Interface) (string, error) { // first try to connect to the cluster for the CRI socket - cfg, err := configutil.FetchConfigFromFileOrCluster(client, os.Stdout, "reset", "", false) + cfg, err := configutil.FetchInitConfigurationFromCluster(client, os.Stdout, "reset", false) if err == nil { return cfg.NodeRegistration.CRISocket, nil } diff --git a/cmd/kubeadm/app/cmd/token.go b/cmd/kubeadm/app/cmd/token.go index 4c3933f4de6..079e65c37a3 100644 --- a/cmd/kubeadm/app/cmd/token.go +++ b/cmd/kubeadm/app/cmd/token.go @@ -211,12 +211,12 @@ func NewCmdTokenGenerate(out io.Writer) *cobra.Command { // RunCreateToken generates a new bootstrap token and stores it as a secret on the server. func RunCreateToken(out io.Writer, client clientset.Interface, cfgPath string, cfg *kubeadmapiv1beta1.InitConfiguration, printJoinCommand bool, kubeConfigFile string) error { // KubernetesVersion is not used, but we set it explicitly to avoid the lookup - // of the version from the internet when executing ConfigFileAndDefaultsToInternalConfig + // of the version from the internet when executing LoadOrDefaultInitConfiguration phaseutil.SetKubernetesVersion(&cfg.ClusterConfiguration) // This call returns the ready-to-use configuration based on the configuration file that might or might not exist and the default cfg populated by flags klog.V(1).Infoln("[token] loading configurations") - internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(cfgPath, cfg) + internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, cfg) if err != nil { return err } diff --git a/cmd/kubeadm/app/cmd/token_test.go b/cmd/kubeadm/app/cmd/token_test.go index ba2b654f7c2..34df0574240 100644 --- a/cmd/kubeadm/app/cmd/token_test.go +++ b/cmd/kubeadm/app/cmd/token_test.go @@ -176,7 +176,7 @@ func TestRunCreateToken(t *testing.T) { cfg := &kubeadmapiv1beta1.InitConfiguration{ ClusterConfiguration: kubeadmapiv1beta1.ClusterConfiguration{ // KubernetesVersion is not used, but we set this explicitly to avoid - // the lookup of the version from the internet when executing ConfigFileAndDefaultsToInternalConfig + // the lookup of the version from the internet when executing LoadOrDefaultInitConfiguration KubernetesVersion: constants.MinimumControlPlaneVersion.String(), }, BootstrapTokens: []kubeadmapiv1beta1.BootstrapToken{ diff --git a/cmd/kubeadm/app/cmd/upgrade/BUILD b/cmd/kubeadm/app/cmd/upgrade/BUILD index fa0e144a6f8..c1b47c849a3 100644 --- a/cmd/kubeadm/app/cmd/upgrade/BUILD +++ b/cmd/kubeadm/app/cmd/upgrade/BUILD @@ -14,7 +14,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", - "//cmd/kubeadm/app/apis/kubeadm/v1beta1:go_default_library", "//cmd/kubeadm/app/apis/kubeadm/validation:go_default_library", "//cmd/kubeadm/app/cmd/options:go_default_library", "//cmd/kubeadm/app/cmd/util:go_default_library", diff --git a/cmd/kubeadm/app/cmd/upgrade/apply.go b/cmd/kubeadm/app/cmd/upgrade/apply.go index b2188aa8fbc..c724ec55476 100644 --- a/cmd/kubeadm/app/cmd/upgrade/apply.go +++ b/cmd/kubeadm/app/cmd/upgrade/apply.go @@ -27,7 +27,6 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/klog" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" - kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util" @@ -90,9 +89,8 @@ func NewCmdApply(apf *applyPlanFlags) *cobra.Command { // If the version is specified in config file, pick up that value. if flags.cfgPath != "" { - klog.V(1).Infof("fetching configuration from file %s", flags.cfgPath) // Note that cfg isn't preserved here, it's just an one-off to populate flags.newK8sVersionStr based on --config - cfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(flags.cfgPath, &kubeadmapiv1beta1.InitConfiguration{}) + cfg, err := configutil.LoadInitConfigurationFromFile(flags.cfgPath) kubeadmutil.CheckErr(err) if cfg.KubernetesVersion != "" { diff --git a/cmd/kubeadm/app/cmd/upgrade/common.go b/cmd/kubeadm/app/cmd/upgrade/common.go index e70a8c0ceaa..c448f7a1be8 100644 --- a/cmd/kubeadm/app/cmd/upgrade/common.go +++ b/cmd/kubeadm/app/cmd/upgrade/common.go @@ -71,7 +71,14 @@ func enforceRequirements(flags *applyPlanFlags, dryRun bool, newK8sVersion strin // Fetch the configuration from a file or ConfigMap and validate it fmt.Println("[upgrade/config] Making sure the configuration is correct:") - cfg, err := configutil.FetchConfigFromFileOrCluster(client, os.Stdout, "upgrade/config", flags.cfgPath, false) + + var cfg *kubeadmapi.InitConfiguration + if flags.cfgPath != "" { + cfg, err = configutil.LoadInitConfigurationFromFile(flags.cfgPath) + } else { + cfg, err = configutil.FetchInitConfigurationFromCluster(client, os.Stdout, "upgrade/config", false) + } + if err != nil { if apierrors.IsNotFound(err) { fmt.Printf("[upgrade/config] In order to upgrade, a ConfigMap called %q in the %s namespace must exist.\n", constants.KubeadmConfigConfigMap, metav1.NamespaceSystem) diff --git a/cmd/kubeadm/app/cmd/upgrade/diff.go b/cmd/kubeadm/app/cmd/upgrade/diff.go index 8c55c253352..7d3f3d14ab4 100644 --- a/cmd/kubeadm/app/cmd/upgrade/diff.go +++ b/cmd/kubeadm/app/cmd/upgrade/diff.go @@ -26,7 +26,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/version" "k8s.io/klog" - kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util" "k8s.io/kubernetes/cmd/kubeadm/app/constants" @@ -78,8 +77,7 @@ func NewCmdDiff(out io.Writer) *cobra.Command { func runDiff(flags *diffFlags, args []string) error { // If the version is specified in config file, pick up that value. - klog.V(1).Infof("fetching configuration from file %s", flags.cfgPath) - cfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(flags.cfgPath, &kubeadmapiv1beta1.InitConfiguration{}) + cfg, err := configutil.LoadInitConfigurationFromFile(flags.cfgPath) if err != nil { return err } diff --git a/cmd/kubeadm/app/cmd/upgrade/node.go b/cmd/kubeadm/app/cmd/upgrade/node.go index f7d25d735ac..34d578bf834 100644 --- a/cmd/kubeadm/app/cmd/upgrade/node.go +++ b/cmd/kubeadm/app/cmd/upgrade/node.go @@ -229,7 +229,7 @@ func RunUpgradeControlPlane(flags *controlplaneUpgradeFlags) error { waiter := apiclient.NewKubeWaiter(client, upgrade.UpgradeManifestTimeout, os.Stdout) // Fetches the cluster configuration - cfg, err := configutil.FetchConfigFromFileOrCluster(client, os.Stdout, "upgrade", "", false) + cfg, err := configutil.FetchInitConfigurationFromCluster(client, os.Stdout, "upgrade", false) if err != nil { return errors.Wrap(err, "unable to fetch the kubeadm-config ConfigMap") } diff --git a/cmd/kubeadm/app/cmd/upgrade/plan.go b/cmd/kubeadm/app/cmd/upgrade/plan.go index 1b047b5dce1..516f469c16c 100644 --- a/cmd/kubeadm/app/cmd/upgrade/plan.go +++ b/cmd/kubeadm/app/cmd/upgrade/plan.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/util/version" "k8s.io/klog" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" - kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" "k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" @@ -62,8 +61,7 @@ func NewCmdPlan(apf *applyPlanFlags) *cobra.Command { // If the version is specified in config file, pick up that value. if flags.cfgPath != "" { - klog.V(1).Infof("fetching configuration from file %s", flags.cfgPath) - cfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(flags.cfgPath, &kubeadmapiv1beta1.InitConfiguration{}) + cfg, err := configutil.LoadInitConfigurationFromFile(flags.cfgPath) kubeadmutil.CheckErr(err) if cfg.KubernetesVersion != "" { diff --git a/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go b/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go index a3d1beb42df..e87ffd803e6 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go +++ b/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go @@ -201,7 +201,7 @@ func TestEnsureProxyAddon(t *testing.T) { masterConfig.Networking.PodSubnet = "2001:101::/96" } - intMaster, err := configutil.ConfigFileAndDefaultsToInternalConfig("", masterConfig) + intMaster, err := configutil.DefaultedInitConfiguration(masterConfig) if err != nil { t.Errorf("test failed to convert external to internal version") break diff --git a/cmd/kubeadm/app/phases/uploadconfig/uploadconfig_test.go b/cmd/kubeadm/app/phases/uploadconfig/uploadconfig_test.go index c10a57376af..1d25bcdb2e2 100644 --- a/cmd/kubeadm/app/phases/uploadconfig/uploadconfig_test.go +++ b/cmd/kubeadm/app/phases/uploadconfig/uploadconfig_test.go @@ -84,7 +84,7 @@ func TestUploadConfiguration(t *testing.T) { CRISocket: "/var/run/custom-cri.sock", }, } - cfg, err := configutil.ConfigFileAndDefaultsToInternalConfig("", initialcfg) + cfg, err := configutil.DefaultedInitConfiguration(initialcfg) // cleans up component config to make cfg and decodedcfg comparable (now component config are not stored anymore in kubeadm-config config map) cfg.ComponentConfigs = kubeadmapi.ComponentConfigs{} diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go index 22147cf7eec..5ff954e1088 100644 --- a/cmd/kubeadm/app/util/config/cluster.go +++ b/cmd/kubeadm/app/util/config/cluster.go @@ -20,7 +20,6 @@ import ( "crypto/x509" "fmt" "io" - "io/ioutil" "path/filepath" "strings" @@ -39,54 +38,26 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) -// FetchConfigFromFileOrCluster fetches configuration required for upgrading your cluster from a file (which has precedence) or a ConfigMap in the cluster -func FetchConfigFromFileOrCluster(client clientset.Interface, w io.Writer, logPrefix, cfgPath string, newControlPlane bool) (*kubeadmapi.InitConfiguration, error) { - // Load the configuration from a file or the cluster - initcfg, err := loadConfiguration(client, w, logPrefix, cfgPath, newControlPlane) +// FetchInitConfigurationFromCluster fetches configuration from a ConfigMap in the cluster +func FetchInitConfigurationFromCluster(client clientset.Interface, w io.Writer, logPrefix string, newControlPlane bool) (*kubeadmapi.InitConfiguration, error) { + fmt.Fprintf(w, "[%s] Reading configuration from the cluster...\n", logPrefix) + fmt.Fprintf(w, "[%s] FYI: You can look at this config file with 'kubectl -n %s get cm %s -oyaml'\n", logPrefix, metav1.NamespaceSystem, constants.KubeadmConfigConfigMap) + + // Fetch the actual config from cluster + cfg, err := getInitConfigurationFromCluster(constants.KubernetesDir, client, newControlPlane) if err != nil { return nil, err } // Apply dynamic defaults - if err := SetInitDynamicDefaults(initcfg); err != nil { - return nil, err - } - return initcfg, err -} - -// loadConfiguration loads the configuration byte slice from either a file or the cluster ConfigMap -func loadConfiguration(client clientset.Interface, w io.Writer, logPrefix, cfgPath string, newControlPlane bool) (*kubeadmapi.InitConfiguration, error) { - // The config file has the highest priority - if cfgPath != "" { - fmt.Fprintf(w, "[%s] Reading configuration options from a file: %s\n", logPrefix, cfgPath) - return loadInitConfigurationFromFile(cfgPath) - } - - fmt.Fprintf(w, "[%s] Reading configuration from the cluster...\n", logPrefix) - fmt.Fprintf(w, "[%s] FYI: You can look at this config file with 'kubectl -n %s get cm %s -oyaml'\n", logPrefix, metav1.NamespaceSystem, constants.KubeadmConfigConfigMap) - return getInitConfigurationFromCluster(constants.KubernetesDir, client, newControlPlane) -} - -func loadInitConfigurationFromFile(cfgPath string) (*kubeadmapi.InitConfiguration, error) { - configBytes, err := ioutil.ReadFile(cfgPath) - if err != nil { + if err := SetInitDynamicDefaults(cfg); err != nil { return nil, err } - // Unmarshal the versioned configuration populated from the file, - // convert it to the internal API types, then default and validate - // NB the file contains multiple YAML, with a combination of - // - a YAML with a InitConfiguration object - // - a YAML with a ClusterConfiguration object (without embedded component configs) - // - separated YAML for components configs - initcfg, err := BytesToInternalConfig(configBytes) - if err != nil { - return nil, err - } - - return initcfg, nil + return cfg, nil } +// getInitConfigurationFromCluster is separate only for testing purposes, don't call it directly, use FetchInitConfigurationFromCluster instead func getInitConfigurationFromCluster(kubeconfigDir string, client clientset.Interface, newControlPlane bool) (*kubeadmapi.InitConfiguration, error) { // TODO: This code should support reading the MasterConfiguration key as well for backwards-compat // Also, the config map really should be KubeadmConfigConfigMap... @@ -124,7 +95,6 @@ func getInitConfigurationFromCluster(kubeconfigDir string, client clientset.Inte return nil, errors.Wrap(err, "failed to getAPIEndpoint") } } - return initcfg, nil } diff --git a/cmd/kubeadm/app/util/config/cluster_test.go b/cmd/kubeadm/app/util/config/cluster_test.go index 1fcc7c72c58..eebc7fe15cc 100644 --- a/cmd/kubeadm/app/util/config/cluster_test.go +++ b/cmd/kubeadm/app/util/config/cluster_test.go @@ -17,7 +17,6 @@ limitations under the License. package config import ( - "bytes" "io/ioutil" "os" "path/filepath" @@ -172,76 +171,6 @@ G+2/lm8TaVjoU7Fi5Ka5G5HY2GLaR7P+IxYcrMHCl62Y7Rqcrnc= `), } -func TestLoadInitConfigurationFromFile(t *testing.T) { - // Create temp folder for the test case - tmpdir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Couldn't create tmpdir") - } - defer os.RemoveAll(tmpdir) - - var tests = []struct { - name string - fileContents []byte - }{ - { - name: "v1beta1.partial1", - fileContents: cfgFiles["InitConfiguration_v1beta1"], - }, - { - name: "v1beta1.partial2", - fileContents: cfgFiles["ClusterConfiguration_v1beta1"], - }, - { - name: "v1beta1.full", - fileContents: bytes.Join([][]byte{ - cfgFiles["InitConfiguration_v1beta1"], - cfgFiles["ClusterConfiguration_v1beta1"], - cfgFiles["Kube-proxy_componentconfig"], - cfgFiles["Kubelet_componentconfig"], - }, []byte(kubeadmconstants.YAMLDocumentSeparator)), - }, - { - name: "v1alpha3.partial1", - fileContents: cfgFiles["InitConfiguration_v1alpha3"], - }, - { - name: "v1alpha3.partial2", - fileContents: cfgFiles["ClusterConfiguration_v1alpha3"], - }, - { - name: "v1alpha3.full", - fileContents: bytes.Join([][]byte{ - cfgFiles["InitConfiguration_v1alpha3"], - cfgFiles["ClusterConfiguration_v1alpha3"], - cfgFiles["Kube-proxy_componentconfig"], - cfgFiles["Kubelet_componentconfig"], - }, []byte(kubeadmconstants.YAMLDocumentSeparator)), - }, - } - - for _, rt := range tests { - t.Run(rt.name, func(t2 *testing.T) { - cfgPath := filepath.Join(tmpdir, rt.name) - err := ioutil.WriteFile(cfgPath, rt.fileContents, 0644) - if err != nil { - t.Errorf("Couldn't create file") - return - } - - obj, err := loadInitConfigurationFromFile(cfgPath) - if err != nil { - t.Errorf("Error reading file: %v", err) - return - } - - if obj == nil { - t.Errorf("Unexpected nil return value") - } - }) - } -} - func TestGetNodeNameFromKubeletConfig(t *testing.T) { tmpdir, err := ioutil.TempDir("", "") if err != nil { diff --git a/cmd/kubeadm/app/util/config/common.go b/cmd/kubeadm/app/util/config/common.go index f3518c339f5..7311b04f289 100644 --- a/cmd/kubeadm/app/util/config/common.go +++ b/cmd/kubeadm/app/util/config/common.go @@ -154,7 +154,7 @@ func MigrateOldConfigFromFile(cfgPath string) ([]byte, error) { // Migrate InitConfiguration and ClusterConfiguration if there are any in the config if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvks...) || kubeadmutil.GroupVersionKindsHasClusterConfiguration(gvks...) { - o, err := ConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.InitConfiguration{}) + o, err := LoadInitConfigurationFromFile(cfgPath) if err != nil { return []byte{}, err } diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go index 31d9ae74616..197467ecd16 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration.go +++ b/cmd/kubeadm/app/util/config/initconfiguration.go @@ -161,32 +161,38 @@ func SetClusterDynamicDefaults(cfg *kubeadmapi.ClusterConfiguration, advertiseAd return nil } -// ConfigFileAndDefaultsToInternalConfig takes a path to a config file and a versioned configuration that can serve as the default config -// If cfgPath is specified, defaultversionedcfg will always get overridden. Otherwise, the default config (often populated by flags) will be used. -// Then the external, versioned configuration is defaulted and converted to the internal type. -// Right thereafter, the configuration is defaulted again with dynamic values (like IP addresses of a machine, etc) -// Lastly, the internal config is validated and returned. -func ConfigFileAndDefaultsToInternalConfig(cfgPath string, defaultversionedcfg *kubeadmapiv1beta1.InitConfiguration) (*kubeadmapi.InitConfiguration, error) { +// DefaultedInitConfiguration takes a versioned init config (often populated by flags), defaults it and converts it into internal InitConfiguration +func DefaultedInitConfiguration(defaultversionedcfg *kubeadmapiv1beta1.InitConfiguration) (*kubeadmapi.InitConfiguration, error) { internalcfg := &kubeadmapi.InitConfiguration{} - if cfgPath != "" { - // Loads configuration from config file, if provided - // Nb. --config overrides command line flags - klog.V(1).Infoln("loading configuration from the given file") + // Takes passed flags into account; the defaulting is executed once again enforcing assignment of + // static default values to cfg only for values not provided with flags + kubeadmscheme.Scheme.Default(defaultversionedcfg) + kubeadmscheme.Scheme.Convert(defaultversionedcfg, internalcfg, nil) - b, err := ioutil.ReadFile(cfgPath) - if err != nil { - return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath) - } - internalcfg, err = BytesToInternalConfig(b) - if err != nil { - return nil, err - } - } else { - // Takes passed flags into account; the defaulting is executed once again enforcing assignment of - // static default values to cfg only for values not provided with flags - kubeadmscheme.Scheme.Default(defaultversionedcfg) - kubeadmscheme.Scheme.Convert(defaultversionedcfg, internalcfg, nil) + // Applies dynamic defaults to settings not provided with flags + if err := SetInitDynamicDefaults(internalcfg); err != nil { + return nil, err + } + // Validates cfg (flags/configs + defaults + dynamic defaults) + if err := validation.ValidateInitConfiguration(internalcfg).ToAggregate(); err != nil { + return nil, err + } + return internalcfg, nil +} + +// LoadInitConfigurationFromFile loads a supported versioned InitConfiguration from a file, converts it into internal config, defaults it and verifies it. +func LoadInitConfigurationFromFile(cfgPath string) (*kubeadmapi.InitConfiguration, error) { + klog.V(1).Infof("loading configuration from %q", cfgPath) + + b, err := ioutil.ReadFile(cfgPath) + if err != nil { + return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath) + } + + internalcfg, err := BytesToInternalConfig(b) + if err != nil { + return nil, err } // Applies dynamic defaults to settings not provided with flags @@ -200,6 +206,21 @@ func ConfigFileAndDefaultsToInternalConfig(cfgPath string, defaultversionedcfg * return internalcfg, nil } +// LoadOrDefaultInitConfiguration takes a path to a config file and a versioned configuration that can serve as the default config +// If cfgPath is specified, defaultversionedcfg will always get overridden. Otherwise, the default config (often populated by flags) will be used. +// Then the external, versioned configuration is defaulted and converted to the internal type. +// Right thereafter, the configuration is defaulted again with dynamic values (like IP addresses of a machine, etc) +// Lastly, the internal config is validated and returned. +func LoadOrDefaultInitConfiguration(cfgPath string, defaultversionedcfg *kubeadmapiv1beta1.InitConfiguration) (*kubeadmapi.InitConfiguration, error) { + if cfgPath != "" { + // Loads configuration from config file, if provided + // Nb. --config overrides command line flags + return LoadInitConfigurationFromFile(cfgPath) + } + + return DefaultedInitConfiguration(defaultversionedcfg) +} + // BytesToInternalConfig converts a byte slice to an internal, defaulted and validated configuration object. // The byte slice may contain one or many different YAML documents. These YAML documents are parsed one-by-one // and well-known ComponentConfig GroupVersionKinds are stored inside of the internal InitConfiguration struct diff --git a/cmd/kubeadm/app/util/config/initconfiguration_test.go b/cmd/kubeadm/app/util/config/initconfiguration_test.go index 307d2f1e603..c7b6afce990 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration_test.go +++ b/cmd/kubeadm/app/util/config/initconfiguration_test.go @@ -19,6 +19,8 @@ package config import ( "bytes" "io/ioutil" + "os" + "path/filepath" "reflect" "testing" @@ -27,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" + "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) const ( @@ -51,13 +54,84 @@ func diff(expected, actual []byte) string { return diffBytes.String() } -func TestConfigFileAndDefaultsToInternalConfig(t *testing.T) { +func TestLoadInitConfigurationFromFile(t *testing.T) { + // Create temp folder for the test case + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Couldn't create tmpdir") + } + defer os.RemoveAll(tmpdir) + + // cfgFiles is in cluster_test.go + var tests = []struct { + name string + fileContents []byte + }{ + { + name: "v1beta1.partial1", + fileContents: cfgFiles["InitConfiguration_v1beta1"], + }, + { + name: "v1beta1.partial2", + fileContents: cfgFiles["ClusterConfiguration_v1beta1"], + }, + { + name: "v1beta1.full", + fileContents: bytes.Join([][]byte{ + cfgFiles["InitConfiguration_v1beta1"], + cfgFiles["ClusterConfiguration_v1beta1"], + cfgFiles["Kube-proxy_componentconfig"], + cfgFiles["Kubelet_componentconfig"], + }, []byte(constants.YAMLDocumentSeparator)), + }, + { + name: "v1alpha3.partial1", + fileContents: cfgFiles["InitConfiguration_v1alpha3"], + }, + { + name: "v1alpha3.partial2", + fileContents: cfgFiles["ClusterConfiguration_v1alpha3"], + }, + { + name: "v1alpha3.full", + fileContents: bytes.Join([][]byte{ + cfgFiles["InitConfiguration_v1alpha3"], + cfgFiles["ClusterConfiguration_v1alpha3"], + cfgFiles["Kube-proxy_componentconfig"], + cfgFiles["Kubelet_componentconfig"], + }, []byte(constants.YAMLDocumentSeparator)), + }, + } + + for _, rt := range tests { + t.Run(rt.name, func(t2 *testing.T) { + cfgPath := filepath.Join(tmpdir, rt.name) + err := ioutil.WriteFile(cfgPath, rt.fileContents, 0644) + if err != nil { + t.Errorf("Couldn't create file") + return + } + + obj, err := LoadInitConfigurationFromFile(cfgPath) + if err != nil { + t.Errorf("Error reading file: %v", err) + return + } + + if obj == nil { + t.Errorf("Unexpected nil return value") + } + }) + } +} + +func TestInitConfigurationMarshallingFromFile(t *testing.T) { var tests = []struct { name, in, out string groupVersion schema.GroupVersion expectedErr bool }{ - // These tests are reading one file, loading it using ConfigFileAndDefaultsToInternalConfig that all of kubeadm is using for unmarshal of our API types, + // These tests are reading one file, loading it using LoadInitConfigurationFromFile that all of kubeadm is using for unmarshal of our API types, // and then marshals the internal object to the expected groupVersion { // v1alpha3 -> internal name: "v1alpha3ToInternal", @@ -83,7 +157,7 @@ func TestConfigFileAndDefaultsToInternalConfig(t *testing.T) { out: master_v1beta1YAML, groupVersion: kubeadmapiv1beta1.SchemeGroupVersion, }, - // These tests are reading one file that has only a subset of the fields populated, loading it using ConfigFileAndDefaultsToInternalConfig, + // These tests are reading one file that has only a subset of the fields populated, loading it using LoadInitConfigurationFromFile, // and then marshals the internal object to the expected groupVersion { // v1beta1 -> default -> validate -> internal -> v1beta1 name: "incompleteYAMLToDefaultedv1beta1", @@ -101,7 +175,7 @@ func TestConfigFileAndDefaultsToInternalConfig(t *testing.T) { for _, rt := range tests { t.Run(rt.name, func(t2 *testing.T) { - internalcfg, err := ConfigFileAndDefaultsToInternalConfig(rt.in, &kubeadmapiv1beta1.InitConfiguration{}) + internalcfg, err := LoadInitConfigurationFromFile(rt.in) if err != nil { if rt.expectedErr { return diff --git a/cmd/kubeadm/test/util.go b/cmd/kubeadm/test/util.go index 6425fa9c530..3ece1d9a0cd 100644 --- a/cmd/kubeadm/test/util.go +++ b/cmd/kubeadm/test/util.go @@ -157,7 +157,7 @@ func AssertError(t *testing.T, err error, expected string) { // GetDefaultInternalConfig returns a defaulted kubeadmapi.InitConfiguration func GetDefaultInternalConfig(t *testing.T) *kubeadmapi.InitConfiguration { - internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig("", &kubeadmapiv1beta1.InitConfiguration{}) + internalcfg, err := configutil.DefaultedInitConfiguration(&kubeadmapiv1beta1.InitConfiguration{}) if err != nil { t.Fatalf("unexpected error getting default config: %v", err) }