WIP Docker PR #2537

tomjn · 2021-10-12T14:22:18Z

I don't expect to merge this PR as we'll create new PRs that take subsets of it out, or implement it differently. This will be similar to when we had M1 support, eventually this PR will have little to nothing in it and we can declare success!

Checks

I've updated the changelog.
I've tested this PR
This PR is for the develop branch not the stable branch.
This PR is complete and ready for review.

…-Vagrants/develop Prepare v3.6 release

…-Vagrants/develop 3.7.1 release

…-Vagrants/develop 3.7.2

- use direct binary call

- docker use same clock as host

- Docker doesn't support persistent /etc/hosts

- By default its always compressed

- I guess backward compat ?

tomjn

Some notes

README.md

tomjn · 2021-10-12T14:28:01Z

README.md

 vagrant plugin install --local
+
+# MAC Only ( see: https://docs.docker.com/desktop/mac/networking/#per-container-ip-addressing-is-not-possible )
+sudo ifconfig lo0 alias 192.168.50.4/24


I suspect this could be done from the vagrant file, though is the IP always the same?

i was thinking to add it to Vagrantfile, but not quite sure in which part and how to add it properly, basically we can sub 192.168.50.4/24 with vvv_config['vm_config']['private_network_ip'] to make it auto adjust.

The link in the comment doesn't mention the command here, I'm unsure how this is meant to work or where it comes from.

It's also not how I've seen other docker set ups work, how do other docker environments get around this?

docker in linux have network bridge accessible in host, so we can assign an IP address to a container, and ping or connect to that IP address from host. Meanwhile in Mac, this network bridge is not accessible by the host, so we can't ping or connect to that IP address ( 192.168.50.4 ) and basically the custom domain we added *.test is inaccessible, because goodhosts mapped it to 192.168.50.4

Vagrantfile

tomjn · 2021-10-12T14:29:40Z

Vagrantfile

@@ -432,6 +451,21 @@ Vagrant.configure('2') do |config|
    override.vm.box = 'bento/ubuntu-20.04'
  end

+  # Docker use image.
+  config.vm.provider :docker do |d|
+    d.image = 'pentatonicfunk/vagrant-ubuntu-base-images:20.04'


I'm guessing this image gets built as a part of provisioning?

this image is "built" image already. provisioning only pulling it in.
https://github.com/pentatonicfunk/docker-vagrant-ubuntu-base-images
https://hub.docker.com/repository/docker/pentatonicfunk/vagrant-ubuntu-base-images
what do you think about these ?

I'm happy to completely remove Ubuntu 18 as an option, it's only there for longterm compatibility at the moment, and at some point Ondrej packages for 18 will disappear. As for prebuilt images I'm not sure

provision/core/nginx/config/nginx.conf

tomjn · 2021-10-12T14:40:03Z

provision/core/vvv/provision.sh

+    # not included in docker images by default, lets add
+    iputils-ping
+    net-tools
+    nano
+    less


are these needed or are they just useful for developing this?

less is the most required ( for wp cli ) and its not included in my docker image yet.
i will add these packages to docker image pentatonicfunk/docker-vagrant-ubuntu-base-images#1

I'd rather we were explicit about these things for portability

@pentatonicfunk I added most of these a while back, just added nano in a draft PR seeing if I can take any more of this PR back into VVV core. At least less/net-tools/etc are all listed twice here

provision/core/vvv/provision.sh

provision/provision-helpers.sh

tomjn · 2021-10-12T14:41:42Z

provision/provision-helpers.sh

+function get_vvv_sites() {
+  local sites=$(shyaml -q keys "sites" <${VVV_CONFIG})
+  echo "${sites}"
+}
+
+function re_init_etc_hosts() {


vvv_get_sites and vvv_update_guest_hosts

pentatonicfunk#7

taken from #2537

It seems it's already disabled Taken from #2537: ``` # text/html Responses with the “text/html” type are always compressed. https://nginx.org/en/docs/http/ngx_http_gzip_module.html#gzip_types ``` https://nginx.org/en/docs/http/ngx_http_gzip_module.html#gzip_types

pentatonicfunk · 2021-10-17T01:15:53Z

@tomjn totally agree with you, this fork and PR merely a PoC and reference for the future when we actually start to fully support docker -- that if we really want to, and won't be a pain point to maintain.

And if there are any stuff here that can improve VVV in general, we can can just cherry pick it and make mini separate PRs like you already did. I don't even expect this to be merged at all, my main intention was to experiment, see how VVV will behave ( especially the provision part ) if i do use it with docker in certain way ( monolithic way ), and push my self to make it somewhat usable, and hopefully some good things get picked up after that.

As for services based containers ( docker-compose kind of ? ), that seems like the standard / ideal way in docker world. Although for development, based on my own personal experience, i can't say that i am comfortable with multi containers, having to interact with different container for different task is tedious. e.g. when i need to interact with mysql i need to go to mysql container, but if i wanna quick modify nginx config i need to interact with nginx container and so on. Meanwhile in development phase, these kind of tasks are often needed to be done and done quickly. Again, its just me. I am not familiar and not get used to it. One of the nice thing i love about VVV / vagrant, is that vagrant ssh and hack everything i want inside that single shell instance, be it modify nginx config or just tailing specific logs.

PS: i really got great input and insights from here, thanks a lot for doing it!

- Solves #8 and #10

…-Vagrants/develop Release 3.8.1

# Conflicts: # config/homebin/vagrant_provision # config/homebin/vagrant_up # provision/core/env/provision.sh # provision/core/mailhog/provision.sh # provision/core/mariadb/provision.sh # provision/core/nginx/config/nginx.conf # provision/provision-helpers.sh # provision/provision.sh

# Conflicts: # Vagrantfile

…beta # Conflicts: # README.md # provision/provision-helpers.sh

tomjn · 2022-09-01T12:52:44Z

provision/core/mariadb/provision.sh

  if [ "${VVV_DOCKER}" != 1 ]; then
    check_mysql_root_password
  fi

  # MySQL gives us an error if we restart a non running service, which
  # happens after a `vagrant halt`. Check to see if it's running before
  # deciding whether to start or restart.
-  if [ $(service mariadb status|grep 'mysql start/running' | wc -l) -ne 1 ]; then
+  if [ $(service mariadb status|grep 'Uptime' | wc -l) -ne 1 ]; then


this doesn't work on Parallels based VMs and maybe others, would it not be better to check for running or active?

tomjn · 2022-09-01T12:54:30Z

provision/core/mailhog/provision.sh

-  if [ "${VVV_DOCKER}" != 1 ]; then
-    service mailhog restart
-  fi
+  killall mailhog; /usr/bin/env /usr/local/bin/mailhog > /dev/null 2>&1 &


what happens if you run sudo service mailhog restart on the docker VM?

at the moment it saysmailhog: unrecognized service

vagrant@vvv:~$ sudo service mailhog restart mailhog: unrecognized service vagrant@vvv:~$

tomjn · 2022-09-01T12:55:44Z

database/sql/init.sql

@@ -1,7 +1,7 @@
 # Create the unit tests DB.
 CREATE DATABASE IF NOT EXISTS `wordpress_unit_tests`;
 GRANT ALL PRIVILEGES ON `wordpress_unit_tests`.* TO 'wp'@'localhost' IDENTIFIED BY 'wp';
-GRANT ALL PRIVILEGES ON `wordpress_unit_tests`.* TO 'wp'@'192.168.50.1' IDENTIFIED BY 'wp';
+GRANT ALL PRIVILEGES ON `wordpress_unit_tests`.* TO 'wp'@'192.168.56.1' IDENTIFIED BY 'wp';


could these lines co-exist rather than requiring an IP change? Is there a specific reason the IP is needed? E.g. for external use?

to be honest i dont know what should be the correct one here, i think the only reason i changed it is we changed the default private ip network here: 39ee551

adding both / co-exists also fine

GRANT ALL PRIVILEGES ON `wordpress_unit_tests`.* TO 'wp'@'192.168.50.1' IDENTIFIED BY 'wp'; GRANT ALL PRIVILEGES ON `wordpress_unit_tests`.* TO 'wp'@'192.168.56.1' IDENTIFIED BY 'wp';

not sure if thats the correct thing though.

external use, no. and its not required by / related with docker

Ah in that case lets kill the 50.1 line, but I wouldn't expect local loopback to go over that network inside the container

tomjn · 2022-09-01T12:56:21Z

config/homebin/vagrant_up

+vvv_info " * Restarting PHP-FPM"
+find /etc/init.d/ -name "php*-fpm" -exec bash -c 'sudo service "$(basename "$0")" restart' {} \;
+
+vvv_info " * Restarting Memcache"
+sudo service memcached restart
+
+vvv_info " * Restarting Mailhog"
+/usr/bin/env /usr/local/bin/mailhog > /dev/null 2>&1 &
+


these could be separate functions so that we don't have to introduce docker specifics in these files, and can put them all in one place without duplication

tomjn · 2022-09-01T13:01:06Z

@pentatonicfunk I've started a PR to see what more I can merge into VVV core, and left some more comments to try and clarify or suggest some things, I'm a little strapped for time today but intend to give this a proper run test sometime this week

pentatonicfunk · 2022-09-01T13:54:11Z

thanks @tomjn, i'll try to update my repo as per your suggestions

tomjn · 2022-09-13T10:33:53Z

@pentatonicfunk I've started #2632 as I couldn't modify this branch, and merged some changes and made others compatible with non-docker.

Would you like to continue there if I add you as a repo member? We're not far from getting this in and it's a big milestone

pentatonicfunk · 2022-09-13T12:29:12Z

yes for sure

- wildcard

tomjn · 2022-11-13T17:40:59Z

closing this as we have #2632

tomjn and others added 26 commits March 16, 2021 09:30

Merge pull request Varying-Vagrant-Vagrants#2375 from Varying-Vagrant…

22ad4b4

…-Vagrants/develop Prepare v3.6 release

3.6.1 (Varying-Vagrant-Vagrants#2394)

c9ec449

3.6.2

62c5796

Merge pull request Varying-Vagrant-Vagrants#2494 from Varying-Vagrant…

ace8de7

…-Vagrants/develop 3.7.1 release

Merge pull request Varying-Vagrant-Vagrants#2511 from Varying-Vagrant…

92758fd

…-Vagrants/develop 3.7.2

Init README

0b6b2ac

Init provision

188bcaf

Removing disksize reference

a646c93

Explicit noninteractive DEBIAN_FRONTEND

ce156b6

Attempt to fix mailhog start and restart

a40ab54

- use direct binary call

Downgrade mariadb to 10.3

e78b6f9

Disable sync clocks ntpdate

d80d105

- docker use same clock as host

Add apt-utils and add /run/php dir manually

79a57ea

Update is_module_enabled_fpm

48ab479

Fix type

23464a6

Refactor mailhog restart

7fd737e

Fix vagrant up for docker container

76a008b

remove unnen

b8a0c44

Reinit /etc/hosts on up

fdade8b

- Docker doesn't support persistent /etc/hosts

Failsafe delete temporary hosts file

d6ed345

Cleanup

a8101df

Mac OS compatibility

636a530

Mariadb 10.5 compat

da3b697

Remove text/html from nginx gzip_types

de8c959

- By default its always compressed

Override debian.cnf

58a2c88

- I guess backward compat ?

Add less package for wp cli

729da51

tomjn commented Oct 12, 2021

View reviewed changes

tomjn added a commit that referenced this pull request Oct 12, 2021

improve ssh service reloading

3ad8848

taken from #2537

tomjn mentioned this pull request Oct 12, 2021

Improve ssh service reloading #2538

Merged

4 tasks

pentatonicfunk added 7 commits October 17, 2021 08:50

Remove debug #6

1cda256

Keep vm.box for non docker compat #4

ff1b71b

Move VB box to its own specific section to not break docker #4

d103405

Minimize default config changes #5

4acaa62

Follow vvv function name convention #7

f0ae39a

Attempt to migrate cat and append to tee

39d794f

Improve macos detection, and move ifconfig command to Vagrantfile

47057c8

- Solves #8 and #10

tomjn mentioned this pull request Nov 12, 2021

WIP docker compat enhancements #2555

Merged

4 tasks

tomjn and others added 8 commits November 15, 2021 11:03

Merge pull request Varying-Vagrant-Vagrants#2557 from Varying-Vagrant…

8252690

…-Vagrants/develop Release 3.8.1

Fix: more new private ip address adjusment

42efd1c

3.9.1 stable release

fabccb8

Merge remote-tracking branch 'upstream/stable' into docker-provider-beta

d442355

# Conflicts: # Vagrantfile

Fix / revert default box

1da5dd1

Update readme, prepare using develop as upstream

00d7499

Merge remote-tracking branch 'upstream/develop' into docker-provider-…

ef6106b

…beta # Conflicts: # README.md # provision/provision-helpers.sh

tomjn mentioned this pull request Sep 1, 2022

Docker PR integrations #2628

Merged

4 tasks

tomjn commented Sep 1, 2022

View reviewed changes

pentatonicfunk added 2 commits September 30, 2022 09:11

Attempt to fix git safe dir issue

dd9691e

- wildcard

Fix git safe.directory script

312aed8

tomjn closed this Nov 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP Docker PR #2537

WIP Docker PR #2537

tomjn commented Oct 12, 2021

tomjn left a comment

tomjn Oct 12, 2021

pentatonicfunk Oct 16, 2021

tomjn Oct 16, 2021

pentatonicfunk Oct 17, 2021

tomjn Oct 12, 2021

pentatonicfunk Oct 16, 2021

tomjn Oct 16, 2021

tomjn Oct 12, 2021

pentatonicfunk Oct 16, 2021

tomjn Nov 12, 2021

tomjn Sep 1, 2022

tomjn Oct 12, 2021

pentatonicfunk Oct 16, 2021

pentatonicfunk commented Oct 17, 2021

tomjn Sep 1, 2022

tomjn Sep 1, 2022

pentatonicfunk Sep 1, 2022

tomjn Sep 1, 2022

pentatonicfunk Sep 1, 2022

tomjn Sep 1, 2022

tomjn Sep 1, 2022

tomjn commented Sep 1, 2022

pentatonicfunk commented Sep 1, 2022

tomjn commented Sep 13, 2022

pentatonicfunk commented Sep 13, 2022

tomjn commented Nov 13, 2022

WIP Docker PR #2537

WIP Docker PR #2537

Conversation

tomjn commented Oct 12, 2021

Checks

tomjn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pentatonicfunk commented Oct 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomjn commented Sep 1, 2022

pentatonicfunk commented Sep 1, 2022

tomjn commented Sep 13, 2022

pentatonicfunk commented Sep 13, 2022

tomjn commented Nov 13, 2022