Add connection-port to provider diskimage

The connection port should be included in the privider diskimage.
This makes it possible to define images using other ports for
connections winrm for Windows which run on a different port than 22.

Change-Id: Ib4b335ffbcc4dc71704c06387377675a4206c663

doc/source/configuration.rst

@@ -262,7 +262,7 @@ Example configuration::
     When set to True, nodepool-builder will not build the diskimage.
 
   ``username`` (string)
-    The username that a consumer should use when connecting onto the node. Defaults
+    The username that a consumer should use when connecting to the node. Defaults
     to ``zuul``.
 
 .. _provider:
@@ -536,6 +536,7 @@ Example configuration::
           key2: value
     - name: windows
       connection-type: winrm
+      connection-port: 5986
 
 **required**
 
@@ -559,10 +560,14 @@ Example configuration::
     values must be 255 characters or less.
 
   ``connection-type`` (string)
-    The connection type that a consumer should use when connecting onto the
+    The connection type that a consumer should use when connecting to the
     node. For most diskimages this is not necessary. However when creating
     Windows images this could be 'winrm' to enable access via ansible.
 
+  ``connection-port`` (int)
+    The port that a consumer should use when connecting to the
+    node. For most diskimages this is not necessary. This defaults to 22 for
+    ssh and 5986 for winrm.
 
 .. _provider_cloud_images:
 
@@ -580,6 +585,7 @@ Example configuration::
       config-drive: False
     - name: windows-external
       connection-type: winrm
+      connection-port: 5986
 
 **required**
 
@@ -608,13 +614,18 @@ Example configuration::
     provider by this name or ID.  Mutually exclusive with ``image-id``.
 
   ``username`` (str)
-    The username that a consumer should use when connecting onto the node.
+    The username that a consumer should use when connecting to the node.
 
   ``connection-type`` (str)
-    The connection type that a consumer should use when connecting onto the
+    The connection type that a consumer should use when connecting to the
     node. For most diskimages this is not necessary. However when creating
     Windows images this could be 'winrm' to enable access via ansible.
 
+  ``connection-port`` (int)
+    The port that a consumer should use when connecting to the
+    node. For most diskimages this is not necessary. This defaults to 22
+    for ssh and 5986 for winrm.
+
 .. _pool_labels:
 
 labels

nodepool/driver/openstack/config.py

@@ -125,6 +125,12 @@ class OpenStackProviderConfig(ProviderConfig):
             'image-name-format',
             '{image_name}-{timestamp}'
         )
+
+        default_port_mapping = {
+            'ssh': 22,
+            'winrm': 5986,
+        }
+
         self.diskimages = {}
         for image in self.provider.get('diskimages', []):
             i = ProviderDiskImage()
@@ -135,6 +141,9 @@ class OpenStackProviderConfig(ProviderConfig):
             i.pause = bool(image.get('pause', False))
             i.config_drive = image.get('config-drive', None)
             i.connection_type = image.get('connection-type', 'ssh')
+            i.connection_port = image.get(
+                'connection-port',
+                default_port_mapping.get(i.connection_type, 22))
 
             # This dict is expanded and used as custom properties when
             # the image is uploaded.
@@ -159,6 +168,9 @@ class OpenStackProviderConfig(ProviderConfig):
             i.image_name = image.get('image-name', None)
             i.username = image.get('username', None)
             i.connection_type = image.get('connection-type', 'ssh')
+            i.connection_port = image.get(
+                'connection-port',
+                default_port_mapping.get(i.connection_type, 22))
             self.cloud_images[i.name] = i
 
         self.pools = {}
@@ -214,12 +226,14 @@ class OpenStackProviderConfig(ProviderConfig):
             'meta': dict,
             'config-drive': bool,
             'connection-type': str,
+            'connection-port': int,
         }
 
         provider_cloud_images = {
             'name': str,
             'config-drive': bool,
             'connection-type': str,
+            'connection-port': int,
             v.Exclusive('image-id', 'cloud-image-name-or-id'): str,
             v.Exclusive('image-name', 'cloud-image-name-or-id'): str,
             'username': str,

nodepool/driver/openstack/handler.py

@@ -97,6 +97,7 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
             image_name = self._diskimage.name
             username = cloud_image.username
             connection_type = self._diskimage.connection_type
+            connection_port = self._diskimage.connection_port
 
         else:
             # launch using unmanaged cloud image
@@ -107,6 +108,7 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
             image_name = self._label.cloud_image.name
             username = self._label.cloud_image.username
             connection_type = self._label.cloud_image.connection_type
+            connection_port = self._label.cloud_image.connection_port
 
         hostname = self._provider_config.hostname_format.format(
             label=self._label, provider=self._provider_config, node=self._node
@@ -144,6 +146,7 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
         if username:
             self._node.username = username
         self._node.connection_type = connection_type
+        self._node.connection_port = connection_port
 
         # Checkpoint save the updated node info
         self._zk.storeNode(self._node)
@@ -205,7 +208,8 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
                 host_keys = utils.nodescan(
                     interface_ip,
                     timeout=self._provider_config.boot_timeout,
-                    gather_hostkeys=gather_host_keys)
+                    gather_hostkeys=gather_host_keys,
+                    port=connection_port)
 
                 if gather_host_keys and not host_keys:
                     raise exceptions.LaunchKeyscanException(

nodepool/nodeutils.py

@@ -92,7 +92,7 @@ def nodescan(ip, port=22, timeout=60, gather_hostkeys=True):
         except socket.error as e:
             if e.errno not in [errno.ECONNREFUSED, errno.EHOSTUNREACH, None]:
                 log.exception(
-                    'Exception with ssh access to %s:' % ip)
+                    'Exception connecting to %s on port %s:' % (ip, port))
         except Exception as e:
             log.exception("ssh-keyscan failure: %s", e)
         finally:

nodepool/tests/fixtures/config_validate/good.yaml

@@ -57,12 +57,14 @@ providers:
       - name: trusty
         pause: False
         connection-type: ssh
+        connection-port: 22
     cloud-images:
       - name: trusty-unmanaged
         config-drive: true
       - name: windows-unmanaged
         username: winzuul
         connection-type: winrm
+        connection-port: 5986
     pools:
       - name: main
         max-servers: 184

nodepool/tests/fixtures/node_unmanaged_image.yaml

@@ -12,6 +12,8 @@ labels:
     min-ready: 1
   - name: fake-label-windows
     min-ready: 1
+  - name: fake-label-arbitrary-port
+    min-ready: 1
 
 providers:
   - name: fake-provider
@@ -24,6 +26,10 @@ providers:
       - name: fake-image-windows
         username: zuul
         connection-type: winrm
+      - name: fake-image-windows-port
+        username: zuul
+        connection-type: winrm
+        connection-port: 1234
     pools:
       - name: main
         max-servers: 96
@@ -34,3 +40,6 @@ providers:
           - name: fake-label-windows
             cloud-image: fake-image-windows
             min-ram: 8192
+          - name: fake-label-arbitrary-port
+            cloud-image: fake-image-windows-port
+            min-ram: 8192

nodepool/tests/test_launcher.py

@@ -60,6 +60,7 @@ class TestLauncher(tests.DBTestCase):
             self.assertEqual(node.az, "az1")
             self.assertEqual(node.username, "zuul")
             self.assertEqual(node.connection_type, 'ssh')
+            self.assertEqual(node.connection_port, 22)
             p = "{path}/{id}".format(
                 path=self.zk._imageUploadPath(image.image_name,
                                               image.build_id,
@@ -940,6 +941,7 @@ class TestLauncher(tests.DBTestCase):
         manager = pool.getProviderManager('fake-provider')
         manager._client.create_image(name="fake-image")
         manager._client.create_image(name="fake-image-windows")
+        manager._client.create_image(name="fake-image-windows-port")
 
         nodes = self.waitForNodes('fake-label')
         self.assertEqual(len(nodes), 1)
@@ -949,6 +951,14 @@ class TestLauncher(tests.DBTestCase):
         self.assertEqual(len(nodes), 1)
         self.assertEqual('zuul', nodes[0].username)
         self.assertEqual('winrm', nodes[0].connection_type)
+        self.assertEqual(5986, nodes[0].connection_port)
+        self.assertEqual(nodes[0].host_keys, [])
+
+        nodes = self.waitForNodes('fake-label-arbitrary-port')
+        self.assertEqual(len(nodes), 1)
+        self.assertEqual('zuul', nodes[0].username)
+        self.assertEqual('winrm', nodes[0].connection_type)
+        self.assertEqual(1234, nodes[0].connection_port)
         self.assertEqual(nodes[0].host_keys, [])
 
     def test_unmanaged_image_provider_name(self):

nodepool/zk.py

@@ -534,6 +534,7 @@ class Node(BaseModel):
                     self.hold_job == other.hold_job and
                     self.username == other.username and
                     self.connection_type == other.connection_type and
+                    self.connection_port == other.connection_port and
                     self.host_keys == other.host_keys and
                     self.hold_expiration == other.hold_expiration)
         else:
@@ -569,6 +570,7 @@ class Node(BaseModel):
         d['host_keys'] = self.host_keys
         d['username'] = self.username
         d['connection_type'] = self.connection_type
+        d['connection_port'] = self.connection_port
         d['hold_expiration'] = self.hold_expiration
         return d
 

releasenotes/notes/diskimage-connection-port-f53b0a9c910cb393.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    The connection port can now be configured in the provider diskimages
+    section.