Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/openshift-install/gather: Recognize "connection refused" #3615

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions cmd/openshift-install/gather.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"
"strconv"
"strings"
"syscall"
"time"

configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -142,11 +143,13 @@ func runGatherBootstrapCmd(directory string) error {
func logGatherBootstrap(bootstrap string, port int, masters []string, directory string) error {
logrus.Info("Pulling debug logs from the bootstrap machine")
client, err := ssh.NewClient("core", net.JoinHostPort(bootstrap, strconv.Itoa(port)), gatherBootstrapOpts.sshKeys)
if err != nil && strings.Contains(err.Error(), "ssh: handshake failed: ssh: unable to authenticate") {
return errors.Wrap(err, "failed to create SSH client, ensure the private key is added to your authentication agent (ssh-agent) or specified with the --key parameter")
} else if err != nil {
if err != nil {
if errors.Is(err, syscall.ECONNREFUSED) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this entire block https://github.com/openshift/installer/pull/3615/files#diff-be4790dcfa83ca14e7942b4c3b50c5f9R146-R156 has multi if else for err, can we standardize to using errors.Is for connectionrefused and unable to authenitcate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried checking for the root error type for the unable to authenticate scenario but the root error is fmt.Errorf which is a basic string error. Not sure if errors.Is will work for that scenario. I checked their test files for the function and the strings.Contains function is being used. Will work to make the code a little cleaner.

return errors.Wrap(err, "failed to connect to the bootstrap machine")
}
return errors.Wrap(err, "failed to create SSH client")
}

gatherID := time.Now().Format("20060102150405")
if err := ssh.Run(client, fmt.Sprintf("/usr/local/bin/installer-gather.sh --id %s %s", gatherID, strings.Join(masters, " "))); err != nil {
return errors.Wrap(err, "failed to run remote command")
Expand Down
20 changes: 8 additions & 12 deletions pkg/gather/ssh/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ import (

// getAgent attempts to connect to the running SSH agent, returning a newly
// initialized static agent if that fails.
func getAgent(keys []string) (agent.Agent, error) {
// Attempt to use the existing SSH agent if it's configured and no keys
// were explicitly passed.
if authSock := os.Getenv("SSH_AUTH_SOCK"); authSock != "" && len(keys) == 0 {
func getAgent(keys []string) (agent.Agent, string, error) {
// Attempt to use the existing SSH agent if it's configured or use the default ssh pair generated.
if authSock := os.Getenv("SSH_AUTH_SOCK"); authSock != "" {
logrus.Debugf("Using SSH_AUTH_SOCK %s to connect to an existing agent", authSock)
if conn, err := net.Dial("unix", authSock); err == nil {
return agent.NewClient(conn), nil
return agent.NewClient(conn), "agent", nil
}
}

Expand All @@ -28,13 +27,10 @@ func getAgent(keys []string) (agent.Agent, error) {

// newAgent initializes an SSH Agent with the keys.
// If no keys are provided, it loads all the keys from the user's environment.
func newAgent(keyPaths []string) (agent.Agent, error) {
func newAgent(keyPaths []string) (agent.Agent, string, error) {
keys, err := loadKeys(keyPaths)
if err != nil {
return nil, err
}
if len(keys) == 0 {
return nil, errors.New("no keys found for SSH agent")
return nil, "", err
}

ag := agent.NewKeyring()
Expand All @@ -46,9 +42,9 @@ func newAgent(keyPaths []string) (agent.Agent, error) {
logrus.Debugf("Added %s to installer's internal agent", name)
}
if agg := utilerrors.NewAggregate(errs); agg != nil {
return nil, agg
return nil, "", agg
}
return ag, nil
return ag, "keys", nil
}

func loadKeys(paths []string) (map[string]interface{}, error) {
Expand Down
9 changes: 8 additions & 1 deletion pkg/gather/ssh/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strings"

"github.com/openshift/installer/pkg/lineprinter"
"github.com/pkg/errors"
Expand All @@ -20,7 +21,7 @@ import (
//
// if keys list is empty, it tries to load the keys from the user's environment.
func NewClient(user, address string, keys []string) (*ssh.Client, error) {
ag, err := getAgent(keys)
ag, agentType, err := getAgent(keys)
if err != nil {
return nil, errors.Wrap(err, "failed to initialize the SSH agent")
}
Expand All @@ -36,6 +37,12 @@ func NewClient(user, address string, keys []string) (*ssh.Client, error) {
HostKeyCallback: ssh.InsecureIgnoreHostKey(),
})
if err != nil {
if strings.Contains(err.Error(), "ssh: handshake failed: ssh: unable to authenticate") {
if agentType == "agent" {
return nil, errors.Wrap(err, "failed to use pre-existing agent, make sure the appropriate keys exist in the agent for authentication")
}
return nil, errors.Wrap(err, "failed to use the provided keys for authentication")
}
return nil, err
}
if err := agent.ForwardToAgent(client, ag); err != nil {
Expand Down