Write Kubelet flags as an option on openshift start node to support moving directly to kubelet

pkg/cmd/server/api/validation/node.go

@@ -24,7 +24,8 @@ func ValidateNodeConfig(config *api.NodeConfig, fldPath *field.Path) ValidationR
 func ValidateInClusterNodeConfig(config *api.NodeConfig, fldPath *field.Path) ValidationResults {
 	validationResults := ValidationResults{}
 
-	if len(config.NodeName) == 0 {
+	hasBootstrapConfig := len(config.KubeletArguments["bootstrap-kubeconfig"]) > 0
+	if len(config.NodeName) == 0 && !hasBootstrapConfig {
 		validationResults.AddErrors(field.Required(fldPath.Child("nodeName"), ""))
 	}
 	if len(config.NodeIP) > 0 {
@@ -42,7 +43,9 @@ func ValidateInClusterNodeConfig(config *api.NodeConfig, fldPath *field.Path) Va
 		validationResults.AddErrors(ValidateHostPort(config.DNSBindAddress, fldPath.Child("dnsBindAddress"))...)
 	}
 	if len(config.DNSIP) > 0 {
-		validationResults.AddErrors(ValidateSpecifiedIP(config.DNSIP, fldPath.Child("dnsIP"))...)
+		if !hasBootstrapConfig || config.DNSIP != "0.0.0.0" {
+			validationResults.AddErrors(ValidateSpecifiedIP(config.DNSIP, fldPath.Child("dnsIP"))...)
+		}
 	}
 	for i, nameserver := range config.DNSNameservers {
 		validationResults.AddErrors(ValidateSpecifiedIPPort(nameserver, fldPath.Child("dnsNameservers").Index(i))...)

pkg/cmd/server/kubernetes/node/options/options.go

@@ -3,6 +3,7 @@ package node
 import (
 	"fmt"
 	"net"
+	"sort"
 	"strings"
 	"time"
 
@@ -23,9 +24,8 @@ import (
 	"github.com/openshift/origin/pkg/network"
 )
 
-// computeKubeletFlags returns the flags to use when starting the kubelet
-// TODO this needs to return a []string and be passed to cobra, but as an intermediate step, we'll compute the map and run it through the existing paths
-func ComputeKubeletFlagsAsMap(startingArgs map[string][]string, options configapi.NodeConfig) (map[string][]string, error) {
+// ComputeKubeletFlags returns the flags to use when starting the kubelet.
+func ComputeKubeletFlags(startingArgs map[string][]string, options configapi.NodeConfig) ([]string, error) {
 	args := map[string][]string{}
 	for key, slice := range startingArgs {
 		for _, val := range slice {
@@ -115,32 +115,35 @@ func ComputeKubeletFlagsAsMap(startingArgs map[string][]string, options configap
 		}
 	}
 
-	// we sometimes have different clusterdns options.  I really don't understand why
-	externalKubeClient, _, err := configapi.GetExternalKubeClient(options.MasterKubeConfig, options.MasterClientConnectionOverrides)
-	if err != nil {
-		return nil, err
-	}
-	args["cluster-dns"] = getClusterDNS(externalKubeClient, args["cluster-dns"])
-
-	return args, nil
-}
-
-func KubeletArgsMapToArgs(argsMap map[string][]string) []string {
-	args := []string{}
-	for key, value := range argsMap {
-		for _, token := range value {
-			args = append(args, fmt.Sprintf("--%s=%v", key, token))
+	// default cluster-dns to the master's DNS if possible, but only if we can reach the master
+	// TODO: this exists to support legacy cases where the node defaulted to the master's DNS.
+	//   we can remove this when we drop support for master DNS when CoreDNS is in use everywhere.
+	if len(args["cluster-dns"]) == 0 {
+		if externalKubeClient, _, err := configapi.GetExternalKubeClient(options.MasterKubeConfig, options.MasterClientConnectionOverrides); err == nil {
+			args["cluster-dns"] = getClusterDNS(externalKubeClient, args["cluster-dns"])
 		}
 	}
 
 	// there is a special case.  If you set `--cgroups-per-qos=false` and `--enforce-node-allocatable` is
 	// an empty string, `--enforce-node-allocatable=""` needs to be explicitly set
 	// cgroups-per-qos defaults to true
-	if cgroupArg, enforceAllocatable := argsMap["cgroups-per-qos"], argsMap["enforce-node-allocatable"]; len(cgroupArg) == 1 && cgroupArg[0] == "false" && len(enforceAllocatable) == 0 {
-		args = append(args, `--enforce-node-allocatable=`)
+	if cgroupArg, enforceAllocatable := args["cgroups-per-qos"], args["enforce-node-allocatable"]; len(cgroupArg) == 1 && cgroupArg[0] == "false" && len(enforceAllocatable) == 0 {
+		args["enforce-node-allocatable"] = []string{""}
 	}
 
-	return args
+	var keys []string
+	for key := range args {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+
+	var arguments []string
+	for _, key := range keys {
+		for _, token := range args[key] {
+			arguments = append(arguments, fmt.Sprintf("--%s=%v", key, token))
+		}
+	}
+	return arguments, nil
 }
 
 func setIfUnset(cmdLineArgs map[string][]string, key string, value ...string) {

pkg/cmd/server/start/node_args.go

@@ -52,6 +52,10 @@ type NodeArgs struct {
 	// Components is the set of enabled components.
 	Components *utilflags.ComponentFlag
 
+	// WriteFlagsOnly will print flags to run the Kubelet from the provided arguments rather than launching
+	// the Kubelet itself.
+	WriteFlagsOnly bool
+
 	// NodeName is the hostname to identify this node with the master.
 	NodeName string
 

pkg/cmd/server/start/start_node.go

@@ -7,6 +7,8 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
+	"strconv"
 	"strings"
 	"syscall"
 
@@ -91,6 +93,7 @@ func NewCommandStartNode(basename string, out, errout io.Writer) (*cobra.Command
 	BindImageFormatArgs(options.NodeArgs.ImageFormatArgs, flags, "")
 	BindKubeConnectionArgs(options.NodeArgs.KubeConnectionArgs, flags, "")
 
+	flags.BoolVar(&options.NodeArgs.WriteFlagsOnly, "write-flags", false, "When this is specified only the arguments necessary to start the Kubelet will be output.")
 	flags.StringVar(&options.NodeArgs.BootstrapConfigName, "bootstrap-config-name", options.NodeArgs.BootstrapConfigName, "On startup, the node will request a client cert from the master and get its config from this config map in the openshift-node namespace (experimental).")
 
 	// autocompletion hints
@@ -172,12 +175,20 @@ func (o NodeOptions) Validate(args []string) error {
 		if o.IsRunFromConfig() {
 			return errors.New("--config may not be set if you're only writing the config")
 		}
+		if o.NodeArgs.WriteFlagsOnly {
+			return errors.New("--write-config and --write-flags are mutually exclusive")
+		}
 	}
 
 	// if we are starting up using a config file, run no validations here
-	if len(o.NodeArgs.BootstrapConfigName) > 0 && !o.IsRunFromConfig() {
-		if err := o.NodeArgs.Validate(); err != nil {
-			return err
+	if len(o.NodeArgs.BootstrapConfigName) > 0 {
+		if o.NodeArgs.WriteFlagsOnly {
+			return errors.New("--write-flags is mutually exclusive with --bootstrap-config-name")
+		}
+		if !o.IsRunFromConfig() {
+			if err := o.NodeArgs.Validate(); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -201,7 +212,7 @@ func (o NodeOptions) StartNode() error {
 		return err
 	}
 
-	if o.IsWriteConfigOnly() {
+	if o.NodeArgs.WriteFlagsOnly || o.IsWriteConfigOnly() {
 		return nil
 	}
 
@@ -224,6 +235,17 @@ func (o NodeOptions) RunNode() error {
 	if addr := o.NodeArgs.ListenArg.ListenAddr; addr.Provided {
 		nodeConfig.ServingInfo.BindAddress = addr.HostPort(o.NodeArgs.ListenArg.ListenAddr.DefaultPort)
 	}
+	// do a local resolution of node config DNS IP, supports bootstrapping cases
+	if nodeConfig.DNSIP == "0.0.0.0" {
+		glog.V(4).Infof("Defaulting to the DNSIP config to the node's IP")
+		nodeConfig.DNSIP = nodeConfig.NodeIP
+		// TODO: the Kubelet should do this defaulting (to the IP it recognizes)
+		if len(nodeConfig.DNSIP) == 0 {
+			if ip, err := cmdutil.DefaultLocalIP4(); err == nil {
+				nodeConfig.DNSIP = ip.String()
+			}
+		}
+	}
 
 	var validationResults validation.ValidationResults
 	switch {
@@ -256,11 +278,11 @@ func (o NodeOptions) RunNode() error {
 		return nil
 	}
 
-	if err := StartNode(*nodeConfig, o.NodeArgs.Components); err != nil {
-		return err
+	if o.NodeArgs.WriteFlagsOnly {
+		return WriteKubeletFlags(*nodeConfig)
 	}
 
-	return nil
+	return StartNode(*nodeConfig, o.NodeArgs.Components)
 }
 
 // resolveNodeConfig creates a new configuration on disk by reading from the master, reads
@@ -371,41 +393,13 @@ func (o NodeOptions) IsRunFromConfig() bool {
 }
 
 // execKubelet attempts to call execve() for the kubelet with the configuration defined
-// in server passed as flags. If the binary is not the same as the current file and
-// the environment variable OPENSHIFT_ALLOW_UNSUPPORTED_KUBELET is unset, the method
-// will return an error. The returned boolean indicates whether fallback to in-process
-// is allowed.
-func execKubelet(kubeletArgs []string) (bool, error) {
-	// verify the Kubelet binary to use
+// in server passed as flags.
+func execKubelet(kubeletArgs []string) error {
 	path := "kubelet"
-	requireSameBinary := true
-	if newPath := os.Getenv("OPENSHIFT_ALLOW_UNSUPPORTED_KUBELET"); len(newPath) > 0 {
-		requireSameBinary = false
-		path = newPath
-	}
 	kubeletPath, err := exec.LookPath(path)
 	if err != nil {
-		return requireSameBinary, err
-	}
-	kubeletFile, err := os.Stat(kubeletPath)
-	if err != nil {
-		return requireSameBinary, err
-	}
-	thisPath, err := exec.LookPath(os.Args[0])
-	if err != nil {
-		return true, err
-	}
-	thisFile, err := os.Stat(thisPath)
-	if err != nil {
-		return true, err
-	}
-	if !os.SameFile(thisFile, kubeletFile) {
-		if requireSameBinary {
-			return true, fmt.Errorf("binary at %q is not the same file as %q, cannot execute", thisPath, kubeletPath)
-		}
-		glog.Warningf("UNSUPPORTED: Executing a different Kubelet than the current binary is not supported: %s", kubeletPath)
+		return err
 	}
-
 	// convert current settings to flags
 	args := append([]string{kubeletPath}, kubeletArgs...)
 	for i := glog.Level(10); i > 0; i-- {
@@ -426,28 +420,57 @@ func execKubelet(kubeletArgs []string) (bool, error) {
 			break
 		}
 	}
+	// execve the child process, replacing this process
 	glog.V(3).Infof("Exec %s %s", kubeletPath, strings.Join(args, " "))
-	return false, syscall.Exec(kubeletPath, args, os.Environ())
+	return syscall.Exec(kubeletPath, args, os.Environ())
+}
+
+// safeArgRegexp matches only characters that are known safe. DO NOT add to this list
+// without fully considering whether that new character can be used to break shell escaping
+// rules.
+var safeArgRegexp = regexp.MustCompile(`^[\da-zA-Z\-=_\.,/\:]+$`)
+
+// shellEscapeArg quotes an argument if it contains characters that my cause a shell
+// interpreter to split the single argument into multiple.
+func shellEscapeArg(s string) string {
+	if safeArgRegexp.MatchString(s) {
+		return s
+	}
+	return strconv.Quote(s)
+}
+
+// WriteKubeletFlags writes the correct set of flags to start a Kubelet from the provided node config to
+// stdout, instead of launching anything.
+func WriteKubeletFlags(nodeConfig configapi.NodeConfig) error {
+	kubeletArgs, err := nodeoptions.ComputeKubeletFlags(nodeConfig.KubeletArguments, nodeConfig)
+	if err != nil {
+		return fmt.Errorf("cannot create kubelet args: %v", err)
+	}
+	if err := nodeoptions.CheckFlags(kubeletArgs); err != nil {
+		return err
+	}
+	var outputArgs []string
+	for _, s := range kubeletArgs {
+		outputArgs = append(outputArgs, shellEscapeArg(s))
+	}
+	fmt.Println(strings.Join(outputArgs, " "))
+	return nil
 }
 
+// StartNode launches the node processes.
 func StartNode(nodeConfig configapi.NodeConfig, components *utilflags.ComponentFlag) error {
-	kubeletFlagsAsMap, err := nodeoptions.ComputeKubeletFlagsAsMap(nodeConfig.KubeletArguments, nodeConfig)
+	kubeletArgs, err := nodeoptions.ComputeKubeletFlags(nodeConfig.KubeletArguments, nodeConfig)
 	if err != nil {
 		return fmt.Errorf("cannot create kubelet args: %v", err)
 	}
-	kubeletArgs := nodeoptions.KubeletArgsMapToArgs(kubeletFlagsAsMap)
 	if err := nodeoptions.CheckFlags(kubeletArgs); err != nil {
 		return err
 	}
 
 	// as a step towards decomposing OpenShift into Kubernetes components, perform an execve
 	// to launch the Kubelet instead of loading in-process
 	if components.Calculated().Equal(sets.NewString(ComponentKubelet)) {
-		ok, err := execKubelet(kubeletArgs)
-		if !ok {
-			return err
-		}
-		if err != nil {
+		if err := execKubelet(kubeletArgs); err != nil {
 			utilruntime.HandleError(fmt.Errorf("Unable to call exec on kubelet, continuing with normal startup: %v", err))
 		}
 	}
@@ -475,9 +498,9 @@ func StartNode(nodeConfig configapi.NodeConfig, components *utilflags.ComponentF
 		glog.V(4).Infof("Unable to build network options: %v", err)
 		return err
 	}
-	clusterDomain := ""
-	if len(kubeletFlagsAsMap["cluster-domain"]) > 0 {
-		clusterDomain = kubeletFlagsAsMap["cluster-domain"][0]
+	clusterDomain := nodeConfig.DNSDomain
+	if len(nodeConfig.KubeletArguments["cluster-domain"]) > 0 {
+		clusterDomain = nodeConfig.KubeletArguments["cluster-domain"][0]
 	}
 	networkConfig, err := network.New(nodeConfig, clusterDomain, proxyConfig, components.Enabled(ComponentProxy), components.Enabled(ComponentDNS) && len(nodeConfig.DNSBindAddress) > 0)
 	if err != nil {