Refactor to make RDS db version a configurable field

@yas please review this patch. This lets the administrator change the MariaDB version for tenants.

@baldwinlouie

Thank you for the patch. I left my comment. Thanks!

@yas, I think we should keep Drupal 9 support for the meantime. We have users testing this and I don't want them to rebuild all their environment to Drupal 10 to continue testing.

I can update the patch and remove TimeZoneFormHelper for now.

What do you think?

@baldwinlouie

I think we don't have a strong motivation to support Drupal 9. Here are the reasons:

• drupal/facade has not officially released yet.
• Drupal 9 will be EOL within this year.
• Eventually we need to refactor system_time_zones once Drupal 10.1.x is released.
• Our users testing the module should upgrade Drupal 10; eventually they need to rebuild the test environment for the production use.

@yas, I agree with your points. There's some additional work to make sure we don't break composer installs.

First, the 5.x branch of https://github.com/docomoinnovations/cloud_orchestrator/tree/5.x contains Facade. If we support 10.x, only, we should remove Facade from the 5.x version of composer.json. Luckily, 5.0.1 never included Facade, so we are good with that.

Secondly, we should quickly update our community CFn templates to supports 10.x. That includes updating the build script to install PHP 8.1. The community Docker file at https://git.drupalcode.org/project/cloud/-/blob/6.x/deployments/docker/D... needs to change to use php 8.1 too.

When working on Facade's specific CFn template, I've already update that to use php 8.1. See https://git.drupalcode.org/project/facade/-/blob/1.0.x/modules/tenants/o... and https://git.drupalcode.org/project/facade/-/blob/1.0.x/modules/tenants/o...

We can move the relevant code over to the community hosted versions.

Our users can then rebuild using the 6.x version Cloud Orchestrator.

What do you think?

@baldwinlouie

Thank you for giving me your thoughts.

First, the 5.x branch of https://github.com/docomoinnovations/cloud_orchestrator/tree/5.x contains Facade. If we support 10.x, only, we should remove Facade from the 5.x version of composer.json. Luckily, 5.0.1 never included Facade, so we are good with that.

I see your point that I've missed. In that case, we can remove drupal/facade from docomoinnovations/cloud_orchesrator:5.x-dev version. Then, I found that it might be necessary to remove drupal/facade from even docomoinnovations/cloud_orchestrator:6.x-dev version since it can be the constraint to use drupal/core:^10.1 for all related-modules (?).

In that sense, we can take either one of the strategies as follows:

1. We can only support drupal/core:10.1.x or higher with drupal/cloud:7.x-dev (*), that should be coming soon, then we can include facade module into it. OR
2. We can create a new drupal/facade:2.0.x-dev branch, and we only support drupal/core:10.1.x or higher on it. We need to create two patches for drupal/facade:1.0.x-dev and 2.0.x-dev for each Drupal version, though.

Secondly, we should quickly update our community CFn templates to supports 10.x. That includes updating the build script to install PHP 8.1. The community Docker file at https://git.drupalcode.org/project/cloud/-/blob/6.x/deployments/docker/D... needs to change to use php 8.1 too.

Thank you for taking care of the upgrades.

Our users can then rebuild using the 6.x version Cloud Orchestrator.

Or, we can ask them to install drupal/facade manually (from PHP composer) until we release docomoinnovations/cloud_orchestrator:7.x-dev.

(*) We need to drop the support 10.0.x or less since we have the similar problem in drupal/cloud. See the patch at 💬 Drupal 10.1.x compatibility: Refactor system_time_zones() to use \Drupal\Core\Datetime\TimeZoneFormHelper::getOptionsList() RTBC and it can be only merged into drupal/cloud:7.x-dev, which will only support drupal/core:10.1.x-dev (or higher).

What do you think?

@yas, thank you for your explanation.

I still need to keep Facade in 6.x version of Cloud Orchestrator. The reason is that when building a Tenant, I need to enable Facade Remote Worker, so that the Facade can communicate with the Tenant's Cloud Orchestrator. The module contains a Drush command (for use during initial setup) and it configures the REST api.

If we remove Facade from 6.x version of composer.json, I can manually include it in the CFn build script. That's not a problem.

However, since you stated the following, we should still continue to support drupal/core:^10.0 until a stable drupal/core:^10.1 release is available (ETA is June 2023)

Then, I found that it might be necessary to remove drupal/facade from even docomoinnovations/cloud_orchestrator:6.x-dev version since it can be the constraint to use drupal/core:^10.1 for all related-modules (?).

Therefore, I think the appropriate action is as follows:
1. Don't do anything with the composer.json files right now.
2. Revert to system_timezone() for now, to keep drupal/core:^10.0 support
3. We align with Cloud module's timeline to support drupal/core:^10.1. When Cloud releases 7.x-dev, we will release a Facade:2.0.x branch which is Drupal 10.1 only.

What do you think?

@baldwinlouie

The reason is that when building a Tenant, I need to enable Facade Remote Worker, so that the Facade can communicate with the Tenant's Cloud Orchestrator.

I see, Thank you for your explanation. It really makes sense to me.

2. Revert to system_timezone() for now, to keep drupal/core:^10.0 support

Agreed. We need to change system_timezone() to TimeZoneFormHelper when we create drupal/cloud:7.x-dev and drupal/facade:2.x-dev in near future, but it should be fine to keep system_timezone() as it is for now. Please update the patch.

Thanks!

@yas, here is the updated patch.

@baldwinlouie

Thank you for the update. It looks good. I'll merge the patch to 1.0.x, and close this issue as Fixed.

Automatically closed - issue fixed for 2 weeks with no activity.