From d2af3d8b6d073dedaeabc945ec88b06158f156a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20L=C3=BCke?= Date: Fri, 6 Mar 2020 12:47:16 +0100 Subject: [PATCH 1/2] kola: Improve debugging abilities through SSH login The tests in "kola run" used a temporary SSH key and directly deleted the machines after a failure. Add the same SSH key option as in "kola spawn" to include additional SSH keys (just "-k" is enough to include the agent/default SSH keys). Also use the option to skip machine removal as in "kola spawn --remove=false". --- cmd/kola/kola.go | 23 ++++++++++++++- cmd/kola/options.go | 63 +++++++++++++++++++++++++++++++++++++++ cmd/kola/spawn.go | 68 ++++--------------------------------------- kola/harness.go | 17 +++++++---- platform/cluster.go | 4 +++ platform/conf/conf.go | 7 +++++ platform/flight.go | 7 ++++- platform/platform.go | 2 ++ 8 files changed, 121 insertions(+), 70 deletions(-) diff --git a/cmd/kola/kola.go b/cmd/kola/kola.go index c53e5d7b5..e31e09b32 100644 --- a/cmd/kola/kola.go +++ b/cmd/kola/kola.go @@ -22,6 +22,8 @@ import ( "sort" "text/tabwriter" + "golang.org/x/crypto/ssh/agent" + "github.com/coreos/pkg/capnslog" "github.com/spf13/cobra" @@ -62,6 +64,10 @@ will be ignored. } listJSON bool + + runRemove bool + runSetSSHKeys bool + runSSHKeys []string ) func init() { @@ -69,6 +75,11 @@ func init() { root.AddCommand(cmdList) cmdList.Flags().BoolVar(&listJSON, "json", false, "format output in JSON") + + cmdRun.Flags().BoolVarP(&runRemove, "remove", "r", true, "remove instances after test exits (--remove=false will keep them)") + cmdRun.Flags().BoolVarP(&runSetSSHKeys, "keys", "k", false, "add SSH keys from --key options") + cmdRun.Flags().StringSliceVar(&runSSHKeys, "key", nil, "path to SSH public key (default: SSH agent + ~/.ssh/id_{rsa,dsa,ecdsa,ed25519}.pub)") + } func main() { @@ -110,7 +121,17 @@ func runRun(cmd *cobra.Command, args []string) { os.Exit(1) } - runErr := kola.RunTests(pattern, kolaPlatform, outputDir) + var sshKeys []agent.Key + if runSetSSHKeys { + sshKeys, err = GetSSHKeys(runSSHKeys) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + os.Exit(1) + } + } else { + sshKeys = nil + } + runErr := kola.RunTests(pattern, kolaPlatform, outputDir, &sshKeys, runRemove) // needs to be after RunTests() because harness empties the directory if err := writeProps(); err != nil { diff --git a/cmd/kola/options.go b/cmd/kola/options.go index f7992fb1e..ef026bd5e 100644 --- a/cmd/kola/options.go +++ b/cmd/kola/options.go @@ -16,9 +16,16 @@ package main import ( "fmt" + "io/ioutil" + "net" "os" + "os/user" + "path/filepath" "strings" + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/agent" + "github.com/coreos/mantle/auth" "github.com/coreos/mantle/kola" "github.com/coreos/mantle/platform" @@ -200,3 +207,59 @@ func syncOptions() error { return nil } + +func GetSSHKeys(sshKeys []string) ([]agent.Key, error) { + var allKeys []agent.Key + // if no keys specified, use keys from agent plus ~/.ssh/id_{rsa,dsa,ecdsa,ed25519}.pub + if len(sshKeys) == 0 { + // add keys directly from the agent + agentEnv := os.Getenv("SSH_AUTH_SOCK") + if agentEnv != "" { + f, err := net.Dial("unix", agentEnv) + if err != nil { + return nil, fmt.Errorf("Couldn't connect to unix socket %q: %v", agentEnv, err) + } + defer f.Close() + + agent := agent.NewClient(f) + keys, err := agent.List() + if err != nil { + return nil, fmt.Errorf("Couldn't talk to ssh-agent: %v", err) + } + for _, key := range keys { + allKeys = append(allKeys, *key) + } + } + + // populate list of key files + userInfo, err := user.Current() + if err != nil { + return nil, err + } + for _, name := range []string{"id_rsa.pub", "id_dsa.pub", "id_ecdsa.pub", "id_ed25519.pub"} { + path := filepath.Join(userInfo.HomeDir, ".ssh", name) + if _, err := os.Stat(path); err == nil { + sshKeys = append(sshKeys, path) + } + } + } + // read key files, failing if any are missing + for _, path := range sshKeys { + keybytes, err := ioutil.ReadFile(path) + if err != nil { + return nil, err + } + pkey, comment, _, _, err := ssh.ParseAuthorizedKey(keybytes) + if err != nil { + return nil, err + } + key := agent.Key{ + Format: pkey.Type(), + Blob: pkey.Marshal(), + Comment: comment, + } + allKeys = append(allKeys, key) + } + + return allKeys, nil +} diff --git a/cmd/kola/spawn.go b/cmd/kola/spawn.go index c6cea5ecc..439bc9949 100644 --- a/cmd/kola/spawn.go +++ b/cmd/kola/spawn.go @@ -19,15 +19,11 @@ import ( "errors" "fmt" "io/ioutil" - "net" "os" - "os/user" "path/filepath" "strings" "github.com/spf13/cobra" - "golang.org/x/crypto/ssh" - "golang.org/x/crypto/ssh/agent" "github.com/coreos/mantle/kola" "github.com/coreos/mantle/platform" @@ -104,13 +100,14 @@ func doSpawn(cmd *cobra.Command, args []string) error { if userdata == nil { userdata = conf.Ignition(`{"ignition": {"version": "2.0.0"}}`) } - // If the user explicitly passed empty userdata, the userdata - // will be non-nil but Empty, and adding SSH keys will - // silently fail. - userdata, err = addSSHKeys(userdata) + sshKeys, err := GetSSHKeys(spawnSSHKeys) if err != nil { return err } + // If the user explicitly passed empty userdata, the userdata + // will be non-nil but Empty, and adding SSH keys will + // silently fail. + userdata = conf.AddSSHKeys(userdata, &sshKeys) } outputDir, err = kola.SetupOutputDir(outputDir, kolaPlatform) @@ -213,58 +210,3 @@ func doSpawn(cmd *cobra.Command, args []string) error { } return nil } - -func addSSHKeys(userdata *conf.UserData) (*conf.UserData, error) { - // if no keys specified, use keys from agent plus ~/.ssh/id_{rsa,dsa,ecdsa,ed25519}.pub - if len(spawnSSHKeys) == 0 { - // add keys directly from the agent - agentEnv := os.Getenv("SSH_AUTH_SOCK") - if agentEnv != "" { - f, err := net.Dial("unix", agentEnv) - if err != nil { - return nil, fmt.Errorf("Couldn't connect to unix socket %q: %v", agentEnv, err) - } - defer f.Close() - - agent := agent.NewClient(f) - keys, err := agent.List() - if err != nil { - return nil, fmt.Errorf("Couldn't talk to ssh-agent: %v", err) - } - for _, key := range keys { - userdata = userdata.AddKey(*key) - } - } - - // populate list of key files - userInfo, err := user.Current() - if err != nil { - return nil, err - } - for _, name := range []string{"id_rsa.pub", "id_dsa.pub", "id_ecdsa.pub", "id_ed25519.pub"} { - path := filepath.Join(userInfo.HomeDir, ".ssh", name) - if _, err := os.Stat(path); err == nil { - spawnSSHKeys = append(spawnSSHKeys, path) - } - } - } - - // read key files, failing if any are missing - for _, path := range spawnSSHKeys { - keybytes, err := ioutil.ReadFile(path) - if err != nil { - return nil, err - } - pkey, comment, _, _, err := ssh.ParseAuthorizedKey(keybytes) - if err != nil { - return nil, err - } - key := agent.Key{ - Format: pkey.Type(), - Blob: pkey.Marshal(), - Comment: comment, - } - userdata = userdata.AddKey(key) - } - return userdata, nil -} diff --git a/kola/harness.go b/kola/harness.go index 33cb200c7..ce67d8a24 100644 --- a/kola/harness.go +++ b/kola/harness.go @@ -24,6 +24,8 @@ import ( "strings" "time" + "golang.org/x/crypto/ssh/agent" + "github.com/coreos/go-semver/semver" "github.com/coreos/pkg/capnslog" @@ -291,7 +293,7 @@ func versionOutsideRange(version, minVersion, endVersion semver.Version) bool { // register tests in their init() function. // outputDir is where various test logs and data will be written for // analysis after the test run. If it already exists it will be erased! -func RunTests(pattern, pltfrm, outputDir string) error { +func RunTests(pattern, pltfrm, outputDir string, sshKeys *[]agent.Key, remove bool) error { var versionStr string // Avoid incurring cost of starting machine in getClusterSemver when @@ -329,7 +331,10 @@ func RunTests(pattern, pltfrm, outputDir string) error { if err != nil { plog.Fatalf("Flight failed: %v", err) } - defer flight.Destroy() + (*flight.GetBaseFlight()).AdditionalSshKeys = sshKeys + if remove { + defer flight.Destroy() + } if !skipGetVersion { plog.Info("Creating cluster to check semver...") @@ -359,7 +364,7 @@ func RunTests(pattern, pltfrm, outputDir string) error { for _, test := range tests { test := test // for the closure run := func(h *harness.H) { - runTest(h, test, pltfrm, flight) + runTest(h, test, pltfrm, flight, remove) } htests.Add(test.Name, run) } @@ -435,7 +440,7 @@ func parseCLVersion(input string) (*semver.Version, error) { // runTest is a harness for running a single test. // outputDir is where various test logs and data will be written for // analysis after the test run. It should already exist. -func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flight) { +func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flight, remove bool) { h.Parallel() rconf := &platform.RuntimeConfig{ @@ -449,7 +454,9 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig h.Fatalf("Cluster failed: %v", err) } defer func() { - c.Destroy() + if remove { + c.Destroy() + } for id, output := range c.ConsoleOutput() { for _, badness := range CheckConsole([]byte(output), t) { h.Errorf("Found %s on machine %s console", badness, id) diff --git a/platform/cluster.go b/platform/cluster.go index 45ca168cd..66c738908 100644 --- a/platform/cluster.go +++ b/platform/cluster.go @@ -151,6 +151,10 @@ func (bc *BaseCluster) RenderUserData(userdata *conf.UserData, ignitionVars map[ } } + if *bc.bf.AdditionalSshKeys != nil { + userdata = conf.AddSSHKeys(userdata, bc.bf.AdditionalSshKeys) + } + conf, err := userdata.Render(bc.bf.ctPlatform) if err != nil { return nil, err diff --git a/platform/conf/conf.go b/platform/conf/conf.go index ca6e9f3ae..9a1eefd1b 100644 --- a/platform/conf/conf.go +++ b/platform/conf/conf.go @@ -862,3 +862,10 @@ func (c *Conf) IsIgnition() bool { func (c *Conf) IsEmpty() bool { return !c.IsIgnition() && c.cloudconfig == nil && c.script == "" } + +func AddSSHKeys(userdata *UserData, keys *[]agent.Key) *UserData { + for _, key := range *keys { + userdata = userdata.AddKey(key) + } + return userdata +} diff --git a/platform/flight.go b/platform/flight.go index 3b7305670..943a6a8a6 100644 --- a/platform/flight.go +++ b/platform/flight.go @@ -34,7 +34,8 @@ type BaseFlight struct { ctPlatform string baseopts *Options - agent *network.SSHAgent + agent *network.SSHAgent + AdditionalSshKeys *[]agent.Key } func NewBaseFlight(opts *Options, platform Name, ctPlatform string) (*BaseFlight, error) { @@ -59,6 +60,10 @@ func NewBaseFlightWithDialer(opts *Options, platform Name, ctPlatform string, di return bf, nil } +func (bf *BaseFlight) GetBaseFlight() *BaseFlight { + return bf +} + func (bf *BaseFlight) Name() string { return bf.name } diff --git a/platform/platform.go b/platform/platform.go index 08c1ad2ea..330fc2a27 100644 --- a/platform/platform.go +++ b/platform/platform.go @@ -134,6 +134,8 @@ type Flight interface { // resources. It should log any failures; since they are not // actionable, it does not return an error. Destroy() + + GetBaseFlight() *BaseFlight } // SystemdDropin is a userdata type agnostic struct representing a systemd dropin From 96f71af6ddf65527d71c0ee3ed08881405a94633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20L=C3=BCke?= Date: Mon, 2 Mar 2020 17:09:28 +0100 Subject: [PATCH 2/2] kola/tests/kubernetes: Use correct GCE variable Use the private IP because the default firewall rules do not allow incoming traffic on 2379 for the public IP. Also, there are no FLATCAR_*_IP* variables set up in /etc/environment for machines running in Google Compute Engine because (as with afterburn) the prefix is still COREOS_. --- kola/tests/kubernetes/controllerInstall.go | 4 ++-- kola/tests/kubernetes/setup.go | 2 +- kola/tests/kubernetes/workerInstall.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/kola/tests/kubernetes/controllerInstall.go b/kola/tests/kubernetes/controllerInstall.go index 0d2104496..cedfda401 100644 --- a/kola/tests/kubernetes/controllerInstall.go +++ b/kola/tests/kubernetes/controllerInstall.go @@ -2,7 +2,7 @@ package kubernetes // https://github.com/coreos/coreos-kubernetes/tree/master/multi-node/generic. // The only change besides paramaterizing the env vars was: -// s/FLATCAR_PUBLIC_IP/FLATCAR_PRIVATE so this works on GCE. +// s/COREOS_PUBLIC_IP/COREOS_PRIVATE_IPV4 so this works on GCE. const controllerInstallScript = `#!/bin/bash set -e @@ -54,7 +54,7 @@ function init_config { fi if [ -z $ADVERTISE_IP ]; then - export ADVERTISE_IP=$(awk -F= '/FLATCAR_PRIVATE_IPV4/ {print $2}' /etc/environment) + export ADVERTISE_IP=$(awk -F= '/COREOS_PRIVATE_IPV4/ {print $2}' /etc/environment) fi for REQ in "${REQUIRED[@]}"; do diff --git a/kola/tests/kubernetes/setup.go b/kola/tests/kubernetes/setup.go index 835e2f996..45e8e083e 100644 --- a/kola/tests/kubernetes/setup.go +++ b/kola/tests/kubernetes/setup.go @@ -318,7 +318,7 @@ func runInstallScript(c cluster.TestCluster, m platform.Machine, script string, var ( etcdConfig = conf.ContainerLinuxConfig(` etcd: - advertise_client_urls: http://{PUBLIC_IPV4}:2379 + advertise_client_urls: http://{PRIVATE_IPV4}:2379 listen_client_urls: http://0.0.0.0:2379 systemd: units: diff --git a/kola/tests/kubernetes/workerInstall.go b/kola/tests/kubernetes/workerInstall.go index 195a3854a..72b214f0a 100644 --- a/kola/tests/kubernetes/workerInstall.go +++ b/kola/tests/kubernetes/workerInstall.go @@ -2,7 +2,7 @@ package kubernetes // https://github.com/coreos/coreos-kubernetes/tree/master/multi-node/generic. // The only change besides paramaterizing the env vars was: -// s/FLATCAR_PUBLIC_IP/FLATCAR_PRIVATE so this works on GCE. +// s/COREOS_PUBLIC_IP/COREOS_PRIVATE_IPV4 so this works on GCE. const workerInstallScript = `#!/bin/bash set -e @@ -43,7 +43,7 @@ function init_config { fi if [ -z $ADVERTISE_IP ]; then - export ADVERTISE_IP=$(awk -F= '/FLATCAR_PRIVATE_IPV4/ {print $2}' /etc/environment) + export ADVERTISE_IP=$(awk -F= '/COREOS_PRIVATE_IPV4/ {print $2}' /etc/environment) fi for REQ in "${REQUIRED[@]}"; do