From a612a03359f7c4daba9b68fe7eae96aa6b6187e2 Mon Sep 17 00:00:00 2001 From: Andrey Trubachev <1354987+atrubachev@users.noreply.github.com> Date: Mon, 20 Apr 2020 17:08:24 +0200 Subject: [PATCH] PSM-2322 Fix a race condition in creation SSHClient --- internal/sshclient/sshclient.go | 146 +++++++------ internal/sshclient/sshclient_test.go | 85 ++++++++ internal/utils.go | 8 + providers/dell/idrac8/actions.go | 95 +++------ providers/dell/idrac8/idrac8.go | 10 +- providers/dell/idrac8/setupConnections.go | 35 +-- providers/dell/idrac9/actions.go | 90 +++----- providers/dell/idrac9/idrac9.go | 9 +- providers/dell/idrac9/setupConnections.go | 34 +-- providers/dell/m1000e/actions.go | 249 ++++++++-------------- providers/dell/m1000e/actions_test.go | 44 ++-- providers/dell/m1000e/m1000e.go | 9 +- providers/dell/m1000e/setupConnections.go | 34 +-- providers/dummy/ibmc/configure.go | 4 +- providers/hp/c7000/actions.go | 220 +++++++------------ providers/hp/c7000/actions_test.go | 36 ++-- providers/hp/c7000/c7000.go | 21 +- providers/hp/c7000/setupConnections.go | 37 +--- providers/hp/ilo/actions.go | 95 +++------ providers/hp/ilo/actions_test.go | 13 +- providers/hp/ilo/ilo.go | 29 ++- providers/hp/ilo/setupConnection.go | 46 ++-- sshmock/sshmock_test.go | 16 +- 23 files changed, 600 insertions(+), 765 deletions(-) create mode 100644 internal/sshclient/sshclient_test.go create mode 100644 internal/utils.go diff --git a/internal/sshclient/sshclient.go b/internal/sshclient/sshclient.go index bc1e1fc2..b560157f 100644 --- a/internal/sshclient/sshclient.go +++ b/internal/sshclient/sshclient.go @@ -3,101 +3,119 @@ package sshclient import ( "fmt" "net" - "strings" + "sync" "time" - "unicode" "golang.org/x/crypto/ssh" ) const ( - // PowerOff defines the action of powering off a device - PowerOff = "poweroff" - // PowerOn defines the action of powering on a device - PowerOn = "poweron" - // PowerCycle defines the action of power cycle a device - PowerCycle = "powercycle" - // HardReset defines the action of hard reset a device - HardReset = "hardreset" - // Reseat defines the action of power reseat a device - Reseat = "reseat" - // IsOn defines the current power status of a device - IsOn = "ison" - // PowerCycleBmc the action of power cycle the bmc of a device - PowerCycleBmc = "powercyclebmc" - // PxeOnce the action of pxe once a device - PxeOnce = "pxeonce" + clientTimeout = 15 * time.Second + sshPort = "22" ) -// SSHClient implements out commom abstraction for ssh +// SSHClient implements out common abstraction for SSH type SSHClient struct { + addr string + config *ssh.ClientConfig client *ssh.Client + lock *sync.Mutex } -// Sleep transforms a sleep statement in a sleep-able time -func Sleep(sleep string) (err error) { - sleep = strings.Replace(sleep, "sleep ", "", 1) - s, err := time.ParseDuration(sleep) +// New creates a new SSH client +func New(addr string, username string, password string) (*SSHClient, error) { + cfg := &ssh.ClientConfig{ + User: username, + Auth: []ssh.AuthMethod{ + ssh.Password(password), + ssh.KeyboardInteractive(func(user, instruction string, questions []string, echos []bool) ([]string, error) { + return []string{}, nil + }), + }, + HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error { + return nil + }, + Timeout: clientTimeout, + } + + addr, err := checkAndBuildAddr(addr) if err != nil { - return fmt.Errorf("error sleeping: %v", err) + return nil, err } - time.Sleep(s) - return err + return &SSHClient{addr: addr, config: cfg, lock: new(sync.Mutex)}, nil } -// Run execute the given command and returns a string with the output -func (s *SSHClient) Run(command string) (result string, err error) { +// Run executes the given command and returns the output as a string +func (s *SSHClient) Run(command string) (string, error) { + if err := s.createClient(); err != nil { + return "", err + } + + return s.run(command) +} + +func (s *SSHClient) run(command string) (string, error) { session, err := s.client.NewSession() if err != nil { - return result, err + return "", err } defer session.Close() output, err := session.CombinedOutput(command) - if err != nil { - return string(output), err - } - return string(output), err } -// IsntLetterOrNumber check if the give rune is not a letter nor a number -func IsntLetterOrNumber(c rune) bool { - return !unicode.IsLetter(c) && !unicode.IsNumber(c) +// Close sends "exit" command and closes the SSH connection +func (s *SSHClient) Close() error { + s.lock.Lock() + defer s.lock.Unlock() + + if s.client == nil { + return nil + } + defer func() { + s.client.Close() + s.client = nil + }() + + // some vendors have issues with the bmc if you don't do it + if _, err := s.run("exit"); err != nil { + return err + } + + return nil } -// New returns a new configured ssh client -func New(host string, username string, password string) (connection *SSHClient, err error) { - if !strings.Contains(host, ":") { - host = fmt.Sprintf("%s:22", host) +func (s *SSHClient) createClient() error { + s.lock.Lock() + defer s.lock.Unlock() + if s.client != nil { + return nil // TODO: check client is alive } - c, err := ssh.Dial( - "tcp", - host, - &ssh.ClientConfig{ - User: username, - Auth: []ssh.AuthMethod{ - ssh.Password(password), - ssh.KeyboardInteractive(func(user, instruction string, questions []string, echos []bool) ([]string, error) { - return []string{}, nil - }), - }, - HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error { - return nil - }, - Timeout: 15 * time.Second, - }, - ) + + c, err := ssh.Dial("tcp", s.addr, s.config) if err != nil { - return connection, fmt.Errorf("unable to connect to bmc: %v", err) + return fmt.Errorf("unable to connect to bmc: %w", err) } - return &SSHClient{c}, err + s.client = c + + return nil } -// Close closed the ssh connection and ensure to always exit, some vendors will have issues with the bmc if you dont do it -func (s *SSHClient) Close() (err error) { - defer s.client.Close() - _, err = s.Run("exit") - return err +func checkAndBuildAddr(addr string) (string, error) { + if addr == "" { + return "", fmt.Errorf("address is empty") + } + + if _, _, err := net.SplitHostPort(addr); err == nil { + return addr, nil + } + + addrWithPort := net.JoinHostPort(addr, sshPort) + if _, _, err := net.SplitHostPort(addrWithPort); err == nil { + return addrWithPort, nil + } + + return "", fmt.Errorf("failed to parse address %q", addr) } diff --git a/internal/sshclient/sshclient_test.go b/internal/sshclient/sshclient_test.go new file mode 100644 index 00000000..d68af297 --- /dev/null +++ b/internal/sshclient/sshclient_test.go @@ -0,0 +1,85 @@ +package sshclient + +import "testing" + +func Test_checkAndBuildAddr(t *testing.T) { + type args struct { + addr string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "OK: only IPv4 address", + args: args{ + "127.0.0.1", + }, + want: "127.0.0.1:22", + wantErr: false, + }, + { + name: "OK: only IPv6 address", + args: args{ + "fe80::1", + }, + want: "[fe80::1]:22", + wantErr: false, + }, + { + name: "OK: only host", + args: args{ + "localhost", + }, + want: "localhost:22", + wantErr: false, + }, + { + name: "OK: IPv4 address with port", + args: args{ + "127.0.0.1:2222", + }, + want: "127.0.0.1:2222", + wantErr: false, + }, + { + name: "OK: IPv6 address with port", + args: args{ + "[fe80::1]:2222", + }, + want: "[fe80::1]:2222", + wantErr: false, + }, + { + name: "OK: host with port", + args: args{ + "localhost:2222", + }, + want: "localhost:2222", + wantErr: false, + }, + { + name: "Not OK: empty addr", + args: args{ + "", + }, + want: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := checkAndBuildAddr(tt.args.addr) + t.Log(got) + if (err != nil) != tt.wantErr { + t.Errorf("checkAndBuildAddr() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("checkAndBuildAddr() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/utils.go b/internal/utils.go new file mode 100644 index 00000000..5096f8a5 --- /dev/null +++ b/internal/utils.go @@ -0,0 +1,8 @@ +package internal + +import "unicode" + +// IsntLetterOrNumber check if the give rune is not a letter nor a number +func IsntLetterOrNumber(c rune) bool { + return !unicode.IsLetter(c) && !unicode.IsNumber(c) +} diff --git a/providers/dell/idrac8/actions.go b/providers/dell/idrac8/actions.go index d17c94e8..d5fe87a9 100644 --- a/providers/dell/idrac8/actions.go +++ b/providers/dell/idrac8/actions.go @@ -7,113 +7,87 @@ import ( ) // PowerCycle reboots the machine via bmc -func (i *IDrac8) PowerCycle() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac8) PowerCycle() (bool, error) { output, err := i.sshClient.Run("racadm serveraction hardreset") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerCycleBmc reboots the bmc we are connected to -func (i *IDrac8) PowerCycleBmc() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac8) PowerCycleBmc() (bool, error) { output, err := i.sshClient.Run("racadm racreset hard") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "initiated successfully") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOn power on the machine via bmc -func (i *IDrac8) PowerOn() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac8) PowerOn() (bool, error) { output, err := i.sshClient.Run("racadm serveraction powerup") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOff power off the machine via bmc -func (i *IDrac8) PowerOff() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac8) PowerOff() (bool, error) { output, err := i.sshClient.Run("racadm serveraction powerdown") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PxeOnce makes the machine to boot via pxe once -func (i *IDrac8) PxeOnce() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac8) PxeOnce() (bool, error) { output, err := i.sshClient.Run("racadm config -g cfgServerInfo -o cfgServerBootOnce 1") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { output, err = i.sshClient.Run("racadm config -g cfgServerInfo -o cfgServerFirstBootDevice PXE") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { return i.PowerCycle() } } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // IsOn tells if a machine is currently powered on -func (i *IDrac8) IsOn() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac8) IsOn() (bool, error) { output, err := i.sshClient.Run("racadm serveraction powerstatus") if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Server power status: ON") { @@ -124,19 +98,14 @@ func (i *IDrac8) IsOn() (status bool, err error) { return false, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // UpdateFirmware updates the bmc firmware -func (i *IDrac8) UpdateFirmware(source, file string) (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac8) UpdateFirmware(source, file string) (bool, error) { u, err := url.Parse(source) if err != nil { - return status, err + return false, err } password, ok := u.User.Password() @@ -147,12 +116,12 @@ func (i *IDrac8) UpdateFirmware(source, file string) (status bool, err error) { cmd := fmt.Sprintf("racadm fwupdate -f %s %s %s -d %s/%s", u.Host, u.User.Username(), password, u.Path, file) output, err := i.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Firmware update completed successfully") { - return true, err + return true, nil } - return status, err + return false, err } diff --git a/providers/dell/idrac8/idrac8.go b/providers/dell/idrac8/idrac8.go index 52eb2613..23241d88 100644 --- a/providers/dell/idrac8/idrac8.go +++ b/providers/dell/idrac8/idrac8.go @@ -41,8 +41,13 @@ type IDrac8 struct { } // New returns a new IDrac8 ready to be used -func New(ip string, username string, password string) (iDrac *IDrac8, err error) { - return &IDrac8{ip: ip, username: username, password: password}, err +func New(host string, username string, password string) (*IDrac8, error) { + sshClient, err := sshclient.New(host, username, password) + if err != nil { + return nil, err + } + + return &IDrac8{ip: host, username: username, password: password, sshClient: sshClient}, err } // CheckCredentials verify whether the credentials are valid or not @@ -115,7 +120,6 @@ func (i *IDrac8) put(endpoint string, payload []byte) (statusCode int, response // posts the payload to the given endpoint func (i *IDrac8) post(endpoint string, data []byte, formDataContentType string) (statusCode int, body []byte, err error) { - u, err := url.Parse(fmt.Sprintf("https://%s/%s", i.ip, endpoint)) if err != nil { return 0, []byte{}, err diff --git a/providers/dell/idrac8/setupConnections.go b/providers/dell/idrac8/setupConnections.go index 18abc450..ecfbb18c 100644 --- a/providers/dell/idrac8/setupConnections.go +++ b/providers/dell/idrac8/setupConnections.go @@ -11,7 +11,6 @@ import ( "github.com/bmc-toolbox/bmclib/errors" "github.com/bmc-toolbox/bmclib/internal/httpclient" - "github.com/bmc-toolbox/bmclib/internal/sshclient" "github.com/bmc-toolbox/bmclib/providers/dell" multierror "github.com/hashicorp/go-multierror" @@ -106,39 +105,23 @@ func (i *IDrac8) loadHwData() (err error) { return err } -// sshLogin initiates the connection to a bmc device -func (i *IDrac8) sshLogin() (err error) { - if i.sshClient != nil { - return - } - - log.WithFields(log.Fields{"step": "bmc connection", "vendor": dell.VendorID, "ip": i.ip}).Debug("connecting to bmc") - i.sshClient, err = sshclient.New(i.ip, i.username, i.password) - if err != nil { - return err - } - - return err -} - // Close closes the connection properly -func (i *IDrac8) Close() (err error) { +func (i *IDrac8) Close() error { + var multiErr error + if i.httpClient != nil { - resp, e := i.httpClient.Get(fmt.Sprintf("https://%s/data/logout", i.ip)) - if e != nil { - err = multierror.Append(e, err) + resp, err := i.httpClient.Get(fmt.Sprintf("https://%s/data/logout", i.ip)) + if err != nil { + multiErr = multierror.Append(multiErr, err) } else { defer resp.Body.Close() defer io.Copy(ioutil.Discard, resp.Body) } } - if i.sshClient != nil { - e := i.sshClient.Close() - if e != nil { - err = multierror.Append(e, err) - } + if err := i.sshClient.Close(); err != nil { + multiErr = multierror.Append(multiErr, err) } - return err + return multiErr } diff --git a/providers/dell/idrac9/actions.go b/providers/dell/idrac9/actions.go index ee5dc06e..893842b1 100644 --- a/providers/dell/idrac9/actions.go +++ b/providers/dell/idrac9/actions.go @@ -7,113 +7,88 @@ import ( ) // PowerCycle reboots the machine via bmc -func (i *IDrac9) PowerCycle() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac9) PowerCycle() (bool, error) { output, err := i.sshClient.Run("racadm serveraction hardreset") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerCycleBmc reboots the bmc we are connected to -func (i *IDrac9) PowerCycleBmc() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac9) PowerCycleBmc() (bool, error) { output, err := i.sshClient.Run("racadm racreset hard") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "initiated successfully") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOn power on the machine via bmc -func (i *IDrac9) PowerOn() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac9) PowerOn() (bool, error) { output, err := i.sshClient.Run("racadm serveraction powerup") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOff power off the machine via bmc -func (i *IDrac9) PowerOff() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac9) PowerOff() (bool, error) { output, err := i.sshClient.Run("racadm serveraction powerdown") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PxeOnce makes the machine to boot via pxe once -func (i *IDrac9) PxeOnce() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac9) PxeOnce() (bool, error) { output, err := i.sshClient.Run("racadm config -g cfgServerInfo -o cfgServerBootOnce 1") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { output, err = i.sshClient.Run("racadm config -g cfgServerInfo -o cfgServerFirstBootDevice PXE") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { return i.PowerCycle() } } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // IsOn tells if a machine is currently powered on func (i *IDrac9) IsOn() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } output, err := i.sshClient.Run("racadm serveraction powerstatus") if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Server power status: ON") { @@ -128,15 +103,10 @@ func (i *IDrac9) IsOn() (status bool, err error) { } // UpdateFirmware updates the bmc firmware -func (i *IDrac9) UpdateFirmware(source, file string) (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *IDrac9) UpdateFirmware(source, file string) (bool, error) { u, err := url.Parse(source) if err != nil { - return status, err + return false, err } password, ok := u.User.Password() @@ -147,12 +117,12 @@ func (i *IDrac9) UpdateFirmware(source, file string) (status bool, err error) { cmd := fmt.Sprintf("racadm fwupdate -f %s %s %s -d %s/%s", u.Host, u.User.Username(), password, u.Path, file) output, err := i.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Firmware update completed successfully") { - return true, err + return true, nil } - return status, err + return false, fmt.Errorf(output) } diff --git a/providers/dell/idrac9/idrac9.go b/providers/dell/idrac9/idrac9.go index 68f4dcf4..9d8fbfcc 100644 --- a/providers/dell/idrac9/idrac9.go +++ b/providers/dell/idrac9/idrac9.go @@ -41,8 +41,13 @@ type IDrac9 struct { } // New returns a new IDrac9 ready to be used -func New(ip string, username string, password string) (iDrac *IDrac9, err error) { - return &IDrac9{ip: ip, username: username, password: password}, err +func New(host string, username string, password string) (*IDrac9, error) { + sshClient, err := sshclient.New(host, username, password) + if err != nil { + return nil, err + } + + return &IDrac9{ip: host, username: username, password: password, sshClient: sshClient}, nil } // CheckCredentials verify whether the credentials are valid or not diff --git a/providers/dell/idrac9/setupConnections.go b/providers/dell/idrac9/setupConnections.go index c7f775a9..e7414e68 100644 --- a/providers/dell/idrac9/setupConnections.go +++ b/providers/dell/idrac9/setupConnections.go @@ -10,7 +10,6 @@ import ( "github.com/bmc-toolbox/bmclib/errors" "github.com/bmc-toolbox/bmclib/internal/httpclient" - "github.com/bmc-toolbox/bmclib/internal/sshclient" "github.com/bmc-toolbox/bmclib/providers/dell" multierror "github.com/hashicorp/go-multierror" @@ -124,36 +123,19 @@ func (i *IDrac9) loadHwData() (err error) { return err } -// sshLogin initiates the connection to a bmc device -func (i *IDrac9) sshLogin() (err error) { - if i.sshClient != nil { - return - } - - log.WithFields(log.Fields{"step": "bmc connection", "vendor": dell.VendorID, "ip": i.ip}).Debug("connecting to bmc") - i.sshClient, err = sshclient.New(i.ip, i.username, i.password) - if err != nil { - return err - } - - return err -} - // Close closes the connection properly -func (i *IDrac9) Close() (err error) { +func (i *IDrac9) Close() error { + var multiErr error + if i.httpClient != nil { - _, _, e := i.delete("sysmgmt/2015/bmc/session") - if e != nil { - err = multierror.Append(e, err) + if _, _, err := i.delete("sysmgmt/2015/bmc/session"); err != nil { + multiErr = multierror.Append(multiErr, err) } } - if i.sshClient != nil { - e := i.sshClient.Close() - if e != nil { - err = multierror.Append(e, err) - } + if err := i.sshClient.Close(); err != nil { + multiErr = multierror.Append(multiErr, err) } - return err + return multiErr } diff --git a/providers/dell/m1000e/actions.go b/providers/dell/m1000e/actions.go index a3dcfa80..676762d2 100644 --- a/providers/dell/m1000e/actions.go +++ b/providers/dell/m1000e/actions.go @@ -7,269 +7,214 @@ import ( "strings" "github.com/bmc-toolbox/bmclib/errors" - "github.com/bmc-toolbox/bmclib/internal/sshclient" + "github.com/bmc-toolbox/bmclib/internal" ) // PowerCycle reboots the chassis -func (m *M1000e) PowerCycle() (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) PowerCycle() (bool, error) { output, err := m.sshClient.Run("racadm racreset") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOn power on the chassis -func (m *M1000e) PowerOn() (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) PowerOn() (bool, error) { output, err := m.sshClient.Run("chassisaction powerup") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOff power off the chassis -func (m *M1000e) PowerOff() (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) PowerOff() (bool, error) { output, err := m.sshClient.Run("chassisaction powerdown") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // IsOn tells if a machine is currently powered on -func (m *M1000e) IsOn() (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) IsOn() (bool, error) { output, err := m.sshClient.Run("getsysinfo") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, " = ON") { - return true, err + return true, nil } - return status, err + return false, err } // FindBladePosition receives a serial and find the position of the blade using it -func (m *M1000e) FindBladePosition(serial string) (position int, err error) { - err = m.sshLogin() - if err != nil { - return position, err - } - +func (m *M1000e) FindBladePosition(serial string) (int, error) { output, err := m.sshClient.Run("getsvctag") if err != nil { - return position, err + return -1, fmt.Errorf("output: %q: %w", output, err) } - for _, line := range strings.Split(string(output), "\n") { + for _, line := range strings.Split(output, "\n") { line = strings.Replace(line, "Server-", "", -1) - data := strings.FieldsFunc(line, sshclient.IsntLetterOrNumber) + data := strings.FieldsFunc(line, internal.IsntLetterOrNumber) for _, field := range data { if strings.ToLower(serial) == strings.ToLower(field) { - position, err := strconv.Atoi(data[0]) - return position, err + return strconv.Atoi(data[0]) } } } - return position, fmt.Errorf("Unable to find the blade in this chassis") + return -1, fmt.Errorf("unable to find the blade in this chassis") } // PowerCycleBlade reboots the machine via bmc -func (m *M1000e) PowerCycleBlade(position int) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) PowerCycleBlade(position int) (bool, error) { output, err := m.sshClient.Run(fmt.Sprintf("serveraction -m server-%d hardreset", position)) if err != nil { if strings.Contains(output, "is already powered OFF") { return m.PowerOnBlade(position) } - return false, fmt.Errorf(output) + + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // ReseatBlade reboots the machine via bmc -func (m *M1000e) ReseatBlade(position int) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) ReseatBlade(position int) (bool, error) { output, err := m.sshClient.Run(fmt.Sprintf("serveraction -m server-%d reseat -f", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOnBlade power on the machine via bmc -func (m *M1000e) PowerOnBlade(position int) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) PowerOnBlade(position int) (bool, error) { output, err := m.sshClient.Run(fmt.Sprintf("serveraction -m server-%d powerup", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOffBlade power off the machine via bmc -func (m *M1000e) PowerOffBlade(position int) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) PowerOffBlade(position int) (bool, error) { output, err := m.sshClient.Run(fmt.Sprintf("serveraction -m server-%d powerdown", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // IsOnBlade tells if a machine is currently powered on -func (m *M1000e) IsOnBlade(position int) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) IsOnBlade(position int) (bool, error) { output, err := m.sshClient.Run(fmt.Sprintf("serveraction -m server-%d powerstatus", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "ON") { - return true, err + return true, nil + } + + if strings.Contains(output, "OFF") { + return false, nil } - return status, err + return false, fmt.Errorf(output) } // PowerCycleBmcBlade reboots the bmc we are connected to -func (m *M1000e) PowerCycleBmcBlade(position int) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) PowerCycleBmcBlade(position int) (bool, error) { output, err := m.sshClient.Run(fmt.Sprintf("racreset -m server-%d", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PxeOnceBlade makes the machine to boot via pxe once -func (m *M1000e) PxeOnceBlade(position int) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - - status, err = m.PowerCycleBlade(position) +func (m *M1000e) PxeOnceBlade(position int) (bool, error) { + status, err := m.PowerCycleBlade(position) if err != nil { return status, err } output, err := m.sshClient.Run(fmt.Sprintf("deploy -m server-%d -b PXE -o yes", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // SetIpmiOverLan Enable/Disable IPMI over lan parameter per blade in chassis -func (m *M1000e) SetIpmiOverLan(position int, enable bool) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) SetIpmiOverLan(position int, enable bool) (bool, error) { var state int if enable { state = 1 - } else { - state = 0 } cmd := fmt.Sprintf("config -g cfgServerInfo -o cfgServerIPMIOverLanEnable -i %d %d", position, state) output, err := m.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } @@ -277,44 +222,31 @@ func (m *M1000e) SetIpmiOverLan(position int, enable bool) (status bool, err err // Dynamic Power Supply Engagement (DPSE) mode is disabled by default. // DPSE saves power by optimizing the power efficiency of the PSUs supplying power to the chassis. // This also increases the PSU life, and reduces heat generation. -func (m *M1000e) SetDynamicPower(enable bool) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) SetDynamicPower(enable bool) (bool, error) { var state int if enable { state = 1 - } else { - state = 0 } cmd := fmt.Sprintf("config -g cfgChassisPower -o cfgChassisDynamicPSUEngagementEnable %d", state) output, err := m.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) - + return false, fmt.Errorf(output) } // SetFlexAddressState Disable/Enable FlexAddress disables flex Addresses for blades // FlexAddress is a virtual addressing scheme -func (m *M1000e) SetFlexAddressState(position int, enable bool) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) SetFlexAddressState(position int, enable bool) (bool, error) { isOn, err := m.IsOnBlade(position) if err != nil { - return false, fmt.Errorf("failed to validate blade %d power status is off, %v", position, err) + return false, fmt.Errorf("failed to validate blade %d power status is off: %w", position, err) } if isOn { @@ -330,26 +262,21 @@ func (m *M1000e) SetFlexAddressState(position int, enable bool) (status bool, er output, err := m.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "successful") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // UpdateFirmware updates the chassis firmware -func (m *M1000e) UpdateFirmware(source, file string) (status bool, err error) { - err = m.sshLogin() - if err != nil { - return status, err - } - +func (m *M1000e) UpdateFirmware(source, file string) (bool, error) { u, err := url.Parse(source) if err != nil { - return status, err + return false, err } password, ok := u.User.Password() @@ -360,33 +287,33 @@ func (m *M1000e) UpdateFirmware(source, file string) (status bool, err error) { cmd := fmt.Sprintf("fwupdate -f %s %s %s -d %s -m cmc-active -m cmc-standby", u.Host, u.User.Username(), password, u.Path) output, err := m.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Firmware update has been initiated") { - return true, err + return true, nil } - return status, err + return false, err } // UpdateFirmwareBmcBlade updates the blade BMC firmware -func (m *M1000e) UpdateFirmwareBmcBlade(position int, host, filepath string) (status bool, err error) { +func (m *M1000e) UpdateFirmwareBmcBlade(position int, host, filepath string) (bool, error) { // iDRAC 7 or later is not supported by fwupdate on the M1000e - return status, errors.ErrNotImplemented + return false, errors.ErrNotImplemented } // AddBladeBmcAdmin adds BMC Admin user accounts through the chassis. -func (m *M1000e) AddBladeBmcAdmin(username string, password string) (err error) { +func (m *M1000e) AddBladeBmcAdmin(username string, password string) error { return errors.ErrNotImplemented } // RemoveBladeBmcUser removes BMC Admin user accounts through the chassis. -func (m *M1000e) RemoveBladeBmcUser(username string) (err error) { +func (m *M1000e) RemoveBladeBmcUser(username string) error { return errors.ErrNotImplemented } // ModBladeBmcUser modifies a BMC Admin user password through the chassis. -func (m *M1000e) ModBladeBmcUser(username string, password string) (err error) { +func (m *M1000e) ModBladeBmcUser(username string, password string) error { return errors.ErrNotImplemented } diff --git a/providers/dell/m1000e/actions_test.go b/providers/dell/m1000e/actions_test.go index ddbdb123..ce11a887 100644 --- a/providers/dell/m1000e/actions_test.go +++ b/providers/dell/m1000e/actions_test.go @@ -194,8 +194,8 @@ func Test_FindBladePosition(t *testing.T) { got, err := bmc.FindBladePosition("74XXX72") - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -214,8 +214,8 @@ func Test_PowerCycleBlade(t *testing.T) { got, err := bmc.PowerCycleBlade(2) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -234,8 +234,8 @@ func Test_ReseatBlade(t *testing.T) { got, err := bmc.ReseatBlade(2) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -254,8 +254,8 @@ func Test_PowerOnBlade(t *testing.T) { got, err := bmc.PowerOnBlade(2) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -274,8 +274,8 @@ func Test_PowerOffBlade(t *testing.T) { got, err := bmc.PowerOffBlade(2) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -294,8 +294,8 @@ func Test_IsOnBlade(t *testing.T) { got, err := bmc.IsOnBlade(2) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -314,8 +314,8 @@ func Test_PowerCycleBmcBlade(t *testing.T) { got, err := bmc.PowerCycleBmcBlade(2) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -334,8 +334,8 @@ func Test_PxeOnceBlade(t *testing.T) { got, err := bmc.PxeOnceBlade(2) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -354,8 +354,8 @@ func Test_SetIpmiOverLan(t *testing.T) { got, err := bmc.SetIpmiOverLan(2, true) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -374,8 +374,8 @@ func Test_SetDynamicPower(t *testing.T) { got, err := bmc.SetDynamicPower(true) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -394,8 +394,8 @@ func Test_SetFlexAddressState(t *testing.T) { got, err := bmc.SetFlexAddressState(1, false) - if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + if (err != nil) != wantErr { + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { diff --git a/providers/dell/m1000e/m1000e.go b/providers/dell/m1000e/m1000e.go index 72d1e336..f6e01d9b 100644 --- a/providers/dell/m1000e/m1000e.go +++ b/providers/dell/m1000e/m1000e.go @@ -45,8 +45,13 @@ type M1000e struct { } // New returns a connection to M1000e -func New(ip string, username string, password string) (chassis *M1000e, err error) { - return &M1000e{ip: ip, username: username, password: password}, err +func New(host string, username string, password string) (*M1000e, error) { + sshClient, err := sshclient.New(host, username, password) + if err != nil { + return nil, err + } + + return &M1000e{ip: host, username: username, password: password, sshClient: sshClient}, nil } // CheckCredentials verify whether the credentials are valid or not diff --git a/providers/dell/m1000e/setupConnections.go b/providers/dell/m1000e/setupConnections.go index 79950012..1d2e55f4 100644 --- a/providers/dell/m1000e/setupConnections.go +++ b/providers/dell/m1000e/setupConnections.go @@ -11,7 +11,6 @@ import ( "github.com/bmc-toolbox/bmclib/errors" "github.com/bmc-toolbox/bmclib/internal/httpclient" - "github.com/bmc-toolbox/bmclib/internal/sshclient" multierror "github.com/hashicorp/go-multierror" "github.com/bmc-toolbox/bmclib/providers/dell" @@ -128,36 +127,19 @@ func (m *M1000e) httpLogin() (err error) { return err } -// sshLogin initiates the connection to a chassis device -func (m *M1000e) sshLogin() (err error) { - if m.sshClient != nil { - return - } - - log.WithFields(log.Fields{"step": "chassis connection", "vendor": dell.VendorID, "ip": m.ip}).Debug("connecting to chassis") - m.sshClient, err = sshclient.New(m.ip, m.username, m.password) - if err != nil { - return err - } - - return err -} - // Close closes the connection properly -func (m *M1000e) Close() (err error) { +func (m *M1000e) Close() error { + var multiErr error if m.httpClient != nil { - _, e := m.httpClient.Get(fmt.Sprintf("https://%s/cgi-bin/webcgi/logout", m.ip)) - if e != nil { - err = multierror.Append(e, err) + _, err := m.httpClient.Get(fmt.Sprintf("https://%s/cgi-bin/webcgi/logout", m.ip)) + if err != nil { + multiErr = multierror.Append(multiErr, err) } } - if m.sshClient != nil { - e := m.sshClient.Close() - if e != nil { - err = multierror.Append(e, err) - } + if err := m.sshClient.Close(); err != nil { + multiErr = multierror.Append(multiErr, err) } - return err + return multiErr } diff --git a/providers/dummy/ibmc/configure.go b/providers/dummy/ibmc/configure.go index 79401944..e1dea38b 100644 --- a/providers/dummy/ibmc/configure.go +++ b/providers/dummy/ibmc/configure.go @@ -79,6 +79,6 @@ func (i *Ibmc) UploadHTTPSCert(cert []byte, certFileName string, key []byte, key // CurrentHTTPSCert returns the current x509 certficates configured on the BMC // The bool value returned indicates if the BMC supports CSR generation. // CurrentHTTPSCert implements the Configure interface. -func (i *Ibmc) CurrentHTTPSCert() (c []*x509.Certificate, b bool, e error) { - return c, b, e +func (i *Ibmc) CurrentHTTPSCert() ([]*x509.Certificate, bool, error) { + return nil, false, nil } diff --git a/providers/hp/c7000/actions.go b/providers/hp/c7000/actions.go index 1f7985db..72f47b50 100644 --- a/providers/hp/c7000/actions.go +++ b/providers/hp/c7000/actions.go @@ -6,86 +6,66 @@ import ( "strings" "github.com/bmc-toolbox/bmclib/errors" - "github.com/bmc-toolbox/bmclib/internal/sshclient" + "github.com/bmc-toolbox/bmclib/internal" ) // PowerCycle reboots the chassis -func (c *C7000) PowerCycle() (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) PowerCycle() (bool, error) { output, err := c.sshClient.Run("RESTART OA ACTIVE") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "Restarting Onboard Administrator") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOn power on the chassis -func (c *C7000) PowerOn() (status bool, err error) { - return status, errors.ErrFeatureUnavailable +func (c *C7000) PowerOn() (bool, error) { + return false, errors.ErrFeatureUnavailable } // PowerOff power off the chassis -func (c *C7000) PowerOff() (status bool, err error) { - return status, errors.ErrFeatureUnavailable +func (c *C7000) PowerOff() (bool, error) { + return false, errors.ErrFeatureUnavailable } // IsOn tells if a machine is currently powered on -func (c *C7000) IsOn() (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - - if c.sshClient != nil { - return true, err +func (c *C7000) IsOn() (bool, error) { + if c.sshClient != nil { // TODO: run "help"? + return true, nil } - return status, err + return false, nil } // FindBladePosition receives a serial and find the position of the blade using it -func (c *C7000) FindBladePosition(serial string) (position int, err error) { - err = c.sshLogin() - if err != nil { - return position, err - } - +func (c *C7000) FindBladePosition(serial string) (int, error) { output, err := c.sshClient.Run("SHOW SERVER NAMES") if err != nil { - return position, err + return -1, err } for _, line := range strings.Split(output, "\n") { line = strings.Replace(line, "Server-", "", -1) - data := strings.FieldsFunc(line, sshclient.IsntLetterOrNumber) + data := strings.FieldsFunc(line, internal.IsntLetterOrNumber) for _, field := range data { if strings.ToLower(serial) == strings.ToLower(field) { - position, err := strconv.Atoi(data[0]) - return position, err + return strconv.Atoi(data[0]) } } } - return position, fmt.Errorf("unable to find the blade in this chassis") + return -1, fmt.Errorf("unable to find the blade in this chassis") } // PowerCycleBlade reboots the machine via bmc -func (c *C7000) PowerCycleBlade(position int) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) PowerCycleBlade(position int) (bool, error) { output, err := c.sshClient.Run(fmt.Sprintf("REBOOT SERVER %d FORCE", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "currently powered off") { @@ -93,133 +73,102 @@ func (c *C7000) PowerCycleBlade(position int) (status bool, err error) { } if strings.Contains(output, "Forcing reboot of Blade") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // ReseatBlade reboots the machine via bmc -func (c *C7000) ReseatBlade(position int) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) ReseatBlade(position int) (bool, error) { output, err := c.sshClient.Run(fmt.Sprintf("RESET SERVER %d", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "Successfully") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOnBlade power on the machine via bmc -func (c *C7000) PowerOnBlade(position int) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) PowerOnBlade(position int) (bool, error) { output, err := c.sshClient.Run(fmt.Sprintf("POWERON SERVER %d", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "Powering on") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOffBlade power off the machine via bmc -func (c *C7000) PowerOffBlade(position int) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) PowerOffBlade(position int) (bool, error) { output, err := c.sshClient.Run(fmt.Sprintf("POWEROFF SERVER %d FORCE", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "powering down.") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // IsOnBlade tells if a machine is currently powered on -func (c *C7000) IsOnBlade(position int) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) IsOnBlade(position int) (bool, error) { output, err := c.sshClient.Run(fmt.Sprintf("SHOW SERVER STATUS %d", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "Power: On") { - return true, err + return true, nil } - return status, err + return false, fmt.Errorf(output) } // PowerCycleBmcBlade reboots the bmc we are connected to -func (c *C7000) PowerCycleBmcBlade(position int) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) PowerCycleBmcBlade(position int) (bool, error) { output, err := c.sshClient.Run(fmt.Sprintf("RESET ILO %d", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "Successfully") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PxeOnceBlade makes the machine to boot via pxe once -func (c *C7000) PxeOnceBlade(position int) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - - status, err = c.PowerCycleBlade(position) +func (c *C7000) PxeOnceBlade(position int) (bool, error) { + status, err := c.PowerCycleBlade(position) if err != nil { return status, err } output, err := c.sshClient.Run(fmt.Sprintf("SET SERVER BOOT ONCE PXE %d", position)) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "boot order changed to PXE") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // SetDynamicPower configure the dynamic power behaviour -func (c *C7000) SetDynamicPower(enable bool) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) SetDynamicPower(enable bool) (bool, error) { var state string if enable { state = "ON" @@ -230,40 +179,34 @@ func (c *C7000) SetDynamicPower(enable bool) (status bool, err error) { cmd := fmt.Sprintf("SET POWER SAVINGS %s", state) output, err := c.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Dynamic Power: Disabled") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // UpdateFirmware updates the chassis firmware -func (c *C7000) UpdateFirmware(source, file string) (status bool, err error) { - err = c.sshLogin() - if err != nil { - return status, err - } - +func (c *C7000) UpdateFirmware(source, file string) (bool, error) { cmd := fmt.Sprintf("update image %s/%s", source, file) output, err := c.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Flashing Active Onboard Administrator") { - return true, err + return true, nil } - return status, err + return false, fmt.Errorf(output) } // ModBladeBmcUser modfies BMC Admin user account password through the chassis, // this method will attempt to modify a user account on all BMCs in a chassis. -func (c *C7000) ModBladeBmcUser(username string, password string) (err error) { - +func (c *C7000) ModBladeBmcUser(username string, password string) error { ribcl := `HPONCFG all << end_marker @@ -279,14 +222,9 @@ end_marker` ribcl = strings.Replace(ribcl, "__USERNAME__", username, -1) ribcl = strings.Replace(ribcl, "__PASSWORD__", password, -1) - err = c.sshLogin() - if err != nil { - return err - } - output, err := c.sshClient.Run(ribcl) if err != nil { - return fmt.Errorf(output) + return fmt.Errorf("output: %q: %w", output, err) } //since there are multiple blades and this command @@ -296,13 +234,12 @@ end_marker` return fmt.Errorf(output) } - return err + return nil } // AddBladeBmcAdmin configures BMC Admin user accounts through the chassis. // this method will attempt to add the user to all BMCs in a chassis. -func (c *C7000) AddBladeBmcAdmin(username string, password string) (err error) { - +func (c *C7000) AddBladeBmcAdmin(username string, password string) error { ribcl := `HPONCFG all << end_marker @@ -325,14 +262,9 @@ end_marker` ribcl = strings.Replace(ribcl, "__USERNAME__", username, -1) ribcl = strings.Replace(ribcl, "__PASSWORD__", password, -1) - err = c.sshLogin() - if err != nil { - return err - } - output, err := c.sshClient.Run(ribcl) if err != nil { - return fmt.Errorf(output) + return fmt.Errorf("output: %q: %w", output, err) } //since there are multiple blades and this command @@ -342,12 +274,11 @@ end_marker` return fmt.Errorf(output) } - return err + return nil } // RemoveBladeBmcUser removes the user account from all BMCs through the chassis. -func (c *C7000) RemoveBladeBmcUser(username string) (err error) { - +func (c *C7000) RemoveBladeBmcUser(username string) error { ribcl := `HPONCFG all << end_marker @@ -360,14 +291,9 @@ end_marker` ribcl = strings.Replace(ribcl, "__USERNAME__", username, -1) - err = c.sshLogin() - if err != nil { - return err - } - output, err := c.sshClient.Run(ribcl) if err != nil { - return fmt.Errorf(output) + return fmt.Errorf("output: %q: %w", output, err) } //since there are multiple blades and this command @@ -377,16 +303,16 @@ end_marker` return fmt.Errorf(output) } - return err + return nil } // SetFlexAddressState Enable/Disable FlexAddress disables flex Addresses for blades // FlexAddress is a virtual addressing scheme -func (c *C7000) SetFlexAddressState(_ int, _ bool) (status bool, err error) { - return status, errors.ErrNotImplemented +func (c *C7000) SetFlexAddressState(_ int, _ bool) (bool, error) { + return false, errors.ErrNotImplemented } // SetIpmiOverLan Enable/Disable IPMI over lan parameter per blade in chassis -func (c *C7000) SetIpmiOverLan(_ int, _ bool) (status bool, err error) { - return status, errors.ErrNotImplemented +func (c *C7000) SetIpmiOverLan(_ int, _ bool) (bool, error) { + return false, errors.ErrNotImplemented } diff --git a/providers/hp/c7000/actions_test.go b/providers/hp/c7000/actions_test.go index fdcc999d..36b56175 100644 --- a/providers/hp/c7000/actions_test.go +++ b/providers/hp/c7000/actions_test.go @@ -1,6 +1,7 @@ package c7000 import ( + "github.com/bmc-toolbox/bmclib/internal/sshclient" "github.com/bmc-toolbox/bmclib/sshmock" "testing" @@ -105,10 +106,16 @@ func setupBMC() (func(), *C7000, error) { return nil, nil, err } + sshClient, err := sshclient.New(address, sshUsername, sshPassword) + if err != nil { + return nil, nil, err + } + bmc := &C7000{ - ip: address, - username: sshUsername, - password: sshPassword, + ip: address, + username: sshUsername, + password: sshPassword, + sshClient: sshClient, } return tearDown, bmc, err @@ -180,7 +187,7 @@ func Test_FindBladePosition(t *testing.T) { got, err := bmc.FindBladePosition("CZXXXXXXEK") if err != nil { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if (err != nil) != wantErr { @@ -200,7 +207,7 @@ func Test_PowerCycleBlade(t *testing.T) { got, err := bmc.PowerCycleBlade(1) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -220,7 +227,7 @@ func Test_ReseatBlade(t *testing.T) { got, err := bmc.ReseatBlade(1) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -240,7 +247,7 @@ func Test_PowerOnBlade(t *testing.T) { got, err := bmc.PowerOnBlade(1) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -260,7 +267,7 @@ func Test_PowerOffBlade(t *testing.T) { got, err := bmc.PowerOffBlade(1) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -280,7 +287,7 @@ func Test_IsOnBlade(t *testing.T) { got, err := bmc.IsOnBlade(1) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -300,7 +307,7 @@ func Test_PowerCycleBmcBlade(t *testing.T) { got, err := bmc.PowerCycleBmcBlade(1) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -320,7 +327,7 @@ func Test_PxeOnceBlade(t *testing.T) { got, err := bmc.PxeOnceBlade(1) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -340,7 +347,7 @@ func Test_SetIpmiOverLan(t *testing.T) { got, err := bmc.SetIpmiOverLan(1, true) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -360,7 +367,7 @@ func Test_SetDynamicPower(t *testing.T) { got, err := bmc.SetDynamicPower(false) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { @@ -380,11 +387,10 @@ func Test_SetFlexAddressState(t *testing.T) { got, err := bmc.SetFlexAddressState(1, false) if (err != nil) != wantErr { - t.Errorf("error = %v, wantErr %v", got, wantErr) + t.Errorf("error = %v, wantErr %v", err, wantErr) } if got != want { t.Errorf("got = %v, want %v", got, want) } } - diff --git a/providers/hp/c7000/c7000.go b/providers/hp/c7000/c7000.go index 67b8080a..21a995da 100644 --- a/providers/hp/c7000/c7000.go +++ b/providers/hp/c7000/c7000.go @@ -34,34 +34,39 @@ type C7000 struct { } // New returns a connection to C7000 -func New(ip string, username string, password string) (chassis *C7000, err error) { +func New(host string, username string, password string) (*C7000, error) { client, err := httpclient.Build() if err != nil { - return chassis, err + return nil, err } - url := fmt.Sprintf("https://%s/xmldata?item=all", ip) + url := fmt.Sprintf("https://%s/xmldata?item=all", host) resp, err := client.Get(url) if err != nil { - return chassis, err + return nil, err } payload, err := ioutil.ReadAll(resp.Body) if err != nil { - return chassis, err + return nil, err } defer resp.Body.Close() Rimp := &hp.Rimp{} err = xml.Unmarshal(payload, Rimp) if err != nil { - return chassis, err + return nil, err } if Rimp.Infra2 == nil { - return chassis, errors.ErrUnableToReadData + return nil, errors.ErrUnableToReadData + } + + sshClient, err := sshclient.New(host, username, password) + if err != nil { + return nil, err } - return &C7000{ip: ip, username: username, password: password, Rimp: Rimp}, err + return &C7000{ip: host, username: username, password: password, Rimp: Rimp, sshClient: sshClient}, nil } // CheckCredentials verify whether the credentials are valid or not diff --git a/providers/hp/c7000/setupConnections.go b/providers/hp/c7000/setupConnections.go index 2d383956..d22e69ea 100644 --- a/providers/hp/c7000/setupConnections.go +++ b/providers/hp/c7000/setupConnections.go @@ -11,9 +11,6 @@ import ( "github.com/bmc-toolbox/bmclib/errors" "github.com/bmc-toolbox/bmclib/internal/httpclient" - "github.com/bmc-toolbox/bmclib/internal/sshclient" - "github.com/bmc-toolbox/bmclib/providers/hp" - multierror "github.com/hashicorp/go-multierror" log "github.com/sirupsen/logrus" ) @@ -96,37 +93,21 @@ func (c *C7000) httpLogin() (err error) { return err } -// Login initiates the connection to a chassis device -func (c *C7000) sshLogin() (err error) { - if c.sshClient != nil { - return - } - - log.WithFields(log.Fields{"step": "chassis connection", "vendor": hp.VendorID, "ip": c.ip}).Debug("connecting to chassis") - c.sshClient, err = sshclient.New(c.ip, c.username, c.password) - if err != nil { - return err - } - - return err -} - // Close closes the connection properly -func (c *C7000) Close() (err error) { +func (c *C7000) Close() error { + var miltiErr error + if c.httpClient != nil { payload := UserLogout{} - _, _, e := c.postXML(payload) - if e != nil { - err = multierror.Append(e, err) + _, _, err := c.postXML(payload) + if err != nil { + miltiErr = multierror.Append(miltiErr, err) } } - if c.sshClient != nil { - e := c.sshClient.Close() - if e != nil { - err = multierror.Append(e, err) - } + if err := c.sshClient.Close(); err != nil { + miltiErr = multierror.Append(miltiErr, err) } - return err + return miltiErr } diff --git a/providers/hp/ilo/actions.go b/providers/hp/ilo/actions.go index 64a3e78b..1e8e6609 100644 --- a/providers/hp/ilo/actions.go +++ b/providers/hp/ilo/actions.go @@ -8,15 +8,10 @@ import ( ) // PowerCycle reboots the machine via bmc -func (i *Ilo) PowerCycle() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *Ilo) PowerCycle() (bool, error) { output, err := i.sshClient.Run("power reset") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Server power off") { @@ -24,98 +19,79 @@ func (i *Ilo) PowerCycle() (status bool, err error) { } if strings.Contains(output, "Server resetting") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerCycleBmc reboots the bmc we are connected to -func (i *Ilo) PowerCycleBmc() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *Ilo) PowerCycleBmc() (bool, error) { output, err := i.sshClient.Run("reset /map1") - if err != nil && !strings.Contains(output, "Resetting iLO") { - return false, fmt.Errorf(output) + if err != nil { + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Resetting iLO") { return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOn power on the machine via bmc -func (i *Ilo) PowerOn() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *Ilo) PowerOn() (bool, error) { output, err := i.sshClient.Run("power on") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "Server powering on") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PowerOff power off the machine via bmc -func (i *Ilo) PowerOff() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *Ilo) PowerOff() (bool, error) { output, err := i.sshClient.Run("power off hard") if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } + if strings.Contains(output, "Forcing server") { - return true, err + return true, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // PxeOnce makes the machine to boot via pxe once -func (i *Ilo) PxeOnce() (status bool, err error) { +func (i *Ilo) PxeOnce() (bool, error) { im, err := ipmi.New(i.username, i.password, i.ip) if err != nil { - return status, err + return false, err } // PXE using uefi, does't work for some models // directly. It only works if you pxe, powercycle and // power on. - status, err = im.PxeOnceEfi() - if err != nil { + if _, err = im.PxeOnceEfi(); err != nil { return false, err } - status, err = im.PowerCycle() - if err != nil { + + if _, err := im.PowerCycle(); err != nil { return false, err } - im.PowerOnForce() - return status, err + + return im.PowerOnForce() } // IsOn tells if a machine is currently powered on -func (i *Ilo) IsOn() (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *Ilo) IsOn() (bool, error) { output, err := i.sshClient.Run("power") if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "currently: On") { @@ -123,28 +99,23 @@ func (i *Ilo) IsOn() (status bool, err error) { } if strings.Contains(output, "currently: Off") { - return true, nil + return false, nil } - return status, fmt.Errorf(output) + return false, fmt.Errorf(output) } // UpdateFirmware updates the bmc firmware -func (i *Ilo) UpdateFirmware(source, file string) (status bool, err error) { - err = i.sshLogin() - if err != nil { - return status, err - } - +func (i *Ilo) UpdateFirmware(source, file string) (bool, error) { cmd := fmt.Sprintf("load /map1/firmware1 -source %s/%s", source, file) output, err := i.sshClient.Run(cmd) if err != nil { - return false, fmt.Errorf(output) + return false, fmt.Errorf("output: %q: %w", output, err) } if strings.Contains(output, "Resetting iLO") { - return true, err + return true, nil } - return status, err + return false, fmt.Errorf(output) } diff --git a/providers/hp/ilo/actions_test.go b/providers/hp/ilo/actions_test.go index ee59f556..26028506 100644 --- a/providers/hp/ilo/actions_test.go +++ b/providers/hp/ilo/actions_test.go @@ -1,6 +1,7 @@ package ilo import ( + "github.com/bmc-toolbox/bmclib/internal/sshclient" "github.com/bmc-toolbox/bmclib/sshmock" "testing" @@ -34,10 +35,16 @@ func setupBMC() (func(), *Ilo, error) { return nil, nil, err } + sshClient, err := sshclient.New(address, sshUsername, sshPassword) + if err != nil { + return nil, nil, err + } + bmc := &Ilo{ - ip: address, - username: sshUsername, - password: sshPassword, + ip: address, + username: sshUsername, + password: sshPassword, + sshClient: sshClient, } return tearDown, bmc, err diff --git a/providers/hp/ilo/ilo.go b/providers/hp/ilo/ilo.go index 3841a689..e4bcf1e2 100644 --- a/providers/hp/ilo/ilo.go +++ b/providers/hp/ilo/ilo.go @@ -49,36 +49,49 @@ type Ilo struct { } // New returns a new Ilo ready to be used -func New(ip string, username string, password string) (ilo *Ilo, err error) { - loginURL, err := url.Parse(fmt.Sprintf("https://%s/json/login_session", ip)) +func New(host string, username string, password string) (*Ilo, error) { + loginURL, err := url.Parse(fmt.Sprintf("https://%s/json/login_session", host)) if err != nil { return nil, err } client, err := httpclient.Build() if err != nil { - return ilo, err + return nil, err } - xmlURL := fmt.Sprintf("https://%s/xmldata?item=all", ip) + xmlURL := fmt.Sprintf("https://%s/xmldata?item=all", host) resp, err := client.Get(xmlURL) if err != nil { - return ilo, err + return nil, err } payload, err := ioutil.ReadAll(resp.Body) if err != nil { - return ilo, err + return nil, err } defer resp.Body.Close() rimpBlade := &hp.RimpBlade{} err = xml.Unmarshal(payload, rimpBlade) if err != nil { - return ilo, err + return nil, err } - return &Ilo{ip: ip, username: username, password: password, loginURL: loginURL, rimpBlade: rimpBlade}, err + sshClient, err := sshclient.New(host, username, password) + if err != nil { + return nil, err + } + + ilo := &Ilo{ + ip: host, + username: username, + password: password, + loginURL: loginURL, + rimpBlade: rimpBlade, + sshClient: sshClient, + } + return ilo, nil } // CheckCredentials verify whether the credentials are valid or not diff --git a/providers/hp/ilo/setupConnection.go b/providers/hp/ilo/setupConnection.go index 5b61d1dc..c300ae7d 100644 --- a/providers/hp/ilo/setupConnection.go +++ b/providers/hp/ilo/setupConnection.go @@ -12,7 +12,6 @@ import ( "github.com/bmc-toolbox/bmclib/errors" "github.com/bmc-toolbox/bmclib/internal/httpclient" - "github.com/bmc-toolbox/bmclib/internal/sshclient" "github.com/bmc-toolbox/bmclib/providers/hp" multierror "github.com/hashicorp/go-multierror" @@ -102,33 +101,19 @@ func (i *Ilo) httpLogin() (err error) { return err } -// Login initiates the connection to a bmc device -func (i *Ilo) sshLogin() (err error) { - if i.sshClient != nil { - return - } - - log.WithFields(log.Fields{"step": "bmc connection", "vendor": hp.VendorID, "ip": i.ip}).Debug("connecting to bmc") - i.sshClient, err = sshclient.New(i.ip, i.username, i.password) - if err != nil { - return err - } - - return err -} - // Close closes the connection properly -func (i *Ilo) Close() (err error) { +func (i *Ilo) Close() error { + var multiErr error + if i.httpClient != nil { log.WithFields(log.Fields{"step": "bmc connection", "vendor": hp.VendorID, "ip": i.ip}).Debug("logout from bmc http") data := []byte(fmt.Sprintf(`{"method":"logout", "session_key": "%s"}`, i.sessionKey)) - req, e := http.NewRequest("POST", i.loginURL.String(), bytes.NewBuffer(data)) - if e != nil { - err = multierror.Append(e, err) + req, err := http.NewRequest("POST", i.loginURL.String(), bytes.NewBuffer(data)) + if err != nil { + multiErr = multierror.Append(multiErr, err) } else { - req.Header.Set("Content-Type", "application/json") if log.GetLevel() == log.TraceLevel { @@ -141,13 +126,13 @@ func (i *Ilo) Close() (err error) { } } - resp, e := i.httpClient.Do(req) - if e != nil { - err = multierror.Append(e, err) + resp, err := i.httpClient.Do(req) + if err != nil { + multiErr = multierror.Append(multiErr, err) } else { defer resp.Body.Close() - defer io.Copy(ioutil.Discard, resp.Body) + if log.GetLevel() == log.TraceLevel { dump, err := httputil.DumpResponse(resp, true) if err == nil { @@ -162,14 +147,9 @@ func (i *Ilo) Close() (err error) { } } - if i.sshClient != nil { - log.WithFields(log.Fields{"step": "bmc connection", "vendor": hp.VendorID, "ip": i.ip}).Debug("logout from bmc ssh") - - e := i.sshClient.Close() - if e != nil { - err = multierror.Append(e, err) - } + if err := i.sshClient.Close(); err != nil { + multiErr = multierror.Append(multiErr, err) } - return err + return multiErr } diff --git a/sshmock/sshmock_test.go b/sshmock/sshmock_test.go index 87f3a9c8..b1448e32 100644 --- a/sshmock/sshmock_test.go +++ b/sshmock/sshmock_test.go @@ -9,12 +9,16 @@ import ( func Test_Server(t *testing.T) { expectedAnswer := "world" command := "hello" - answers := map[string][]byte{command: []byte(expectedAnswer)} + answers := map[string][]byte{ + command: []byte(expectedAnswer), + "exit": []byte("see you"), + } s, err := New(answers) if err != nil { t.Fatalf(err.Error()) } + shutdown, address, err := s.ListenAndServe() if err != nil { t.Fatalf(err.Error()) @@ -23,15 +27,19 @@ func Test_Server(t *testing.T) { sshClient, err := sshclient.New(address, "super", "test") if err != nil { - t.Fatalf("unable to connect to ssh server %s", err.Error()) + t.Fatalf("unable to connect to ssh server: %v", err) } answer, err := sshClient.Run(command) if err != nil { - t.Fatalf("unable to run command %s: %s", command, err.Error()) + t.Fatalf("unable to run command %s: %v", command, err) } if answer != expectedAnswer { - t.Errorf("Expected answer %v: found %v", expectedAnswer, answer) + t.Errorf("expected answer %v: found %v", expectedAnswer, answer) + } + + if err := sshClient.Close(); err != nil { + t.Errorf("Close() returns an error:%v", err) } }