From 220e6027cef42990eec6cdfc06514605ab5b3834 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2016 15:02:15 +0000 Subject: [PATCH 1/7] Test that swappiness is used by StartApplication --- flocker/node/_container.py | 1 + flocker/node/test/test_container.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/flocker/node/_container.py b/flocker/node/_container.py index 5ea3811284..243e102d06 100644 --- a/flocker/node/_container.py +++ b/flocker/node/_container.py @@ -111,6 +111,7 @@ def run(self, deployer, state_persister): # The only supported policy is "never". See FLOC-2449. restart_policy=RestartNever(), command_line=application.command_line, + swappiness=application.swappiness, ) diff --git a/flocker/node/test/test_container.py b/flocker/node/test/test_container.py index c3257df454..25af43fe2c 100644 --- a/flocker/node/test/test_container.py +++ b/flocker/node/test/test_container.py @@ -460,6 +460,34 @@ def test_command_line(self): pvector(command_line), ) + def test_swappiness(self): + """ + ``StartApplication.run()`` passes ``swappiness`` to + ``DockerClient.add`` which is used when creating a Unit. + """ + expected_swappiness = 99 + fake_docker = FakeDockerClient() + deployer = ApplicationNodeDeployer(u'example.com', fake_docker) + + application_name = u'site-example.com' + application = Application( + name=application_name, + image=DockerImage(repository=u'clusterhq/postgresql', + tag=u'9.3.5'), + swappiness=expected_swappiness, + ) + + StartApplication( + application=application, + node_state=EMPTY_NODESTATE, + ).run(deployer, state_persister=InMemoryStatePersister()) + + [unit] = self.successResultOf(fake_docker.list()) + self.assertEqual( + expected_swappiness, + unit.swappiness, + ) + class LinkEnviromentTests(TestCase): """ From 004c9b269e603accc2903d53154ec8db6c7128b8 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2016 15:14:22 +0000 Subject: [PATCH 2/7] Set the swappiness attribute of applications during discovery --- flocker/node/_container.py | 1 + flocker/node/test/test_container.py | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/flocker/node/_container.py b/flocker/node/_container.py index 243e102d06..3a075cdd76 100644 --- a/flocker/node/_container.py +++ b/flocker/node/_container.py @@ -316,6 +316,7 @@ def _applications_from_containers( restart_policy=container.restart_policy, running=(container.activation_state == u"active"), command_line=container.command_line, + swappiness=container.swappiness, )) return applications diff --git a/flocker/node/test/test_container.py b/flocker/node/test/test_container.py index 25af43fe2c..9abf548a32 100644 --- a/flocker/node/test/test_container.py +++ b/flocker/node/test/test_container.py @@ -705,6 +705,17 @@ def test_discover_application_with_memory_limit(self): applications = [APP.set("memory_limit", memory_limit)] self._verify_discover_state_applications(units, applications) + def test_discover_application_with_swappiness(self): + """ + An ``Application`` with a swappiness value is discovered from a + ``Unit`` with a swappiness value. + """ + swappiness = 99 + unit1 = UNIT_FOR_APP.set("swappiness", swappiness) + units = {unit1.name: unit1} + applications = [APP.set("swappiness", swappiness)] + self._verify_discover_state_applications(units, applications) + def test_discover_application_with_environment(self): """ An ``Application`` with ``Environment`` objects is discovered from a From 986e4900132f6ad25eeb5e0440ab03114ad590bd Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2016 16:56:21 +0000 Subject: [PATCH 3/7] A failing test demonstrating a bug in the current swappiness setting code --- flocker/node/functional/test_deploy.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/flocker/node/functional/test_deploy.py b/flocker/node/functional/test_deploy.py index 631d5dd27e..d2a226bde8 100644 --- a/flocker/node/functional/test_deploy.py +++ b/flocker/node/functional/test_deploy.py @@ -368,6 +368,21 @@ def test_command_line_introspection(self): results.node_state.applications.values()])) return d + @if_docker_configured + def test_swappiness_introspection(self): + """ + The swappiness value discovered by the ApplicationDeployer is the same + as was passed in. + """ + swappiness = 98 + d = self._start_container_for_introspection(swappiness=swappiness) + d.addCallback( + lambda results: self.assertEqual( + [swappiness], + [app.swappiness for app in + results.node_state.applications.values()])) + return d + @if_docker_configured def test_memory_limit(self): """ From 4415015ea2cf02636846ac8b8f9f2c2be19a1f8f Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2016 16:59:44 +0000 Subject: [PATCH 4/7] remove the broken code --- flocker/node/_docker.py | 5 ----- flocker/node/functional/test_deploy.py | 4 ++-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/flocker/node/_docker.py b/flocker/node/_docker.py index 98bb14bea3..5178aa0db5 100644 --- a/flocker/node/_docker.py +++ b/flocker/node/_docker.py @@ -724,10 +724,6 @@ def _create(): if command_line_values is not None: command_line_values = list(command_line_values) - memswap_limit = -1 - if swappiness != 0: - memswap_limit = mem_limit + mem_limit * swappiness - self._client.create_container( name=container_name, image=image_name, @@ -737,7 +733,6 @@ def _create(): mem_limit=mem_limit, cpu_shares=cpu_shares, host_config=host_config, - memswap_limit=memswap_limit, ) def _add(): diff --git a/flocker/node/functional/test_deploy.py b/flocker/node/functional/test_deploy.py index d2a226bde8..654c242fd9 100644 --- a/flocker/node/functional/test_deploy.py +++ b/flocker/node/functional/test_deploy.py @@ -371,8 +371,8 @@ def test_command_line_introspection(self): @if_docker_configured def test_swappiness_introspection(self): """ - The swappiness value discovered by the ApplicationDeployer is the same - as was passed in. + The swappiness value discovered by the ``ApplicationDeployer`` is the + same as was passed in. """ swappiness = 98 d = self._start_container_for_introspection(swappiness=swappiness) From 588c5764274b8fd45b32955c58a485507e07c56f Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2016 17:11:09 +0000 Subject: [PATCH 5/7] Discover the swappiness value and set it on the Application....also requires us to use a newer docker API version and some corresponding changes to the cpu shares and memory discovery. --- flocker/node/_docker.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/flocker/node/_docker.py b/flocker/node/_docker.py index 5178aa0db5..1c43da38da 100644 --- a/flocker/node/_docker.py +++ b/flocker/node/_docker.py @@ -495,7 +495,7 @@ def __init__( long_timeout=600): self.namespace = namespace self._client = dockerpy_client( - version="1.15", base_url=base_url, + version="1.20", base_url=base_url, long_timeout=timedelta(seconds=long_timeout), ) self._image_cache = LRUCache(100) @@ -717,6 +717,7 @@ def _create(): binds=binds, port_bindings=port_bindings, restart_policy=restart_policy_dict, + mem_swappiness=swappiness, ) # We're likely to get e.g. pvector, so make sure we're passing # in something JSON serializable: @@ -971,9 +972,9 @@ def _list(): # mem_limit in containers without specified limits, however # Docker returns the values in these cases as zero, so we # manually convert. - cpu_shares = data[u"Config"][u"CpuShares"] + cpu_shares = data[u"HostConfig"][u"CpuShares"] cpu_shares = None if cpu_shares == 0 else cpu_shares - mem_limit = data[u"Config"][u"Memory"] + mem_limit = data[u"HostConfig"][u"Memory"] mem_limit = None if mem_limit == 0 else mem_limit restart_policy = self._parse_restart_policy( data[U"HostConfig"][u"RestartPolicy"]) @@ -988,7 +989,8 @@ def _list(): mem_limit=mem_limit, cpu_shares=cpu_shares, restart_policy=restart_policy, - command_line=command) + command_line=command, + swappiness=data[u"HostConfig"][u"MemorySwappiness"]) ) return result return deferToThread(_list) From c758b1f7f20d6c5ecf01d75627519b14d10a92b1 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2016 17:22:20 +0000 Subject: [PATCH 6/7] Since Docker API 1.19 the mem_limit is configured through HostConfig --- flocker/node/_docker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/node/_docker.py b/flocker/node/_docker.py index 1c43da38da..a5ba585293 100644 --- a/flocker/node/_docker.py +++ b/flocker/node/_docker.py @@ -717,6 +717,7 @@ def _create(): binds=binds, port_bindings=port_bindings, restart_policy=restart_policy_dict, + mem_limit=mem_limit, mem_swappiness=swappiness, ) # We're likely to get e.g. pvector, so make sure we're passing @@ -731,7 +732,6 @@ def _create(): command=command_line_values, environment=environment, ports=[p.internal_port for p in ports], - mem_limit=mem_limit, cpu_shares=cpu_shares, host_config=host_config, ) From 7d389c821419f0ec59389c0d7e39e17cc466bfe8 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 9 Nov 2016 17:38:03 +0000 Subject: [PATCH 7/7] Add a test that swappiness changes trigger an application restart --- flocker/node/test/test_container.py | 43 +++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/flocker/node/test/test_container.py b/flocker/node/test/test_container.py index 9abf548a32..eed82bd0ac 100644 --- a/flocker/node/test/test_container.py +++ b/flocker/node/test/test_container.py @@ -1520,6 +1520,49 @@ def test_app_with_changed_links_restarted(self): self.assertEqual(expected, result) + def test_app_with_changed_swappiness_restarted(self): + """ + An ``Application`` running on a given node that has different + swappiness specified in the desired state to the swappiness specified + by the application's current state is added to the list of applications + to restart. + """ + api = ApplicationNodeDeployer( + u'node1.example.com', + docker_client=FakeDockerClient(), + ) + old_wordpress_app = Application( + name=u'wordpress-example', + image=DockerImage.from_string(u'clusterhq/wordpress:latest'), + volume=None, + swappiness=97, + ) + postgres_app = Application( + name=u'postgres-example', + image=DockerImage.from_string(u'clusterhq/postgres:latest') + ) + new_wordpress_app = old_wordpress_app.set("swappiness", 98) + + desired = Deployment(nodes=frozenset({ + Node(hostname=u'node1.example.com', + applications=frozenset({new_wordpress_app, postgres_app})), + })) + node_state = NodeState(hostname=api.hostname, + applications={postgres_app, old_wordpress_app}) + result = api.calculate_changes( + desired_configuration=desired, + current_cluster_state=DeploymentState(nodes={node_state}), + local_state=NodeLocalState(node_state=node_state), + ) + expected = sequentially(changes=[in_parallel(changes=[ + sequentially(changes=[ + StopApplication(application=old_wordpress_app), + StartApplication(application=new_wordpress_app, + node_state=node_state) + ]), + ])]) + self.assertEqual(expected, result) + def test_stopped_app_with_change_restarted(self): """ An ``Application`` that is stopped, and then reconfigured such that it