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.

Sign up for GitHub

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

Jump to bottom

MCLOUD-5859: Fix for conflicting volumes - #43 #168

Merged
merged 15 commits into from Mar 23, 2020
Merged

MCLOUD-5859: Fix for conflicting volumes - #43 #168

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
79a57db
Multiple Volume issue, Prefixed Volume name with app name
Mar 9, 2020
af79a37
removed Cloud source direct usage
Mar 9, 2020
894868f
added config file
Mar 10, 2020
e359fec
made name as required field
Mar 10, 2020
1357d88
used string instead of String
Mar 12, 2020
74bbca7
removed null validation on name
Mar 12, 2020
c52756d
resolved conflicts
Mar 12, 2020
ac52092
added get name to Config
Mar 12, 2020
5dcbb46
Merge branch 'develop' into MAGECLOUD-43
G-Arvind Mar 12, 2020
73b2508
empty space removed
Mar 12, 2020
1a9695e
support for production mode modified .exp files
Mar 13, 2020
f03d9d1
Updated the code from dev branch
Mar 19, 2020
f1dcc6c
Merge branch 'develop' into MAGECLOUD-43
G-Arvind Mar 19, 2020
313555b
Merge branch 'develop' into MAGECLOUD-43
G-Arvind Mar 19, 2020
cf3c926
Merge branch 'develop' into MAGECLOUD-43
G-Arvind Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/Compose/DeveloperBuilder.php
View file
Open in desktop
Expand Up @@ -83,6 +83,8 @@ public function __construct(
*/
public function build(Config $config): Manager
{
$volumePrefix = $config->getName() . '-';

$manager = $this->builderFactory
->create(BuilderFactory::BUILDER_PRODUCTION)
->build($config);
Expand All @@ -105,9 +107,9 @@ public function build(Config $config): Manager
}

$manager->setVolumes([
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better if the concatenation of the prefix and volumes fit inside the "Manager" object
Thus, you would not need to add a prefix every time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started to work on your comment but I have a few questions, this change introduces 4 if statements inside setVolumes() alone is this fine?, other than this should addService() and update service() present in Manager Class be able to prefix the volumes, or directly prefixing them in DeveloperBulider /ProductionBuilder is enough(Current Approach)?

Example snippet: like this 4cases should be done inside setVolumes()

if(array_key_exists( "magento-db", $volumes)) 
{
        $oldkey= "magento-db";
        $keys = array_keys($volumes);
        $keys[array_search($oldkey, $keys)] = $this->config->getName().'-'.$oldkey;
        $volumes= array_combine($keys, $volumes);
        $extConfig["volumes"]=$volumes;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The main idea of ​​my proposal was that prefix concatenation be implemented in the Manager class
After that, in the Manager, we can add a method to get the mounts for a specific service.

Now, in order to expedite the delivery, I suggest for now just updating the branch without rework.

self::VOLUME_MAGENTO_SYNC => $syncConfig,
self::VOLUME_MAGENTO_DB => [],
self::VOLUME_MARIADB_CONF => [
$volumePrefix . self::VOLUME_MAGENTO_SYNC => $syncConfig,
$volumePrefix . self::VOLUME_MAGENTO_DB => [],
$volumePrefix . self::VOLUME_MARIADB_CONF => [
'driver_opts' => [
'type' => 'none',
'device' => $this->resolver->getRootPath('/.docker/mysql/mariadb.conf.d'),
Expand Down Expand Up @@ -145,9 +147,9 @@ public function build(Config $config): Manager
'volumes' => array_merge(
$volumes,
[
self::VOLUME_MAGENTO_DB . ':/var/lib/mysql',
self::VOLUME_DOCKER_ETRYPOINT . ':/docker-entrypoint-initdb.d',
self::VOLUME_MARIADB_CONF . ':/etc/mysql/mariadb.conf.d',
$volumePrefix . self::VOLUME_MAGENTO_DB . ':/var/lib/mysql',
$volumePrefix . self::VOLUME_DOCKER_ETRYPOINT . ':/docker-entrypoint-initdb.d',
$volumePrefix . self::VOLUME_MARIADB_CONF . ':/etc/mysql/mariadb.conf.d',
]
)
]);
Expand Down Expand Up @@ -176,14 +178,16 @@ public function getPath(): string
*/
private function getMagentoVolumes(Config $config): array
{
$volumePrefix = $config->getName() . '-';

if ($config->getSyncEngine() !== self::SYNC_ENGINE_NATIVE) {
return [
self::VOLUME_MAGENTO_SYNC . ':' . self::DIR_MAGENTO . ':nocopy'
$volumePrefix . self::VOLUME_MAGENTO_SYNC . ':' . self::DIR_MAGENTO . ':nocopy'
];
}

return [
self::VOLUME_MAGENTO_SYNC . ':' . self::DIR_MAGENTO . ':delegated',
$volumePrefix . self::VOLUME_MAGENTO_SYNC . ':' . self::DIR_MAGENTO . ':delegated',
];
}
}
9 changes: 9 additions & 0 deletions src/Config/Config.php
View file
Open in desktop
Expand Up @@ -311,4 +311,13 @@ public function getPort(): string
{
return $this->get(SourceInterface::CONFIG_PORT);
}

/**
* @return String
* @throws ConfigurationMismatchException
*/
public function getName(): String
{
return $this->all()->get(SourceInterface::NAME);
}
}
22 changes: 22 additions & 0 deletions src/Config/Source/CloudSource.php
View file
Open in desktop
Expand Up @@ -89,6 +89,10 @@ public function read(): Repository
throw new SourceException('Relationships could not be parsed.');
}

if (!isset($appConfig['name'])) {
throw new SourceException('Name could not be parsed.');
}

[$type, $version] = explode(':', $appConfig['type']);
/**
* RC versions are not supported
Expand Down Expand Up @@ -126,6 +130,10 @@ public function read(): Repository
$repository,
$appConfig['relationships'] ?? []
);
$repository = $this->addName(
$repository,
$appConfig['name']
);

return $repository;
}
Expand Down Expand Up @@ -288,4 +296,18 @@ private function addMounts(Repository $repository, array $mounts): Repository

return $repository;
}

/**
* @param Repository $repository
* @param string $name
* @return Repository
*/
private function addName(Repository $repository, string $name): Repository
{
$repository->set([
self::NAME => $name,
]);

return $repository;
}
}
4 changes: 4 additions & 0 deletions src/Config/Source/ConfigSource.php
View file
Open in desktop
Expand Up @@ -50,6 +50,10 @@ public function read(): Repository
if ($this->filesystem->exists($configFile)) {
$config = Yaml::parseFile($configFile);

if (!isset($config['name'])) {
throw new SourceException('Name could not be parsed.');
}

/**
* Enable services which were added from the file by default
*/
Expand Down
1 change: 1 addition & 0 deletions src/Config/Source/SourceInterface.php
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ interface SourceInterface
public const DIR_MAGENTO = '/app';

public const MOUNTS = 'mounts';
public const NAME = 'name';

/**
* Services
Expand Down