Merge pull request #14 from iamluc/feature/refacto

Minor refacto + bug fixes
iamluc · Jan 5, 2016 · 832ed19 · 832ed19
2 parents b8c8a03 + b092f5e
commit 832ed19
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 67 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -12,14 +12,14 @@ install: composer install --prefer-source
 script: vendor/bin/phpunit --configuration phpunit.xml.dist
 
 before_deploy:
-    - curl -LSs http://box-project.github.io/box2/installer.php | php
-    - php box.phar build
+  - curl -LSs http://box-project.github.io/box2/installer.php | php
+  - php box.phar build
 
 deploy:
   provider: releases
   skip_cleanup: true
   api_key:
-    secure: NuHZx1ORZGojbPuY2ZZrB1BNfe2UUmO5wBv4dM6Qu2WluU0zp96LmKlOsbNml6k167cKj4R1AHgbVsbBARjtUeoYNBYti1G5/m2iXjRRqfPGNkuhG7A2THnYtrLqRb2jCI3hypsZSpOtj//4N+Q3CEKT9nd69HeV90/cAFd+YgDN5ldk9mhHbMnFAJ5hAFzYzUg2zukExVWObWkQ9Y3nP/zJ8jQvmvfiS2yLO+DjPEDH63rQorCbFh9SI0hSWXUQxTtbUk8Jj1H46Mrp9RAZlNhfEL2HiUt4S6lKciCQxkZ3P9yXp8ebjYPqH7fPPhn8G3ewYQqGFG/AYQGX463eyh0VAIt4PbJyIeqtEoPKC1h16b7a9tVt2kroQMImkYV1pwKg/aNZ0x9T3PXBjaCU8Mb5qrUvYpIoxjONI2qxi6lDV0kikumzhFnbfayFTA004ybl+yL1Ag3LxXyqo2jeu3vzX0e9kSDbQD1Cf4lp0riDkbKIDrJ8Ptqv67QV+dcAPyynDkRYDvPoSRKnraVKtHeRu1P5XHH7gpL/xdU8UnvO83ofDaLu923fkDE/dlGNhR/MU8P4EMWC4U/y0ROp1Wx27+qatrzCFZBlJVjtq6urTZ5Z6fMrD9f/Nelu8UR4CI8hnWMFl+UQVoQpSDYYVh71DHPU1QK+A/TpGI4GYm8=
+    secure: "xYPmNWQNTRqLSQinnM64dRqDGAm6bKYODFmGdVLwjNqbICMHR0/sF00EPwBgIsXF100xbnjgJVRXD7lbL7nUYXWyzR2EcaW8yOxhYC0BYQMpOGI6RF0p+sMskCI5hb8Y+FT++nQZTwHPo9MtrAv2ec5r42RO2K0YM6WS6URsCqbTQnDtWcLReszRrLGVy41tkdlse9uqk63IbJmRwLunMkQBJ/BhVSRDl5Qm+Q3aDhjckZanX4QH0UrR75azut3CUIQ1l/wVF9dhKPHuvIc5+3qwkxqgOmaBFozE2hvlviWCunQsZMpaWG9L3v19VzuvypDvvvK+rhwytXsOO2gz0JGh/AL6TsonGqePYdESE7tBZ+sJz5tZ0q0yqEOLGSlxa7i5bF3KN3PCqK8eBdgBHWWDnWgO0blmPFKLYaehxZqnDHr8w5bHlW2yS1fYq8X5zkmz1fbkjpPFXX6TWsm8imlKsqzhSPBTrF+E/6f91TOLlv7tXIA0hi7Ex4ZOuzUCSs6qYWfPYPKWnguL8kmkG9wKnFahQwxVz2CM2ZPxhNQ8j03ao+wkBr6+pt6KhghqYJQ83c5GrzDXJWXbpuNdFt+RfPVLT0w17tWj3H80b/QZFa2TKQtSZ41jnAKPHHt3m178s3DfpQ6Hf1Zi1kvru0jiGKdph94j7zcYeB7uV3A="
   file: bin/docker-hostmanager.phar
   on:
     tags: true
diff --git a/Dockerfile b/Dockerfile
@@ -2,7 +2,7 @@ FROM iamluc/composer
 
 ADD . /usr/local/src/docker-hostmanager
 
-RUN composer install --no-interaction --prefer-dist --working-dir=/usr/local/src/docker-hostmanager \
+RUN composer install --no-interaction --no-dev --prefer-dist --working-dir=/usr/local/src/docker-hostmanager \
     && ln -s /usr/local/src/docker-hostmanager/bin/docker-hostmanager /usr/local/bin/docker-hostmanager
 
 ENV HOSTS_FILE=/hosts

diff --git a/bin/docker-hostmanager b/bin/docker-hostmanager
@@ -7,9 +7,10 @@ if (PHP_SAPI !== 'cli') {
 
 require __DIR__.'/../vendor/autoload.php';
 
-use DockerHostManager\Command\RunCommand;
+use DockerHostManager\Command\SynchronizeHostsCommand;
 use Symfony\Component\Console\Application;
 
 $application = new Application('DockerHostManager', '@package_version@');
-$application->add(new RunCommand());
+$application->add(new SynchronizeHostsCommand());
+$application->setDefaultCommand('synchronize-hosts');
 $application->run();
diff --git a/src/Command/RunCommand.php → src/Command/SynchronizeHostsCommand.php b/src/Command/RunCommand.php → src/Command/SynchronizeHostsCommand.php
@@ -2,34 +2,39 @@
 
 namespace DockerHostManager\Command;
 
-use DockerHostManager\Application;
+use Docker\Http\DockerClient;
+use DockerHostManager\Docker\Docker;
+use DockerHostManager\Synchronizer;
 use Symfony\Component\Console\Command\Command;
-use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
-class RunCommand extends Command
+class SynchronizeHostsCommand extends Command
 {
     protected function configure()
     {
         $this
             ->setName('synchronize-hosts')
             ->setDescription('Run the application')
-            ->addArgument(
+            ->addOption(
                 'entrypoint',
-                InputArgument::OPTIONAL,
+                'p',
+                InputOption::VALUE_REQUIRED,
                 'The docker entrypoint',
                 getenv('DOCKER_ENTRYPOINT') ?: 'unix:///var/run/docker.sock'
             )
-            ->addArgument(
+            ->addOption(
                 'hosts_file',
-                InputArgument::OPTIONAL,
+                'f',
+                InputOption::VALUE_REQUIRED,
                 'The host file to update',
                 getenv('HOSTS_FILE') ?: '/etc/hosts'
             )
-            ->addArgument(
+            ->addOption(
                 'tld',
-                InputArgument::OPTIONAL,
+                't',
+                InputOption::VALUE_REQUIRED,
                 'The TLD to use',
                 getenv('TLD') ?: '.docker'
             )
@@ -38,10 +43,12 @@ protected function configure()
 
     protected function execute(InputInterface $input, OutputInterface $output)
     {
-        $app = new Application(
-            $input->getArgument('entrypoint'),
-            $input->getArgument('hosts_file'),
-            $input->getArgument('tld')
+        $client = new DockerClient([], $input->getOption('entrypoint'));
+
+        $app = new Synchronizer(
+            new Docker($client),
+            $input->getOption('hosts_file'),
+            $input->getOption('tld')
         );
 
         $app->run();

diff --git a/src/Application.php → src/Synchronizer.php b/src/Application.php → src/Synchronizer.php
@@ -3,91 +3,74 @@
 namespace DockerHostManager;
 
 use Docker\Container;
+use Docker\Exception\APIException;
 use Docker\Exception\ContainerNotFoundException;
 use DockerHostManager\Docker\Docker;
 use DockerHostManager\Docker\Event;
-use Docker\Http\DockerClient;
 
-class Application
+class Synchronizer
 {
     const START_TAG = '## docker-hostmanager-start';
     const END_TAG = '## docker-hostmanager-end';
 
-    /** @var string */
-    private $entrypoint;
+    /** @var Docker  */
+    private $docker;
     /** @var string */
     private $hostsFile;
     /** @var string */
     private $tld;
 
-    /** @var Docker  */
-    private $docker;
     /** @var array Container */
     private $activeContainers = [];
 
     /**
-     * @param string $entrypoint
+     * @param Docker $docker
      * @param string $hostsFile
      * @param string $tld
      */
-    public function __construct($entrypoint, $hostsFile, $tld)
+    public function __construct(Docker $docker, $hostsFile, $tld)
     {
-        $this->entrypoint = $entrypoint;
+        $this->docker = $docker;
         $this->hostsFile = $hostsFile;
         $this->tld = $tld;
-        $client = new DockerClient([], $this->entrypoint);
-        $this->docker = new Docker($client);
     }
 
     public function run()
     {
+        if (!is_writable($this->hostsFile)) {
+            throw new \RuntimeException(sprintf('File "%s" is not writable.', $this->hostsFile));
+        }
+
         $this->init();
         $this->listen();
     }
 
     private function init()
     {
-        $this->activeContainers = array_filter($this->docker->getContainerManager()->findAll(), function (Container $container) {
-            $this->docker->getContainerManager()->inspect($container);
+        foreach ($this->docker->getContainerManager()->findAll() as $container) {
+            if ($this->isExposed($container)) {
+                $this->activeContainers[$container->getId()] = $container;
+            }
+        }
 
-            return $this->isExposed($container);
-        });
         $this->write();
     }
 
     private function listen()
     {
         $this->docker->listenEvents(function (Event $event) {
             $container = $this->docker->getContainerManager()->find($event->getId());
-            $this->docker->getContainerManager()->inspect($container);
+            if (null === $container) {
+                return;
+            }
+
             if ($this->isExposed($container)) {
-                $this->addActiveContainer($container);
+                $this->activeContainers[$container->getId()] = $container;
             } else {
-                $this->removeActiveContainer($container);
+                unset($this->activeContainers[$container->getId()]);
             }
-            $this->write();
-        });
-    }
 
-    /**
-     * @param Container $container
-     */
-    private function addActiveContainer(Container $container)
-    {
-        $id = $container->getId();
-        if (!empty($this->activeContainers[$id])) {
-            return;
-        }
-        $this->activeContainers[$id] = $container;
-    }
-
-    /**
-     * @param Container $container
-     */
-    private function removeActiveContainer(Container $container)
-    {
-        $this->activeContainers = array_filter($this->activeContainers, function (Container $c) use ($container) {
-            return $c->getId() !== $container->getId();
+            $this->write();
         });
     }
 
@@ -121,8 +104,6 @@ function (Container $container) {
      */
     private function getHostsLines(Container $container)
     {
-        $this->docker->getContainerManager()->inspect($container);
-
         $lines = [];
         $hosts = $this->getContainerHosts($container);
         foreach ($this->getContainerIps($container) as $ip) {
@@ -183,6 +164,13 @@ private function getContainerHosts(Container $container)
      */
     private function isExposed(Container $container)
     {
+        try {
+            $this->docker->getContainerManager()->inspect($container);
+        } catch (APIException $e) {
+            // Happen on "docker build"
+            return false;
+        }
+
         $inspection = $container->getRuntimeInformations();
         if (empty($inspection['NetworkSettings']['Ports']) || empty($inspection['State']['Running'])) {
             return false;

diff --git a/tests/DockerHostManager/ApplicationTest.php → tests/DockerHostManager/SynchronizerTest.php b/tests/DockerHostManager/ApplicationTest.php → tests/DockerHostManager/SynchronizerTest.php
@@ -3,17 +3,19 @@
 namespace Test\DockerHostManager;
 
 use Docker\Docker;
-use Docker\Http\DockerClient;
-use DockerHostManager\Application;
+use DockerHostManager\Synchronizer;
 use Test\Utils\PropertyAccessor;
 
-class ApplicationTest extends \PHPUnit_Framework_TestCase
+class SynchronizerTest extends \PHPUnit_Framework_TestCase
 {
     public function testThatAppCanBeConstructed()
     {
-        $application = new Application('unix:///var/run/docker.sock', '/etc/hosts', 'docker');
+        $docker = $this->prophesize('DockerHostManager\Docker\Docker');
+        $docker = $docker->reveal();
 
-        $this->assertSame('unix:///var/run/docker.sock', PropertyAccessor::getProperty($application, 'entrypoint'));
+        $application = new Synchronizer($docker, '/etc/hosts', 'docker');
+
+        $this->assertSame($docker, PropertyAccessor::getProperty($application, 'docker'));
         $this->assertSame('/etc/hosts', PropertyAccessor::getProperty($application, 'hostsFile'));
         $this->assertSame('docker', PropertyAccessor::getProperty($application, 'tld'));
         $this->assertInstanceOf(Docker::class, PropertyAccessor::getProperty($application, 'docker'));

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -1,3 +1,3 @@
 <?php
 
-require __DIR__ . '/../vendor/autoload.php';
+require __DIR__.'/../vendor/autoload.php';