Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLOC-4530] Pass the swappiness configuration value to the Docker API #2946

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions flocker/node/_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)


Expand Down Expand Up @@ -315,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

Expand Down
17 changes: 7 additions & 10 deletions flocker/node/_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -717,27 +717,23 @@ 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
# in something JSON serializable:
command_line_values = command_line
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,
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,
memswap_limit=memswap_limit,
)

def _add():
Expand Down Expand Up @@ -976,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to do a similar thing for swappiness here?

restart_policy = self._parse_restart_policy(
data[U"HostConfig"][u"RestartPolicy"])
Expand All @@ -993,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)
Expand Down
15 changes: 15 additions & 0 deletions flocker/node/functional/test_deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
82 changes: 82 additions & 0 deletions flocker/node/test/test_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why dont we use a smaller image here? Any reason we are using postgresql just to test the swappiness? Could mean longer pulls for new test nodes.

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):
"""
Expand Down Expand Up @@ -677,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
Expand Down Expand Up @@ -1481,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
Expand Down