Commit d92ee41e authored by Walter Fender's avatar Walter Fender

Fix issue where missing external IP address breaks SSH Tunnel.

Added unit test to cover missing external IP case. Justin's feedback. Lint fix. Lavalmap's feedback.
parent 05183bff
...@@ -444,15 +444,26 @@ func (n nodeAddressProvider) externalAddresses() ([]string, error) { ...@@ -444,15 +444,26 @@ func (n nodeAddressProvider) externalAddresses() ([]string, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
var matchErr error
addrs := []string{} addrs := []string{}
for ix := range nodes.Items { for ix := range nodes.Items {
node := &nodes.Items[ix] node := &nodes.Items[ix]
addr, err := nodeutil.GetPreferredNodeAddress(node, preferredAddressTypes) addr, err := nodeutil.GetPreferredNodeAddress(node, preferredAddressTypes)
if err != nil { if err != nil {
if _, ok := err.(*nodeutil.NoMatchError); ok {
matchErr = err
continue
}
return nil, err return nil, err
} }
addrs = append(addrs, addr) addrs = append(addrs, addr)
} }
if len(addrs) == 0 && matchErr != nil {
// We only return an error if we have items.
// Currently we return empty list/no error if Items is empty.
// We do this for backward compatibility reasons.
return nil, matchErr
}
return addrs, nil return addrs, nil
} }
......
...@@ -278,6 +278,22 @@ func TestGetNodeAddresses(t *testing.T) { ...@@ -278,6 +278,22 @@ func TestGetNodeAddresses(t *testing.T) {
assert.Equal([]string{"127.0.0.1", "127.0.0.1"}, addrs) assert.Equal([]string{"127.0.0.1", "127.0.0.1"}, addrs)
} }
func TestGetNodeAddressesWithOnlySomeExternalIP(t *testing.T) {
assert := assert.New(t)
fakeNodeClient := fake.NewSimpleClientset(makeNodeList([]string{"node1", "node2", "node3"}, apiv1.NodeResources{})).Core().Nodes()
addressProvider := nodeAddressProvider{fakeNodeClient}
// Pass case with 1 External type IP (index == 1) and nodes (indexes 0 & 2) have no External IP.
nodes, _ := fakeNodeClient.List(metav1.ListOptions{})
nodes.Items[1].Status.Addresses = []apiv1.NodeAddress{{Type: apiv1.NodeExternalIP, Address: "127.0.0.1"}}
fakeNodeClient.Update(&nodes.Items[1])
addrs, err := addressProvider.externalAddresses()
assert.NoError(err, "addresses should not have returned an error.")
assert.Equal([]string{"127.0.0.1"}, addrs)
}
func decodeResponse(resp *http.Response, obj interface{}) error { func decodeResponse(resp *http.Response, obj interface{}) error {
defer resp.Body.Close() defer resp.Body.Close()
......
...@@ -64,6 +64,17 @@ func GetHostname(hostnameOverride string) (string, error) { ...@@ -64,6 +64,17 @@ func GetHostname(hostnameOverride string) (string, error) {
return strings.ToLower(hostName), nil return strings.ToLower(hostName), nil
} }
// NoMatchError is a typed implementation of the error interface. It indicates a failure to get a matching Node.
type NoMatchError struct {
addresses []v1.NodeAddress
}
// Error is the implementation of the conventional interface for
// representing an error condition, with the nil value representing no error.
func (e *NoMatchError) Error() string {
return fmt.Sprintf("no preferred addresses found; known addresses: %v", e.addresses)
}
// GetPreferredNodeAddress returns the address of the provided node, using the provided preference order. // GetPreferredNodeAddress returns the address of the provided node, using the provided preference order.
// If none of the preferred address types are found, an error is returned. // If none of the preferred address types are found, an error is returned.
func GetPreferredNodeAddress(node *v1.Node, preferredAddressTypes []v1.NodeAddressType) (string, error) { func GetPreferredNodeAddress(node *v1.Node, preferredAddressTypes []v1.NodeAddressType) (string, error) {
...@@ -74,7 +85,7 @@ func GetPreferredNodeAddress(node *v1.Node, preferredAddressTypes []v1.NodeAddre ...@@ -74,7 +85,7 @@ func GetPreferredNodeAddress(node *v1.Node, preferredAddressTypes []v1.NodeAddre
} }
} }
} }
return "", fmt.Errorf("no preferred addresses found; known addresses: %v", node.Status.Addresses) return "", &NoMatchError{addresses: node.Status.Addresses}
} }
// GetNodeHostIP returns the provided node's IP, based on the priority: // GetNodeHostIP returns the provided node's IP, based on the priority:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment