From 52f7d4fb6294733c6625ae21887e7da94cbcbe89 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Wed, 23 Jan 2019 13:15:20 +0100 Subject: [PATCH] Make public ip configurable in aws When running nodepool against private cloud rooms it can be desirable that the nodes don't get a public ip address. Let the user specify this on pool level. Change-Id: I3d636517837fd8a6593c12e4309372da5c062b06 --- doc/source/configuration.rst | 6 +++ nodepool/driver/aws/config.py | 4 ++ nodepool/driver/aws/handler.py | 2 +- nodepool/driver/aws/provider.py | 2 +- nodepool/tests/fixtures/aws.yaml | 11 +++++ nodepool/tests/unit/test_driver_aws.py | 42 ++++++++++++++++++- .../notes/aws-public-ip-d9a7cc9aae98054c.yaml | 5 +++ 7 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/aws-public-ip-d9a7cc9aae98054c.yaml diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 97c49d1ed..ecb4fc4f8 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -1748,6 +1748,12 @@ section of the configuration. If provided, specifies the security group ID to assign to the primary network interface of nodes. + .. attr:: public-ip-address + :type: bool + :default: True + + Specify if a public ip address shall be attached to nodes. + .. attr:: host-key-checking :type: bool :default: True diff --git a/nodepool/driver/aws/config.py b/nodepool/driver/aws/config.py index 25de6f26e..d8c1f50f4 100644 --- a/nodepool/driver/aws/config.py +++ b/nodepool/driver/aws/config.py @@ -85,6 +85,7 @@ class ProviderPool(ConfigPool): self.max_ram = None self.subnet_id = None self.security_group_id = None + self.public_ip = True self.host_key_checking = True self.labels = None # The ProviderConfig object that owns this pool. @@ -102,6 +103,7 @@ class ProviderPool(ConfigPool): self.subnet_id = pool_config.get('subnet-id') self.host_key_checking = bool( pool_config.get('host-key-checking', True)) + self.public_ip = bool(pool_config.get('public-ip-address', True)) for label in pool_config.get('labels', []): pl = ProviderLabel() @@ -135,6 +137,7 @@ class ProviderPool(ConfigPool): and other.name == self.name and other.subnet_id == self.subnet_id and other.security_group_id == self.security_group_id + and other.public_ip == self.public_ip and other.host_key_checking == self.host_key_checking and other.labels == self.labels) return False @@ -238,6 +241,7 @@ class AwsProviderConfig(ProviderConfig): 'host-key-checking': bool, 'security-group-id': str, 'subnet-id': str, + 'public-ip-address': bool, }) image_filters = { diff --git a/nodepool/driver/aws/handler.py b/nodepool/driver/aws/handler.py index 1d01043ee..bc0a0b3b6 100644 --- a/nodepool/driver/aws/handler.py +++ b/nodepool/driver/aws/handler.py @@ -73,7 +73,7 @@ class AwsInstanceLauncher(NodeLauncher): raise exceptions.LaunchStatusException( "Instance %s failed to start: %s" % (instance_id, state)) - server_ip = instance.public_ip_address + server_ip = instance.public_ip_address or instance.private_ip_address if not server_ip: raise exceptions.LaunchStatusException( "Instance %s doesn't have a public ip" % instance_id) diff --git a/nodepool/driver/aws/provider.py b/nodepool/driver/aws/provider.py index 08e3a24a7..d5057623e 100644 --- a/nodepool/driver/aws/provider.py +++ b/nodepool/driver/aws/provider.py @@ -169,7 +169,7 @@ class AwsProvider(Provider): KeyName=label.key_name, InstanceType=label.instance_type, NetworkInterfaces=[{ - 'AssociatePublicIpAddress': True, + 'AssociatePublicIpAddress': label.pool.public_ip, 'DeviceIndex': 0}]) if label.pool.security_group_id: diff --git a/nodepool/tests/fixtures/aws.yaml b/nodepool/tests/fixtures/aws.yaml index 52891a819..d439f0b8e 100644 --- a/nodepool/tests/fixtures/aws.yaml +++ b/nodepool/tests/fixtures/aws.yaml @@ -10,6 +10,7 @@ labels: - name: ubuntu1404-by-capitalized-filters - name: ubuntu1404-bad-config - name: ubuntu1404-non-host-key-checking + - name: ubuntu1404-private-ip - name: ubuntu1404-userdata providers: @@ -83,3 +84,13 @@ providers: cloud-image: ubuntu1404 instance-type: t3.medium key-name: zuul + - name: private-ip-address + max-servers: 1 + subnet-id: null + security-group-id: null + public-ip-address: false + labels: + - name: ubuntu1404-private-ip + cloud-image: ubuntu1404 + instance-type: t3.medium + key-name: zuul diff --git a/nodepool/tests/unit/test_driver_aws.py b/nodepool/tests/unit/test_driver_aws.py index b32f7b39b..163fc9060 100644 --- a/nodepool/tests/unit/test_driver_aws.py +++ b/nodepool/tests/unit/test_driver_aws.py @@ -27,16 +27,28 @@ import yaml from nodepool import tests from nodepool import zk +from nodepool.nodeutils import iterate_timeout class TestDriverAws(tests.DBTestCase): log = logging.getLogger("nodepool.TestDriverAws") + def _wait_for_provider(self, nodepool, provider): + for _ in iterate_timeout( + 30, Exception, 'wait for provider'): + try: + provider_manager = nodepool.getProviderManager(provider) + if provider_manager.ec2 is not None: + break + except Exception: + pass + @mock_ec2 def _test_ec2_machine(self, label, is_valid_config=True, host_key_checking=True, - userdata=None): + userdata=None, + public_ip=True): aws_id = 'AK000000000000000000' aws_key = '0123456789abcdef0123456789abcdef0123456789abcdef' self.useFixture( @@ -70,6 +82,8 @@ class TestDriverAws(tests.DBTestCase): raw_config['providers'][0]['pools'][0]['security-group-id'] = sg_id raw_config['providers'][0]['pools'][1]['subnet-id'] = subnet_id raw_config['providers'][0]['pools'][1]['security-group-id'] = sg_id + raw_config['providers'][0]['pools'][2]['subnet-id'] = subnet_id + raw_config['providers'][0]['pools'][2]['security-group-id'] = sg_id with tempfile.NamedTemporaryFile() as tf: tf.write(yaml.safe_dump( @@ -78,6 +92,28 @@ class TestDriverAws(tests.DBTestCase): configfile = self.setup_config(tf.name) pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() + + self._wait_for_provider(pool, 'ec2-us-west-2') + provider_manager = pool.getProviderManager('ec2-us-west-2') + + # Note: boto3 doesn't handle private ip addresses correctly + # when in fake mode so we need to intercept the + # create_instances call and validate the args we supply. + def _fake_create_instances(*args, **kwargs): + self.assertIsNotNone(kwargs.get('NetworkInterfaces')) + interfaces = kwargs.get('NetworkInterfaces') + self.assertEqual(1, len(interfaces)) + if not public_ip: + self.assertEqual( + False, + interfaces[0].get('AssociatePublicIpAddress')) + return provider_manager.ec2.create_instances_orig( + *args, **kwargs) + + provider_manager.ec2.create_instances_orig = \ + provider_manager.ec2.create_instances + provider_manager.ec2.create_instances = _fake_create_instances + req = zk.NodeRequest() req.state = zk.REQUESTED req.node_types.append(label) @@ -169,3 +205,7 @@ class TestDriverAws(tests.DBTestCase): def test_ec2_machine_userdata(self): self._test_ec2_machine('ubuntu1404-userdata', userdata=True) + + def test_ec2_machine_private_ip(self): + self._test_ec2_machine('ubuntu1404-private-ip', + public_ip=False) diff --git a/releasenotes/notes/aws-public-ip-d9a7cc9aae98054c.yaml b/releasenotes/notes/aws-public-ip-d9a7cc9aae98054c.yaml new file mode 100644 index 000000000..06d49e18c --- /dev/null +++ b/releasenotes/notes/aws-public-ip-d9a7cc9aae98054c.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + It is now possible to specify if AWS nodes shall get a + :attr:`providers.[aws].pools.public-ip-address`.