From 049d2474922c254b8d79a2bb949ac89a14c1d147 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 28 Jan 2018 13:37:41 -0500 Subject: [PATCH] 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. --- contrib/completions/bash/openshift | 2 + contrib/completions/zsh/openshift | 2 + pkg/cmd/server/api/validation/node.go | 7 +- .../server/kubernetes/node/options/options.go | 45 ++++--- pkg/cmd/server/start/node_args.go | 4 + pkg/cmd/server/start/start_node.go | 121 +++++++++++------- 6 files changed, 109 insertions(+), 72 deletions(-) diff --git a/contrib/completions/bash/openshift b/contrib/completions/bash/openshift index 70b62fb2287c..051b6a67c1ad 100644 --- a/contrib/completions/bash/openshift +++ b/contrib/completions/bash/openshift @@ -541,6 +541,8 @@ _openshift_start_node() flags_with_completion+=("--volume-dir") flags_completion+=("_filedir") local_nonpersistent_flags+=("--volume-dir=") + flags+=("--write-flags") + local_nonpersistent_flags+=("--write-flags") flags+=("--azure-container-registry-config=") flags+=("--google-json-key=") flags+=("--log-flush-frequency=") diff --git a/contrib/completions/zsh/openshift b/contrib/completions/zsh/openshift index 8fda21fea6d8..b1a319f9f85d 100644 --- a/contrib/completions/zsh/openshift +++ b/contrib/completions/zsh/openshift @@ -683,6 +683,8 @@ _openshift_start_node() flags_with_completion+=("--volume-dir") flags_completion+=("_filedir") local_nonpersistent_flags+=("--volume-dir=") + flags+=("--write-flags") + local_nonpersistent_flags+=("--write-flags") flags+=("--azure-container-registry-config=") flags+=("--google-json-key=") flags+=("--log-flush-frequency=") diff --git a/pkg/cmd/server/api/validation/node.go b/pkg/cmd/server/api/validation/node.go index 3534b9419797..2dae0611dd24 100644 --- a/pkg/cmd/server/api/validation/node.go +++ b/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))...) diff --git a/pkg/cmd/server/kubernetes/node/options/options.go b/pkg/cmd/server/kubernetes/node/options/options.go index 4e48dc11a9b3..24d184f44ecb 100644 --- a/pkg/cmd/server/kubernetes/node/options/options.go +++ b/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) { diff --git a/pkg/cmd/server/start/node_args.go b/pkg/cmd/server/start/node_args.go index b67a6a842fb6..4a041c76e465 100644 --- a/pkg/cmd/server/start/node_args.go +++ b/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 diff --git a/pkg/cmd/server/start/start_node.go b/pkg/cmd/server/start/start_node.go index 32f4aa7c8cf5..820667c0deaf 100644 --- a/pkg/cmd/server/start/start_node.go +++ b/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,16 +420,49 @@ 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 } @@ -443,11 +470,7 @@ func StartNode(nodeConfig configapi.NodeConfig, components *utilflags.ComponentF // 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 {