Merge pull request #18322 from smarterclayton/bootstrap

Automatic merge from submit-queue.

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

Instead of having openshift start node bootstrap, prepare to move to directly invoking the kubelet by having a flag on `openshift start node` called `--write-flags` that generates the arguments to invoke the kubelet for a given `--config`. Instead of calling `openshift start node` to do bootstrapping, we'd instead invoke the --write-flags path and call the kubelet directly.  The generated node-config on the system would have the correct bootstrap settings.

Would require us to move to dynamic config in the kubelet or to add a secondary loop to pull down the latest kube-config. That's probably acceptable.

Also contains a set of changes that allow certificate rotation to happen completely in the background, rather than blocking the kubelet startup. This allows us to keep bootstrapping the node from the master, but to still launch static pods in the bacgkround (right now we can't launch static pods while bootstrapping because bootstrapping is happening *before* the kubelet pod sync loop runs).  In this model, master containers as static pods will not require any node changes to make work (so master nodes wouldn't be different from other nodes).  I'm going to clean this up and propose upstream.

Note that this path would *not* require --runonce mode, which is very good because it's effectively unsupported.

@deads2k we're block on static pod for kubelet until we sort out the path forward. I don't want to have two separate types of node config, and I think this is probably the best position in the long run (all nodes bootstrap and have static pod config, nodes background loop waiting for bootstrapping and reject requests that require client/server connections until bootstrapping completes).

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 {