Support --write-flags on openshift start node

This acts like --write-config, but instead outputs the flags that the
specified --config file would pass to the Kubelet. This prepares us to
stop calling openshift start node and instead direct invoke the Kubelet.

Remove the unsupported kubelet binary code because we are no longer
calling ourself. Also move a small chunk of flag specialization code
into a more appropriate place.

Author: smarterclayton

contrib/completions/bash/openshift

@@ -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))...)

contrib/completions/zsh/openshift

@@ -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/api/validation/node.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/kubernetes/node/options/options.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 {