Move all logic for NodeConfiguration unmarshal to the dedicated package

cmd/kubeadm/app/cmd/join.go

@@ -19,7 +19,6 @@ package cmd
 import (
 	"fmt"
 	"io"
-	"io/ioutil"
 	"path/filepath"
 	"strings"
 
@@ -28,7 +27,6 @@ import (
 	"github.com/spf13/cobra"
 	flag "github.com/spf13/pflag"
 
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	certutil "k8s.io/client-go/util/cert"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -41,8 +39,8 @@ import (
 	kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet"
 	"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
+	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
-	nodeutil "k8s.io/kubernetes/pkg/util/node"
 	utilsexec "k8s.io/utils/exec"
 )
 
@@ -115,9 +113,8 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 		Short: "Run this on any machine you wish to join an existing cluster",
 		Long:  joinLongDescription,
 		Run: func(cmd *cobra.Command, args []string) {
-			j, err := NewValidJoin(cfg, args, skipPreFlight, cfgPath, featureGatesString, ignorePreflightErrors)
+			j, err := NewValidJoin(cmd.PersistentFlags(), cfg, args, skipPreFlight, cfgPath, featureGatesString, ignorePreflightErrors)
 			kubeadmutil.CheckErr(err)
-			kubeadmutil.CheckErr(j.Validate(cmd))
 			kubeadmutil.CheckErr(j.Run(out))
 		},
 	}
@@ -129,7 +126,7 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 }
 
 // NewValidJoin validates the command line that are passed to the cobra command
-func NewValidJoin(cfg *kubeadmapiv1alpha2.NodeConfiguration, args []string, skipPreFlight bool, cfgPath, featureGatesString string, ignorePreflightErrors []string) (*Join, error) {
+func NewValidJoin(flagSet *flag.FlagSet, cfg *kubeadmapiv1alpha2.NodeConfiguration, args []string, skipPreFlight bool, cfgPath, featureGatesString string, ignorePreflightErrors []string) (*Join, error) {
 	cfg.DiscoveryTokenAPIServers = args
 
 	var err error
@@ -137,16 +134,16 @@ func NewValidJoin(cfg *kubeadmapiv1alpha2.NodeConfiguration, args []string, skip
 		return nil, err
 	}
 
-	kubeadmscheme.Scheme.Default(cfg)
+	if err := validation.ValidateMixedArguments(flagSet); err != nil {
-	internalcfg := &kubeadmapi.NodeConfiguration{}
+		return nil, err
-	kubeadmscheme.Scheme.Convert(cfg, internalcfg, nil)
+	}
 
 	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(ignorePreflightErrors, skipPreFlight)
 	if err != nil {
 		return nil, err
 	}
 
-	return NewJoin(cfgPath, args, internalcfg, ignorePreflightErrorsSet)
+	return NewJoin(cfgPath, args, cfg, ignorePreflightErrorsSet)
 }
 
 // AddJoinConfigFlags adds join flags bound to the config to the specified flagset
@@ -205,31 +202,23 @@ type Join struct {
 }
 
 // NewJoin instantiates Join struct with given arguments
-func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, ignorePreflightErrors sets.String) (*Join, error) {
+func NewJoin(cfgPath string, args []string, defaultcfg *kubeadmapiv1alpha2.NodeConfiguration, ignorePreflightErrors sets.String) (*Join, error) {
 
-	if cfg.NodeName == "" {
+	if defaultcfg.NodeName == "" {
 		glog.V(1).Infoln("[join] found NodeName empty")
 		glog.V(1).Infoln("[join] considered OS hostname as NodeName")
-		cfg.NodeName = nodeutil.GetHostname("")
 	}
 
-	if cfgPath != "" {
+	internalcfg, err := configutil.NodeConfigFileAndDefaultsToInternalConfig(cfgPath, defaultcfg)
-		glog.V(1).Infoln("[join] reading configuration from", cfgPath)
-		b, err := ioutil.ReadFile(cfgPath)
 	if err != nil {
-			return nil, fmt.Errorf("unable to read config from %q [%v]", cfgPath, err)
+		return nil, err
-		}
-		glog.V(1).Infoln("[join] decoding configuration information")
-		if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), b, cfg); err != nil {
-			return nil, fmt.Errorf("unable to decode config from %q [%v]", cfgPath, err)
-		}
 	}
 
 	glog.Infoln("[preflight] running pre-flight checks")
 
 	// Then continue with the others...
 	glog.V(1).Infoln("[preflight] running various checks on all nodes")
-	if err := preflight.RunJoinNodeChecks(utilsexec.New(), cfg, ignorePreflightErrors); err != nil {
+	if err := preflight.RunJoinNodeChecks(utilsexec.New(), internalcfg, ignorePreflightErrors); err != nil {
 		return nil, err
 	}
 
@@ -237,15 +226,7 @@ func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, i
 	glog.V(1).Infoln("[preflight] starting kubelet service if it's inactive")
 	preflight.TryStartKubelet(ignorePreflightErrors)
 
-	return &Join{cfg: cfg}, nil
+	return &Join{cfg: internalcfg}, nil
-}
-
-// Validate validates mixed arguments passed to cobra.Command
-func (j *Join) Validate(cmd *cobra.Command) error {
-	if err := validation.ValidateMixedArguments(cmd.PersistentFlags()); err != nil {
-		return err
-	}
-	return validation.ValidateNodeConfiguration(j.cfg).ToAggregate()
 }
 
 // Run executes worker node provisioning and tries to join an existing cluster.

cmd/kubeadm/app/cmd/join_test.go

@@ -156,14 +156,15 @@ func TestNewValidJoin(t *testing.T) {
 			t.Fatalf("Unable to write file %q: %v", tc.cfgPath, err)
 		}
 
-		join, err := NewValidJoin(cfg, tc.args, tc.skipPreFlight, tc.cfgPath, tc.featureGatesString, tc.ignorePreflightErrors)
 		cmd := NewCmdJoin(&out)
-
 		if tc.cmdPersistentFlags != nil {
 			for key, value := range tc.cmdPersistentFlags {
 				cmd.PersistentFlags().Set(key, value)
 			}
 		}
+
+		join, err := NewValidJoin(cmd.PersistentFlags(), cfg, tc.args, tc.skipPreFlight, tc.cfgPath, tc.featureGatesString, tc.ignorePreflightErrors)
+
 		if tc.nodeConfig != nil {
 			join.cfg = tc.nodeConfig
 		}
@@ -174,11 +175,6 @@ func TestNewValidJoin(t *testing.T) {
 			if (err != nil) != tc.expectedError {
 				t.Fatalf(errorFormat, tc.name, tc.expectedError, (err != nil), err)
 			}
-			// test Join.Validate()
-		} else if err == nil && tc.testJoinValidate {
-			if err = join.Validate(cmd); (err != nil) != tc.expectedError {
-				t.Fatalf(errorFormat, tc.name, tc.expectedError, (err != nil), err)
-			}
 			// check error for NewValidJoin()
 		} else if (err != nil) != tc.expectedError {
 			t.Fatalf(errorFormat, tc.name, tc.expectedError, (err != nil), err)

cmd/kubeadm/app/util/config/masterconfig.go

@@ -37,7 +37,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/version"
 )
 
-// SetInitDynamicDefaults checks and sets configuration values for Master node
+// SetInitDynamicDefaults checks and sets configuration values for the MasterConfiguration object
 func SetInitDynamicDefaults(cfg *kubeadmapi.MasterConfiguration) error {
 
 	// validate cfg.API.AdvertiseAddress.

cmd/kubeadm/app/util/config/nodeconfig.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2018 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 config
+
+import (
+	"fmt"
+	"io/ioutil"
+
+	"github.com/golang/glog"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme"
+	kubeadmapiv1alpha1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
+	kubeadmapiv1alpha2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha2"
+	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
+	"k8s.io/kubernetes/pkg/util/node"
+)
+
+// SetJoinDynamicDefaults checks and sets configuration values for the NodeConfiguration object
+func SetJoinDynamicDefaults(cfg *kubeadmapi.NodeConfiguration) error {
+	cfg.NodeName = node.GetHostname(cfg.NodeName)
+	return nil
+}
+
+// NodeConfigFileAndDefaultsToInternalConfig
+func NodeConfigFileAndDefaultsToInternalConfig(cfgPath string, defaultversionedcfg *kubeadmapiv1alpha2.NodeConfiguration) (*kubeadmapi.NodeConfiguration, error) {
+	internalcfg := &kubeadmapi.NodeConfiguration{}
+
+	if cfgPath != "" {
+		// Loads configuration from config file, if provided
+		// Nb. --config overrides command line flags
+		glog.V(1).Infoln("loading configuration from the given file")
+
+		b, err := ioutil.ReadFile(cfgPath)
+		if err != nil {
+			return nil, fmt.Errorf("unable to read config from %q [%v]", cfgPath, err)
+		}
+		runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(kubeadmapiv1alpha1.SchemeGroupVersion, kubeadmapiv1alpha2.SchemeGroupVersion), b, internalcfg)
+	} else {
+		// Takes passed flags into account; the defaulting is executed once again enforcing assignement 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 := SetJoinDynamicDefaults(internalcfg); err != nil {
+		return nil, err
+	}
+	// Validates cfg (flags/configs + defaults)
+	if err := validation.ValidateNodeConfiguration(internalcfg).ToAggregate(); err != nil {
+		return nil, err
+	}
+
+	return internalcfg, nil
+}