TritonDataCenter / sdc-docker

Docker Engine for Triton
Mozilla Public License 2.0
183 stars 49 forks source link

DOCKER-458: Can't specify memory for containers with 1.7 client #43

Closed misterdjules closed 9 years ago

misterdjules commented 9 years ago

Instead of getting the value Memory from just the root of the container object, which is where docker clients < 1.7 put it, we also check in container.HostConfig.Memory, which is where docker clients >= 1.7 put it.

This commit adds integration tests for this change. These tests create docker containers with a memory value different than the default memory value and mimic the behavior of docker clients pre and post 1.7.

This commit also moves loadConfigSync to its own module so that it can be used by tests that need to load the configuration to lookup default values (such as defaultMemory in this case).

misterdjules commented 9 years ago

/cc @joshwilsdon @trentm @twhiteman

misterdjules commented 9 years ago

@joshwilsdon Updated the changes according to your code review, thank you :)

joshwilsdon commented 9 years ago

I notice there's still MEMORY_IN_MBS while BYTES_IN_MBS was changed to BYTES_IN_MB, should we change MEMORY_IN_MBS too?

Otherwise: LGTM

misterdjules commented 9 years ago

@joshwilsdon I wrote MEMORY_IN_MBS as "This value represents megabytes of data", hence the plural form, whereas BYTES_IN_MBS was changed to BYTES_IN_MB since it means "How many bytes in one megabyte`. However my English is not that great, so if that's not correct I'll change it :)

misterdjules commented 9 years ago

Landed in c0cc2ee5c4bb77416c15f8a29bf60e37f83760d3. Thank you @joshwilsdon :+1: