From 0c84b7fa4e991f1bd7c1c32964b67f0b6f674f3e Mon Sep 17 00:00:00 2001 From: Albin Vass Date: Fri, 12 Feb 2021 13:22:14 +0100 Subject: [PATCH] Add shell-type config Ansible needs to know which shell type the node uses to operate correctly, especially for ssh connections for windows nodes because otherwise ansible defaults to trying bash. Change-Id: I71abfefa57aaafd88f199be19ee7caa64efda538 --- doc/source/aws.rst | 11 ++++++++ doc/source/configuration.rst | 11 ++++++++ doc/source/gce.rst | 11 ++++++++ doc/source/kubernetes.rst | 11 ++++++++ doc/source/openshift-pods.rst | 11 ++++++++ doc/source/openshift.rst | 11 ++++++++ doc/source/openstack.rst | 11 ++++++++ doc/source/static.rst | 11 ++++++++ nodepool/builder.py | 9 +++++-- nodepool/config.py | 5 ++++ nodepool/driver/aws/config.py | 3 +++ nodepool/driver/aws/handler.py | 1 + nodepool/driver/gce/config.py | 6 ++++- nodepool/driver/kubernetes/config.py | 3 +++ nodepool/driver/kubernetes/handler.py | 1 + nodepool/driver/openshift/config.py | 3 +++ nodepool/driver/openshift/handler.py | 1 + nodepool/driver/openshiftpods/config.py | 1 + nodepool/driver/openshiftpods/handler.py | 1 + nodepool/driver/openstack/config.py | 4 +++ nodepool/driver/openstack/handler.py | 3 +++ nodepool/driver/simple.py | 1 + nodepool/driver/static/config.py | 2 ++ nodepool/driver/static/provider.py | 7 +++-- nodepool/tests/fixtures/aws.yaml | 17 ++++++++++++ .../tests/fixtures/config_validate/good.yaml | 12 +++++++++ .../tests/fixtures/node_unmanaged_image.yaml | 1 + nodepool/tests/fixtures/openshift.yaml | 1 + .../tests/fixtures/static-shell-type.yaml | 26 +++++++++++++++++++ nodepool/tests/unit/test_driver_aws.py | 10 ++++++- nodepool/tests/unit/test_driver_azure.py | 1 + nodepool/tests/unit/test_driver_openshift.py | 3 +++ nodepool/tests/unit/test_driver_static.py | 21 +++++++++++++++ nodepool/tests/unit/test_launcher.py | 4 +++ nodepool/zk.py | 15 +++++++++-- 35 files changed, 242 insertions(+), 8 deletions(-) create mode 100644 nodepool/tests/fixtures/static-shell-type.yaml diff --git a/doc/source/aws.rst b/doc/source/aws.rst index 432459c6e..1d760bad2 100644 --- a/doc/source/aws.rst +++ b/doc/source/aws.rst @@ -189,6 +189,17 @@ section of the configuration. most diskimages this is not necessary. This defaults to 22 for ssh and 5986 for winrm. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g + ``csh`` or ``fish``. For a windows image with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: pools :type: list diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 3e13ceecd..fbdb76f9a 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -401,6 +401,17 @@ Options interpreter on Ansible >=2.8, and default to ``/usr/bin/python2`` for earlier versions. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g. + ``csh`` or ``fish``. For a windows image with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: dib-cmd :type: string :default: disk-image-create diff --git a/doc/source/gce.rst b/doc/source/gce.rst index 8bdbd758d..70d61966f 100644 --- a/doc/source/gce.rst +++ b/doc/source/gce.rst @@ -153,6 +153,17 @@ section of the configuration. interpreter on Ansible >=2.8, and default to ``/usr/bin/python2`` for earlier versions. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g. + ``csh`` or ``fish``. For a windows image with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: connection-type :type: str diff --git a/doc/source/kubernetes.rst b/doc/source/kubernetes.rst index 243677c9f..0c1abda1f 100644 --- a/doc/source/kubernetes.rst +++ b/doc/source/kubernetes.rst @@ -130,6 +130,17 @@ Selecting the kubernetes driver adds the following options to the interpreter on Ansible >=2.8, and default to ``/usr/bin/python2`` for earlier versions. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g. + ``csh`` or ``fish``. For a windows image with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: cpu :type: int diff --git a/doc/source/openshift-pods.rst b/doc/source/openshift-pods.rst index c65e31fee..d5486588c 100644 --- a/doc/source/openshift-pods.rst +++ b/doc/source/openshift-pods.rst @@ -111,6 +111,17 @@ Selecting the openshift pods driver adds the following options to the interpreter on Ansible >=2.8, and default to ``/usr/bin/python2`` for earlier versions. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g + ``csh`` or ``fish``. For a windows node with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: env :type: list :default: [] diff --git a/doc/source/openshift.rst b/doc/source/openshift.rst index 28e5bf651..33d548332 100644 --- a/doc/source/openshift.rst +++ b/doc/source/openshift.rst @@ -138,6 +138,17 @@ Selecting the openshift driver adds the following options to the interpreter on Ansible >=2.8, and default to ``/usr/bin/python2`` for earlier versions. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g + ``csh`` or ``fish``. For a windows image with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: cpu :type: int diff --git a/doc/source/openstack.rst b/doc/source/openstack.rst index e4cc984b8..42b0f5801 100644 --- a/doc/source/openstack.rst +++ b/doc/source/openstack.rst @@ -347,6 +347,17 @@ Selecting the OpenStack driver adds the following options to the interpreter on Ansible >=2.8, and default to ``/usr/bin/python2`` for earlier versions. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g + ``csh`` or ``fish``. For a windows image with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: connection-type :type: str diff --git a/doc/source/static.rst b/doc/source/static.rst index 93056b592..a6fa002bd 100644 --- a/doc/source/static.rst +++ b/doc/source/static.rst @@ -145,6 +145,17 @@ Selecting the static driver adds the following options to the interpreter on Ansible >=2.8, and default to ``/usr/bin/python2`` for earlier versions. + .. attr:: shell-type + :type: str + :default: sh + + The shell type of the node's default shell executable. Used by Zuul + to set ``ansible_shell_type``. This setting should not be used + unless the default shell is a non-Bourne (sh) compatible shell, e.g + ``csh`` or ``fish``. For a windows node with the experimental + `connection-type` ``ssh``, ``cmd`` or ``powershell`` should be set + and reflect the node's ``DefaultShell`` configuration. + .. attr:: max-parallel-jobs :type: int :default: 1 diff --git a/nodepool/builder.py b/nodepool/builder.py index 017166d25..d1a48f736 100644 --- a/nodepool/builder.py +++ b/nodepool/builder.py @@ -938,6 +938,7 @@ class BuildWorker(BaseWorker): build_data.builder = self._hostname build_data.username = diskimage.username build_data.python_path = diskimage.python_path + build_data.shell_type = diskimage.shell_type if self._statsd: pipeline = self._statsd.pipeline() @@ -1045,7 +1046,7 @@ class UploadWorker(BaseWorker): self._config = new_config def _uploadImage(self, build_id, upload_id, image_name, images, provider, - username, python_path): + username, python_path, shell_type): ''' Upload a local DIB image build to a provider. @@ -1057,6 +1058,7 @@ class UploadWorker(BaseWorker): :param provider: The provider from the parsed config file. :param username: :param python_path: + :param shell_type: ''' start_time = time.time() timestamp = int(start_time) @@ -1166,6 +1168,7 @@ class UploadWorker(BaseWorker): data.format = image.extension data.username = username data.python_path = python_path + data.shell_type = shell_type return data @@ -1264,13 +1267,15 @@ class UploadWorker(BaseWorker): data.state = zk.UPLOADING data.username = build.username data.python_path = build.python_path + data.shell_type = build.shell_type upnum = self._zk.storeImageUpload( image.name, build.id, provider.name, data) data = self._uploadImage(build.id, upnum, image.name, local_images, provider, - build.username, build.python_path) + build.username, build.python_path, + build.shell_type) # Set final state self._zk.storeImageUpload(image.name, build.id, diff --git a/nodepool/config.py b/nodepool/config.py index 46f4c69ea..8e5a7c21c 100644 --- a/nodepool/config.py +++ b/nodepool/config.py @@ -210,6 +210,7 @@ class DiskImage(ConfigValue): self.image_types = set([]) self.pause = False self.python_path = 'auto' + self.shell_type = None self.rebuild_age = self.REBUILD_AGE self.release = '' self.username = 'zuul' @@ -249,6 +250,9 @@ class DiskImage(ConfigValue): python_path = config.get('python-path', None) if python_path: self.python_path = python_path + shell_type = config.get('shell-type', None) + if shell_type: + self.shell_type = shell_type rebuild_age = config.get('rebuild-age', None) if rebuild_age: self.rebuild_age = rebuild_age @@ -269,6 +273,7 @@ class DiskImage(ConfigValue): other.image_types == self.image_types and other.pause == self.pause and other.python_path == self.python_path and + other.shell_type == self.shell_type and other.rebuild_age == self.rebuild_age and other.release == self.release and other.username == self.username) diff --git a/nodepool/driver/aws/config.py b/nodepool/driver/aws/config.py index cb586fe8e..a33a72240 100644 --- a/nodepool/driver/aws/config.py +++ b/nodepool/driver/aws/config.py @@ -35,6 +35,7 @@ class ProviderCloudImage(ConfigValue): and self.image_id == other.image_id and self.username == other.username and self.python_path == other.python_path + and self.shell_type == other.shell_type and self.connection_type == other.connection_type and self.connection_port == other.connection_port) return False @@ -225,6 +226,7 @@ class AwsProviderConfig(ProviderConfig): i.username = image.get('username', None) i.python_path = image.get('python-path', 'auto') + i.shell_type = image.get('shell-type', None) i.connection_type = image.get('connection-type', 'ssh') i.connection_port = image.get( 'connection-port', @@ -272,6 +274,7 @@ class AwsProviderConfig(ProviderConfig): 'name': str, 'connection-type': str, 'connection-port': int, + 'shell-type': str, 'image-id': str, "image-filters": [image_filters], 'username': str, diff --git a/nodepool/driver/aws/handler.py b/nodepool/driver/aws/handler.py index 845cd60f7..0a451561a 100644 --- a/nodepool/driver/aws/handler.py +++ b/nodepool/driver/aws/handler.py @@ -111,6 +111,7 @@ class AwsInstanceLauncher(NodeLauncher): self.node.host_keys = keys self.node.username = self.label.cloud_image.username self.node.python_path = self.label.cloud_image.python_path + self.node.shell_type = self.label.cloud_image.shell_type self.zk.storeNode(self.node) self.log.info("Instance %s is ready", instance_id) diff --git a/nodepool/driver/gce/config.py b/nodepool/driver/gce/config.py index 207cd5104..7131618a2 100644 --- a/nodepool/driver/gce/config.py +++ b/nodepool/driver/gce/config.py @@ -30,6 +30,7 @@ class ProviderCloudImage(ConfigValue): self.python_path = None self.connection_type = None self.connection_port = None + self.shell_type = None def __eq__(self, other): if isinstance(other, ProviderCloudImage): @@ -39,7 +40,8 @@ class ProviderCloudImage(ConfigValue): and self.key == other.key and self.python_path == other.python_path and self.connection_type == other.connection_type - and self.connection_port == other.connection_port) + and self.connection_port == other.connection_port + and self.shell_type == other.shell_type) return False def __repr__(self): @@ -202,6 +204,7 @@ class GCEProviderConfig(ProviderConfig): i.connection_port = image.get( 'connection-port', default_port_mapping.get(i.connection_type, 22)) + i.shell_type = image.get('shell-type', None) self.cloud_images[i.name] = i for pool in self.provider.get('pools', []): @@ -229,6 +232,7 @@ class GCEProviderConfig(ProviderConfig): 'name': str, 'connection-type': str, 'connection-port': int, + 'shell-type': str, 'image-id': str, 'image-project': str, 'image-family': str, diff --git a/nodepool/driver/kubernetes/config.py b/nodepool/driver/kubernetes/config.py index aaf6c986e..509479bdd 100644 --- a/nodepool/driver/kubernetes/config.py +++ b/nodepool/driver/kubernetes/config.py @@ -28,6 +28,7 @@ class KubernetesLabel(ConfigValue): other.type == self.type and other.image_pull == self.image_pull and other.python_path == self.python_path and + other.shell_type == self.shell_type and other.image == self.image and other.cpu == self.cpu and other.memory == self.memory and @@ -61,6 +62,7 @@ class KubernetesPool(ConfigPool): pl.image = label.get('image') pl.image_pull = label.get('image-pull', 'IfNotPresent') pl.python_path = label.get('python-path', 'auto') + pl.shell_type = label.get('shell-type') pl.cpu = label.get('cpu') pl.memory = label.get('memory') pl.env = label.get('env', []) @@ -112,6 +114,7 @@ class KubernetesProviderConfig(ProviderConfig): 'image': str, 'image-pull': str, 'python-path': str, + 'shell-type': str, 'cpu': int, 'memory': int, 'env': [env_var], diff --git a/nodepool/driver/kubernetes/handler.py b/nodepool/driver/kubernetes/handler.py index 02474f309..f26c2381c 100644 --- a/nodepool/driver/kubernetes/handler.py +++ b/nodepool/driver/kubernetes/handler.py @@ -38,6 +38,7 @@ class K8SLauncher(NodeLauncher): self.node.state = zk.READY self.node.python_path = self.label.python_path + self.node.shell_type = self.label.shell_type # NOTE: resource access token may be encrypted here self.node.connection_port = resource if self.label.type == "namespace": diff --git a/nodepool/driver/openshift/config.py b/nodepool/driver/openshift/config.py index e50a4f307..6038bb866 100644 --- a/nodepool/driver/openshift/config.py +++ b/nodepool/driver/openshift/config.py @@ -32,6 +32,7 @@ class OpenshiftLabel(ConfigValue): other.cpu == self.cpu and other.memory == self.memory and other.python_path == self.python_path and + other.shell_type == self.shell_type and other.env == self.env and other.node_selector == self.node_selector) return False @@ -64,6 +65,7 @@ class OpenshiftPool(ConfigPool): pl.cpu = label.get('cpu') pl.memory = label.get('memory') pl.python_path = label.get('python-path', 'auto') + pl.shell_type = label.get('shell-type') pl.env = label.get('env', []) pl.node_selector = label.get('node-selector') pl.pool = self @@ -116,6 +118,7 @@ class OpenshiftProviderConfig(ProviderConfig): 'cpu': int, 'memory': int, 'python-path': str, + 'shell-type': str, 'env': [env_var], 'node-selector': dict, } diff --git a/nodepool/driver/openshift/handler.py b/nodepool/driver/openshift/handler.py index 1ce94c20c..e0e416bf9 100644 --- a/nodepool/driver/openshift/handler.py +++ b/nodepool/driver/openshift/handler.py @@ -47,6 +47,7 @@ class OpenshiftLauncher(NodeLauncher): self.node.state = zk.READY self.node.python_path = self.label.python_path + self.node.shell_type = self.label.shell_type # NOTE: resource access token may be encrypted here self.node.connection_port = resource self.zk.storeNode(self.node) diff --git a/nodepool/driver/openshiftpods/config.py b/nodepool/driver/openshiftpods/config.py index 6cc531560..e688d2df8 100644 --- a/nodepool/driver/openshiftpods/config.py +++ b/nodepool/driver/openshiftpods/config.py @@ -56,6 +56,7 @@ class OpenshiftPodsProviderConfig(OpenshiftProviderConfig): 'cpu': int, 'memory': int, 'python-path': str, + 'shell-type': str, 'env': [env_var], 'node-selector': dict } diff --git a/nodepool/driver/openshiftpods/handler.py b/nodepool/driver/openshiftpods/handler.py index c39cfacec..6f1beabdc 100644 --- a/nodepool/driver/openshiftpods/handler.py +++ b/nodepool/driver/openshiftpods/handler.py @@ -34,6 +34,7 @@ class OpenshiftPodLauncher(OpenshiftLauncher): self.node.state = zk.READY self.node.python_path = self.label.python_path + self.node.shell_type = self.label.shell_type # NOTE: resource access token may be encrypted here k8s = self.handler.manager.k8s_client self.node.connection_port = { diff --git a/nodepool/driver/openstack/config.py b/nodepool/driver/openstack/config.py index 65d95c1b7..a554b9589 100644 --- a/nodepool/driver/openstack/config.py +++ b/nodepool/driver/openstack/config.py @@ -53,6 +53,7 @@ class ProviderCloudImage(ConfigValue): self.image_name = None self.username = None self.python_path = None + self.shell_type = None self.connection_type = None self.connection_port = None @@ -64,6 +65,7 @@ class ProviderCloudImage(ConfigValue): self.image_name == other.image_name and self.username == other.username and self.python_path == other.python_path and + self.shell_type == other.shell_type and self.connection_type == other.connection_type and self.connection_port == other.connection_port) return False @@ -336,6 +338,7 @@ class OpenStackProviderConfig(ProviderConfig): i.image_name = image.get('image-name', None) i.username = image.get('username', None) i.python_path = image.get('python-path', 'auto') + i.shell_type = image.get('shell-type', None) i.connection_type = image.get('connection-type', 'ssh') i.connection_port = image.get( 'connection-port', @@ -366,6 +369,7 @@ class OpenStackProviderConfig(ProviderConfig): v.Exclusive('image-name', 'cloud-image-name-or-id'): str, 'username': str, 'python-path': str, + 'shell-type': str, } pool_label_main = { diff --git a/nodepool/driver/openstack/handler.py b/nodepool/driver/openstack/handler.py index be9a058ce..4e49f6610 100644 --- a/nodepool/driver/openstack/handler.py +++ b/nodepool/driver/openstack/handler.py @@ -87,6 +87,7 @@ class OpenStackNodeLauncher(NodeLauncher): image_name = diskimage.name username = cloud_image.username python_path = cloud_image.python_path + shell_type = cloud_image.shell_type connection_type = diskimage.connection_type connection_port = diskimage.connection_port @@ -105,6 +106,7 @@ class OpenStackNodeLauncher(NodeLauncher): image_name = self.label.cloud_image.name username = self.label.cloud_image.username python_path = self.label.cloud_image.python_path + shell_type = self.label.cloud_image.shell_type connection_type = self.label.cloud_image.connection_type connection_port = self.label.cloud_image.connection_port @@ -159,6 +161,7 @@ class OpenStackNodeLauncher(NodeLauncher): self.node.username = username self.node.python_path = python_path + self.node.shell_type = shell_type self.node.connection_type = connection_type self.node.connection_port = connection_port diff --git a/nodepool/driver/simple.py b/nodepool/driver/simple.py index 2fdd6b30b..56064d57f 100644 --- a/nodepool/driver/simple.py +++ b/nodepool/driver/simple.py @@ -153,6 +153,7 @@ class SimpleTaskManagerLauncher(NodeLauncher): self.node.host_keys = keys self.node.username = self.label.cloud_image.username self.node.python_path = self.label.cloud_image.python_path + self.node.shell_type = self.label.cloud_image.shell_type self.zk.storeNode(self.node) self.log.info("Instance %s is ready", hostname) diff --git a/nodepool/driver/static/config.py b/nodepool/driver/static/config.py index aa0e3d272..f08528b01 100644 --- a/nodepool/driver/static/config.py +++ b/nodepool/driver/static/config.py @@ -57,6 +57,7 @@ class StaticPool(ConfigPool): 'connection-port': int( node.get('connection-port', node.get('ssh-port', 22))), 'connection-type': node.get('connection-type', 'ssh'), + 'shell-type': node.get('shell-type', None), 'username': node.get('username', 'zuul'), 'max-parallel-jobs': int(node.get('max-parallel-jobs', 1)), 'python-path': node.get('python-path', 'auto'), @@ -108,6 +109,7 @@ class StaticProviderConfig(ProviderConfig): 'host-key': v.Any(str, [str]), 'connection-port': int, 'connection-type': str, + 'shell-type': str, 'max-parallel-jobs': int, 'python-path': str, } diff --git a/nodepool/driver/static/provider.py b/nodepool/driver/static/provider.py index a313741e3..07c759570 100644 --- a/nodepool/driver/static/provider.py +++ b/nodepool/driver/static/provider.py @@ -194,6 +194,7 @@ class StaticNodeProvider(Provider): node.connection_port = static_node["connection-port"] node.connection_type = static_node["connection-type"] node.python_path = static_node["python-path"] + node.shell_type = static_node["shell-type"] nodeutils.set_node_ip(node) node.host_keys = host_keys node.attributes = pool.node_attributes @@ -218,14 +219,15 @@ class StaticNodeProvider(Provider): static_node["username"], static_node["connection-port"], static_node["connection-type"], + static_node["shell-type"], static_node["python-path"], host_keys, ) for node in nodes: original_attrs = (node.type, node.username, node.connection_port, - node.connection_type, node.python_path, - node.host_keys) + node.shell_type, node.connection_type, + node.python_path, node.host_keys) if original_attrs == new_attrs: continue @@ -236,6 +238,7 @@ class StaticNodeProvider(Provider): node.username = static_node["username"] node.connection_port = static_node["connection-port"] node.connection_type = static_node["connection-type"] + node.shell_type = static_node["shell-type"] node.python_path = static_node["python-path"] nodeutils.set_node_ip(node) node.host_keys = host_keys diff --git a/nodepool/tests/fixtures/aws.yaml b/nodepool/tests/fixtures/aws.yaml index 2088026ea..7a75b8e56 100644 --- a/nodepool/tests/fixtures/aws.yaml +++ b/nodepool/tests/fixtures/aws.yaml @@ -17,6 +17,7 @@ labels: - name: ubuntu1404-iam-instance-profile-arn - name: ubuntu1404-with-tags - name: ubuntu1404-with-name-tag + - name: ubuntu1404-with-shell-type providers: - name: ec2-us-west-2 @@ -48,6 +49,10 @@ providers: values: - ubuntu* username: ubuntu + - name: ubuntu1404-with-shell-type + image-id: ami-1e749f67 + username: ubuntu + shell-type: csh pools: - name: ebs-optimized max-servers: 1 @@ -161,3 +166,15 @@ providers: key-name: zuul tags: Name: different-name + - name: shell-type + max-servers: 1 + subnet-id: null + security-group-id: null + node-attributes: + key1: value1 + key2: value2 + labels: + - name: ubuntu1404-with-shell-type + cloud-image: ubuntu1404-with-shell-type + instance-type: t3.medium + key-name: zuul diff --git a/nodepool/tests/fixtures/config_validate/good.yaml b/nodepool/tests/fixtures/config_validate/good.yaml index 88ef94730..248a93ab0 100644 --- a/nodepool/tests/fixtures/config_validate/good.yaml +++ b/nodepool/tests/fixtures/config_validate/good.yaml @@ -24,6 +24,9 @@ labels: - name: openshift-project - name: openshift-pod - name: centos-ami + - name: winrm + - name: winssh + providers: - name: cloud1 @@ -124,6 +127,15 @@ providers: connection-port: 22022 username: zuul max-parallel-jobs: 1 + - name: windows.example.com + labels: winrm + username: zuul + connection-type: winrm + connection-port: 5986 + - name: windows-ssh.example.com + labels: winssh + username: zuul + shell-type: cmd - name: kubespray driver: kubernetes diff --git a/nodepool/tests/fixtures/node_unmanaged_image.yaml b/nodepool/tests/fixtures/node_unmanaged_image.yaml index 478652008..11e3c5520 100644 --- a/nodepool/tests/fixtures/node_unmanaged_image.yaml +++ b/nodepool/tests/fixtures/node_unmanaged_image.yaml @@ -29,6 +29,7 @@ providers: cloud-images: - name: fake-image python-path: /usr/bin/python3 + shell-type: csh - name: fake-image-windows username: zuul connection-type: winrm diff --git a/nodepool/tests/fixtures/openshift.yaml b/nodepool/tests/fixtures/openshift.yaml index d48f8c2df..9c97c5019 100644 --- a/nodepool/tests/fixtures/openshift.yaml +++ b/nodepool/tests/fixtures/openshift.yaml @@ -28,3 +28,4 @@ providers: type: pod image: docker.io/fedora:28 python-path: '/usr/bin/python3' + shell-type: csh diff --git a/nodepool/tests/fixtures/static-shell-type.yaml b/nodepool/tests/fixtures/static-shell-type.yaml new file mode 100644 index 000000000..183b7a55a --- /dev/null +++ b/nodepool/tests/fixtures/static-shell-type.yaml @@ -0,0 +1,26 @@ +zookeeper-servers: + - host: {zookeeper_host} + port: {zookeeper_port} + chroot: {zookeeper_chroot} + +zookeeper-tls: + ca: {zookeeper_ca} + cert: {zookeeper_cert} + key: {zookeeper_key} + +labels: + - name: fake-label + +providers: + - name: static-provider + driver: static + pools: + - name: main + nodes: + - name: fake-host-1 + labels: fake-label + host-key: ssh-rsa FAKEKEY + timeout: 13 + connection-port: 22022 + username: zuul + shell-type: cmd diff --git a/nodepool/tests/unit/test_driver_aws.py b/nodepool/tests/unit/test_driver_aws.py index 9343a6b3b..50a6e7a50 100644 --- a/nodepool/tests/unit/test_driver_aws.py +++ b/nodepool/tests/unit/test_driver_aws.py @@ -49,7 +49,8 @@ class TestDriverAws(tests.DBTestCase): host_key_checking=True, userdata=None, public_ip=True, - tags=[]): + tags=[], + shell_type=None): aws_id = 'AK000000000000000000' aws_key = '0123456789abcdef0123456789abcdef0123456789abcdef' self.useFixture( @@ -97,6 +98,8 @@ class TestDriverAws(tests.DBTestCase): raw_config['providers'][0]['pools'][4]['security-group-id'] = sg_id raw_config['providers'][0]['pools'][5]['subnet-id'] = subnet_id raw_config['providers'][0]['pools'][5]['security-group-id'] = sg_id + raw_config['providers'][0]['pools'][6]['subnet-id'] = subnet_id + raw_config['providers'][0]['pools'][6]['security-group-id'] = sg_id with tempfile.NamedTemporaryFile() as tf: tf.write(yaml.safe_dump( @@ -152,6 +155,7 @@ class TestDriverAws(tests.DBTestCase): self.assertEqual(node.connection_type, 'ssh') self.assertEqual(node.attributes, {'key1': 'value1', 'key2': 'value2'}) + self.assertEqual(node.shell_type, shell_type) if host_key_checking: nodescan.assert_called_with( node.interface_ip, @@ -251,3 +255,7 @@ class TestDriverAws(tests.DBTestCase): tags=[ {"Key": "Name", "Value": "different-name"} ]) + + def test_ec2_machine_shell_type(self): + self._test_ec2_machine('ubuntu1404-with-shell-type', + shell_type="csh") diff --git a/nodepool/tests/unit/test_driver_azure.py b/nodepool/tests/unit/test_driver_azure.py index 4c9b73224..45b1ff004 100644 --- a/nodepool/tests/unit/test_driver_azure.py +++ b/nodepool/tests/unit/test_driver_azure.py @@ -195,3 +195,4 @@ class TestDriverAzure(tests.DBTestCase): self.assertEqual(node.state, zk.READY) self.assertIsNotNone(node.launcher) self.assertEqual(node.connection_type, 'ssh') + self.assertIsNone(node.shell_type) diff --git a/nodepool/tests/unit/test_driver_openshift.py b/nodepool/tests/unit/test_driver_openshift.py index f47fd66b9..e5c624bd2 100644 --- a/nodepool/tests/unit/test_driver_openshift.py +++ b/nodepool/tests/unit/test_driver_openshift.py @@ -155,6 +155,7 @@ class TestDriverOpenshift(tests.DBTestCase): self.assertEqual(node.connection_type, 'kubectl') self.assertEqual(node.connection_port.get('token'), 'fake-token') self.assertEqual(node.python_path, '/usr/bin/python3') + self.assertEqual(node.shell_type, 'csh') self.assertEqual(node.attributes, {'key1': 'value1', 'key2': 'value2'}) @@ -183,6 +184,8 @@ class TestDriverOpenshift(tests.DBTestCase): self.assertIsNotNone(node.launcher) self.assertEqual(node.connection_type, 'project') self.assertEqual(node.connection_port.get('token'), 'fake-token') + self.assertEqual(node.python_path, 'auto') + self.assertIsNone(node.shell_type) node.state = zk.DELETING self.zk.storeNode(node) diff --git a/nodepool/tests/unit/test_driver_static.py b/nodepool/tests/unit/test_driver_static.py index 56984557b..13a2e6d0f 100644 --- a/nodepool/tests/unit/test_driver_static.py +++ b/nodepool/tests/unit/test_driver_static.py @@ -64,6 +64,8 @@ class TestDriverStatic(tests.DBTestCase): self.assertEqual(nodes[0].host_keys, ['ssh-rsa FAKEKEY']) self.assertEqual(nodes[0].attributes, {'key1': 'value1', 'key2': 'value2'}) + self.assertEqual(nodes[0].python_path, 'auto') + self.assertIsNone(nodes[0].shell_type) def test_static_python_path(self): ''' @@ -528,6 +530,25 @@ class TestDriverStatic(tests.DBTestCase): self.assertEqual(len(nodes), 1) nodescan_mock.assert_not_called() + def test_static_shell_type(self): + ''' + Test that static python-path works. + ''' + configfile = self.setup_config('static-shell-type.yaml') + pool = self.useNodepool(configfile, watermark_sleep=1) + pool.start() + + self.log.debug("Waiting for node pre-registration") + nodes = self.waitForNodes('fake-label') + self.assertEqual(nodes[0].shell_type, "cmd") + + nodes[0].state = zk.USED + self.zk.storeNode(nodes[0]) + + self.log.debug("Waiting for node to be re-available") + nodes = self.waitForNodes('fake-label') + self.assertEqual(nodes[0].shell_type, "cmd") + def test_missing_static_node(self): """Test that a missing static node is added""" configfile = self.setup_config('static-2-nodes.yaml') diff --git a/nodepool/tests/unit/test_launcher.py b/nodepool/tests/unit/test_launcher.py index 148014e8b..4e87dbea4 100644 --- a/nodepool/tests/unit/test_launcher.py +++ b/nodepool/tests/unit/test_launcher.py @@ -1383,6 +1383,7 @@ class TestLauncher(tests.DBTestCase): self.assertEqual(len(nodes), 1) self.assertIsNone(nodes[0].username) self.assertEqual(nodes[0].python_path, '/usr/bin/python3') + self.assertEqual(nodes[0].shell_type, 'csh') nodes = self.waitForNodes('fake-label-windows') self.assertEqual(len(nodes), 1) @@ -1391,6 +1392,7 @@ class TestLauncher(tests.DBTestCase): self.assertEqual(5986, nodes[0].connection_port) self.assertEqual(nodes[0].host_keys, []) self.assertEqual(nodes[0].python_path, 'auto') + self.assertIsNone(nodes[0].shell_type) nodes = self.waitForNodes('fake-label-arbitrary-port') self.assertEqual(len(nodes), 1) @@ -1398,6 +1400,8 @@ class TestLauncher(tests.DBTestCase): self.assertEqual('winrm', nodes[0].connection_type) self.assertEqual(1234, nodes[0].connection_port) self.assertEqual(nodes[0].host_keys, []) + self.assertEqual(nodes[0].python_path, 'auto') + self.assertIsNone(nodes[0].shell_type) def test_unmanaged_image_provider_name(self): """ diff --git a/nodepool/zk.py b/nodepool/zk.py index f59e63ee7..773f6d27a 100644 --- a/nodepool/zk.py +++ b/nodepool/zk.py @@ -299,6 +299,7 @@ class ImageBuild(BaseModel): self.builder_id = None # Unique ID self.username = None self.python_path = None + self.shell_type = None def __repr__(self): d = self.toDict() @@ -332,6 +333,7 @@ class ImageBuild(BaseModel): d['formats'] = ','.join(self.formats) d['username'] = self.username d['python_path'] = self.python_path + d['shell_type'] = self.shell_type return d @staticmethod @@ -350,6 +352,7 @@ class ImageBuild(BaseModel): o.builder_id = d.get('builder_id') o.username = d.get('username', 'zuul') o.python_path = d.get('python_path', '/usr/bin/python2') + o.shell_type = d.get('shell_type') # Only attempt the split on non-empty string if d.get('formats', ''): o.formats = d.get('formats', '').split(',') @@ -363,7 +366,8 @@ class ImageUpload(BaseModel): VALID_STATES = set([UPLOADING, READY, DELETING, FAILED]) def __init__(self, build_id=None, provider_name=None, image_name=None, - upload_id=None, username=None, python_path=None): + upload_id=None, username=None, python_path=None, + shell_type=None): super(ImageUpload, self).__init__(upload_id) self.build_id = build_id self.provider_name = provider_name @@ -371,6 +375,7 @@ class ImageUpload(BaseModel): self.format = None self.username = username self.python_path = python_path + self.shell_type = shell_type self.external_id = None # Provider ID of the image self.external_name = None # Provider name of the image @@ -403,6 +408,7 @@ class ImageUpload(BaseModel): d['format'] = self.format d['username'] = self.username d['python_path'] = self.python_path + d['shell_type'] = self.shell_type return d @staticmethod @@ -425,6 +431,7 @@ class ImageUpload(BaseModel): o.format = d.get('format') o.username = d.get('username', 'zuul') o.python_path = d.get('python_path', '/usr/bin/python2') + o.shell_type = d.get('shell_type') return o @@ -551,7 +558,9 @@ class Node(BaseModel): self.public_ipv6 = None self.host_id = None self.interface_ip = None + self.connection_type = None self.connection_port = 22 + self.shell_type = None self.image_id = None self.launcher = None self.created_time = None @@ -560,7 +569,6 @@ class Node(BaseModel): self.comment = None self.hold_job = None self.username = None - self.connection_type = None self.host_keys = [] self.hold_expiration = None self.resources = None @@ -600,6 +608,7 @@ class Node(BaseModel): self.username == other.username and self.connection_type == other.connection_type and self.connection_port == other.connection_port and + self.shell_type == other.shell_type and self.host_keys == other.host_keys and self.hold_expiration == other.hold_expiration and self.resources == other.resources and @@ -650,6 +659,7 @@ class Node(BaseModel): d['username'] = self.username d['connection_type'] = self.connection_type d['connection_port'] = self.connection_port + d['shell_type'] = self.shell_type d['hold_expiration'] = self.hold_expiration d['resources'] = self.resources d['attributes'] = self.attributes @@ -718,6 +728,7 @@ class Node(BaseModel): self.resources = d.get('resources') self.attributes = d.get('attributes') self.python_path = d.get('python_path') + self.shell_type = d.get('shell_type') class ZooKeeper(object):