From 179af2c3e1eadecfca78a2d799496225ba3855c7 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Sat, 24 Dec 2016 17:30:19 -0600 Subject: [PATCH] Remove passing hostname and ports to Docker These two options are conflicting with network mode 'none'. Let's skip it on creating containers. Instead, we treat these properties as outputted attributes. In particular, after a docker container is created, we inspect the container and populate these fields. Depends-On: I063e91e22c891d86ac1733222bb6a8c48bb68a70 Change-Id: I931d5e5d4425b455fe3a825aceb5199fcac15fcb Related-Bug: #1650964 --- nova/virt/zun/driver.py | 2 +- zun/api/controllers/v1/schemas/containers.py | 2 -- zun/common/validation/parameter_types.py | 16 ---------- zun/container/docker/driver.py | 18 +++++++++-- zun/db/sqlalchemy/models.py | 2 +- zun/tests/unit/common/test_validations.py | 20 ------------- .../container/docker/test_docker_driver.py | 30 +++++++++++++------ 7 files changed, 38 insertions(+), 52 deletions(-) diff --git a/nova/virt/zun/driver.py b/nova/virt/zun/driver.py index 75609a90a..2b8a822f2 100644 --- a/nova/virt/zun/driver.py +++ b/nova/virt/zun/driver.py @@ -490,7 +490,7 @@ class DockerDriver(driver.ComputeDriver): flavor=None): image_name = self._get_image_name(context, instance, image_meta) args = { - 'hostname': instance['display_name'], + 'hostname': instance['display_name'][:63], 'mem_limit': self._get_memory_limit_bytes(instance), 'cpu_shares': self._get_cpu_shares(instance), 'network_disabled': True, diff --git a/zun/api/controllers/v1/schemas/containers.py b/zun/api/controllers/v1/schemas/containers.py index 59948beb8..c5d3f1d7c 100644 --- a/zun/api/controllers/v1/schemas/containers.py +++ b/zun/api/controllers/v1/schemas/containers.py @@ -19,11 +19,9 @@ _container_properties = { 'cpu': parameter_types.cpu, 'memory': parameter_types.memory, 'workdir': parameter_types.workdir, - 'hostname': parameter_types.hostname, 'image_pull_policy': parameter_types.image_pull_policy, 'labels': parameter_types.labels, 'environment': parameter_types.environment, - 'ports': parameter_types.ports, } container_create = { diff --git a/zun/common/validation/parameter_types.py b/zun/common/validation/parameter_types.py index 3d5d32cc7..9f3998153 100644 --- a/zun/common/validation/parameter_types.py +++ b/zun/common/validation/parameter_types.py @@ -61,12 +61,6 @@ workdir = { 'type': ['string', 'null'] } -hostname = { - 'type': ['string', 'null'], - 'minLength': 0, - 'maxLength': 255 -} - image_pull_policy = { 'type': ['string', 'null'], 'enum': ['never', 'always', 'ifnotpresent', None] @@ -80,16 +74,6 @@ environment = { 'type': ['object', 'null'] } -ports = { - 'type': ['array', 'null'], - 'items': { - 'type': ['integer', 'string'], - 'pattern': '^[0-9]*$', - 'minimum': 1, 'maximum': 65535, - 'minLength': 1 - } -} - image_id = { 'type': ['string', 'null'], 'minLength': 2, diff --git a/zun/container/docker/driver.py b/zun/container/docker/driver.py index 79dc39ce0..a75cff5ac 100644 --- a/zun/container/docker/driver.py +++ b/zun/container/docker/driver.py @@ -68,11 +68,9 @@ class DockerDriver(driver.ContainerDriver): kwargs = { 'name': self.get_container_name(container), - 'hostname': container.hostname, 'command': container.command, 'environment': container.environment, 'working_dir': container.workdir, - 'ports': container.ports, 'labels': container.labels, } @@ -140,6 +138,19 @@ class DockerDriver(driver.ContainerDriver): else: container.status = fields.ContainerStatus.STOPPED + config = response.get('Config') + if config: + # populate hostname + container.hostname = config.get('Hostname') + # populate ports + ports = [] + exposed_ports = config.get('ExposedPorts') + if exposed_ports: + for key in exposed_ports: + port = key.split('/')[0] + ports.append(int(port)) + container.ports = ports + @check_container_id def reboot(self, container, timeout): with docker_utils.docker_client() as docker: @@ -222,7 +233,8 @@ class DockerDriver(driver.ContainerDriver): def create_sandbox(self, context, container, image='kubernetes/pause'): with docker_utils.docker_client() as docker: name = self.get_sandbox_name(container) - response = docker.create_container(image, name=name) + response = docker.create_container(image, name=name, + hostname=name[:63]) sandbox_id = response['Id'] docker.start(sandbox_id) return sandbox_id diff --git a/zun/db/sqlalchemy/models.py b/zun/db/sqlalchemy/models.py index 84535724a..dfe377b03 100644 --- a/zun/db/sqlalchemy/models.py +++ b/zun/db/sqlalchemy/models.py @@ -140,7 +140,7 @@ class Container(Base): environment = Column(JSONEncodedDict) workdir = Column(String(255)) ports = Column(JSONEncodedList) - hostname = Column(String(255)) + hostname = Column(String(63)) labels = Column(JSONEncodedDict) meta = Column(JSONEncodedDict) addresses = Column(JSONEncodedDict) diff --git a/zun/tests/unit/common/test_validations.py b/zun/tests/unit/common/test_validations.py index aa7fba8a9..b170d630d 100644 --- a/zun/tests/unit/common/test_validations.py +++ b/zun/tests/unit/common/test_validations.py @@ -25,9 +25,7 @@ CONTAINER_CREATE = { 'cpu': parameter_types.cpu, 'memory': parameter_types.memory, 'workdir': parameter_types.workdir, - 'hostname': parameter_types.hostname, 'image_pull_policy': parameter_types.image_pull_policy, - 'ports': parameter_types.ports, 'labels': parameter_types.labels, 'environment': parameter_types.environment }, @@ -45,9 +43,7 @@ class TestSchemaValidations(base.BaseTestCase): request_to_validate = {'name': 'test1', 'image': 'nginx', 'command': '/bin/sh', 'cpu': 1.0, 'memory': '5', 'workdir': '/workdir', - 'hostname': 'container1', 'image_pull_policy': 'never', - 'ports': ['123', '1', 1, '65535', 65535, 123], 'labels': {'abc': 12, 'bcd': 'xyz'}, 'environment': {'xyz': 'pqr', 'pqr': 2}} self.schema_validator.validate(request_to_validate) @@ -56,9 +52,7 @@ class TestSchemaValidations(base.BaseTestCase): request_to_validate = {'name': None, 'image': 'nginx', 'command': None, 'cpu': None, 'memory': None, 'workdir': None, - 'hostname': None, 'image_pull_policy': None, - 'ports': None, 'labels': None, 'environment': None} self.schema_validator.validate(request_to_validate) @@ -99,20 +93,6 @@ class TestSchemaValidations(base.BaseTestCase): request_to_validate = {'memory': value, 'image': 'nginx'} self.schema_validator.validate(request_to_validate) - def test_create_schema_invalid_ports(self): - invalid_ports = [56, 0, 1, 65535, "", [0, '0', 65536, '65536', - "", 'x', " "]] - for value in invalid_ports: - request_to_validate = {'image': 'nginx', 'ports': value} - # TODO(pksingh): if value inside port array is not valid, - # message like below is raised: - # 'Invalid input for field '2'. Value: '65536'. - # 65536 is greater than the maximum of 65535' - # I think field '2' in message is not informative. - with self.assertRaisesRegexp(exception.SchemaValidationError, - "Invalid input for field"): - self.schema_validator.validate(request_to_validate) - def test_create_schema_cpu(self): valid_cpu = [4, 5, '4', '5', '0.5', '123.50', 0.5, 123.50, None] invalid_cpu = ['12a', 'abc', '0.a', 'a.90', "", " ", " 0.9 "] diff --git a/zun/tests/unit/container/docker/test_docker_driver.py b/zun/tests/unit/container/docker/test_docker_driver.py index 1fe5c3054..9f2296d80 100644 --- a/zun/tests/unit/container/docker/test_docker_driver.py +++ b/zun/tests/unit/container/docker/test_docker_driver.py @@ -75,11 +75,9 @@ class TestDockerDriver(base.DriverTestCase): kwargs = { 'name': 'zun-ea8e2a25-2901-438d-8157-de7ffd68d051', - 'hostname': 'testhost', 'command': 'fake_command', 'environment': {'key1': 'val1', 'key2': 'val2'}, 'working_dir': '/home/ubuntu', - 'ports': [80, 443], 'labels': {'key1': 'val1', 'key2': 'val2'}, 'host_config': {'Id1': 'val1', 'key2': 'val2'}, } @@ -113,11 +111,9 @@ class TestDockerDriver(base.DriverTestCase): **host_config) kwargs = { - 'hostname': 'testhost', 'command': 'fake_command', 'environment': {'key1': 'val1', 'key2': 'val2'}, 'working_dir': '/home/ubuntu', - 'ports': [80, 443], 'labels': {'key1': 'val1', 'key2': 'val2'}, 'host_config': {'Id1': 'val1', 'key2': 'val2'}, 'name': 'zun-ea8e2a25-2901-438d-8157-de7ffd68d051', @@ -165,7 +161,7 @@ class TestDockerDriver(base.DriverTestCase): self.mock_docker.list_instances.assert_called_once_with() def test_show_success(self): - self.mock_docker.inspect_container = mock.Mock() + self.mock_docker.inspect_container = mock.Mock(return_value={}) db_container = db_utils.create_test_container(context=self.context) self.driver.show(db_container) self.mock_docker.inspect_container.assert_called_once_with( @@ -266,7 +262,7 @@ class TestDockerDriver(base.DriverTestCase): def test_kill_successful_signal_is_none(self): self.mock_docker.kill = mock.Mock() - self.mock_docker.inspect_container = mock.Mock() + self.mock_docker.inspect_container = mock.Mock(return_value={}) db_container = db_utils.create_test_container(context=self.context) self.driver.kill(db_container, signal=None) self.mock_docker.kill.assert_called_once_with( @@ -276,7 +272,7 @@ class TestDockerDriver(base.DriverTestCase): def test_kill_successful_signal_is_not_none(self): self.mock_docker.kill = mock.Mock() - self.mock_docker.inspect_container = mock.Mock() + self.mock_docker.inspect_container = mock.Mock(return_value={}) db_container = db_utils.create_test_container(context=self.context) self.driver.kill(db_container, signal='test') self.mock_docker.kill.assert_called_once_with( @@ -318,7 +314,8 @@ class TestDockerDriver(base.DriverTestCase): @mock.patch('zun.container.docker.driver.DockerDriver.get_sandbox_name') def test_create_sandbox(self, mock_get_sandbox_name): - mock_get_sandbox_name.return_value = 'my_test_sandbox' + sandbox_name = 'my_test_sandbox' + mock_get_sandbox_name.return_value = sandbox_name self.mock_docker.create_container = mock.Mock( return_value={'Id': 'val1', 'key1': 'val2'}) self.mock_docker.start() @@ -327,7 +324,22 @@ class TestDockerDriver(base.DriverTestCase): db_container, 'kubernetes/pause') self.mock_docker.create_container.assert_called_once_with( - 'kubernetes/pause', name='my_test_sandbox') + 'kubernetes/pause', name=sandbox_name, hostname=sandbox_name) + self.assertEqual(result_sandbox_id, 'val1') + + @mock.patch('zun.container.docker.driver.DockerDriver.get_sandbox_name') + def test_create_sandbox_with_long_name(self, mock_get_sandbox_name): + sandbox_name = 'x' * 100 + mock_get_sandbox_name.return_value = sandbox_name + self.mock_docker.create_container = mock.Mock( + return_value={'Id': 'val1', 'key1': 'val2'}) + self.mock_docker.start() + db_container = db_utils.create_test_container(context=self.context) + result_sandbox_id = self.driver.create_sandbox(self.context, + db_container, + 'kubernetes/pause') + self.mock_docker.create_container.assert_called_once_with( + 'kubernetes/pause', name=sandbox_name, hostname=sandbox_name[:63]) self.assertEqual(result_sandbox_id, 'val1') def test_delete_sandbox(self):