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.

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

Update Docker Image to Ubuntu 24.04 ( Noble ) #2739

Open
wants to merge 38 commits into
base: develop
Choose a base branch
from

Conversation

pentatonicfunk
Copy link
Contributor

Checks

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

Note on testing:

  • Currently i only have linux host testing machine. I've tested with Docker provider, and it went okay, logs attached
  • Tried using Virtualbox, but it keeps timing out / error
Timed out while waiting for the machine to boot. This means that
Vagrant was unable to communicate with the guest machine within
the configured ("config.vm.boot_timeout" value) time period.

If you look above, you should be able to see the error(s) that
Vagrant had when attempting to connect to the machine. These errors
are usually good hints as to what may be wrong.

If you're using a custom box, make sure that networking is properly
working and you're able to connect to the machine. It is a common
problem that networking isn't setup properly in these boxes.
Verify that authentication configurations are also setup properly,
as well.

If the box appears to be booting properly, you may want to increase
the timeout ("config.vm.boot_timeout") value.
  • If anyone can help testing it, please

Changelogs:

  • Please advise

tomjn and others added 23 commits September 10, 2022 11:55
- Add OVH url in hosts_to_test ( keep the mariadb.gb.ssimn.org )
- Better messages
Copy link

update-docs bot commented Oct 20, 2024

Thanks for opening this pull request! Make sure CHANGELOG.md gets updated with this change, additionally any docs that need updated can be found at https://github.com/Varying-Vagrant-Vagrants/varyingvagrantvagrants.org

GitHub
The VVV docs and website. Contribute to Varying-Vagrant-Vagrants/varyingvagrantvagrants.org development by creating an account on GitHub.

@tomjn
Copy link
Member

tomjn commented Oct 20, 2024

@pentatonicfunk how much of this can we split into separate PRs? Upgrading Ubuntu isn't a small task but adding the sources could be done and released pretty quickly to simplfy merging. Likewise some of the other changes are useful and quick to test on their own.

I am mindful that upgrading Ubuntu the last time resulted in some weirdness that needed investigation.

Also, the service changes, I was thinking of changing how services are restarted to use a VVV specific function to insulate against service and systemctl since there's a lot of duplication going on

@pentatonicfunk
Copy link
Contributor Author

i will try to cherry picks into separate branches next week.

This was referenced Oct 20, 2024
@@ -146,6 +146,9 @@ general:
# GitHub token to use from composer
#github_token: xxxxxx

# Set the default version of PHP CLI
# php_cli_version: 8.2
Copy link
Member

Choose a reason for hiding this comment

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

note that by itself this would lead to 8.2 being installed then VVV failing once it restored PHP default as the newly set version wouldn't be installed. It also wouldn't change the dashboard and other sites which would still be configured to 8.2 as the default upstream. There's a lot more work needed to support this, and previously we've added the php: ... parameter to sites to make it easier to wrangle since most requests to change the default PHP were to fix the provisioner and CLI using the wrong PHP to run stuff which php:.. fixes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is valid unintended effect that i didn't foresee. so i think its better to revert this change as it could break things rather than do good.

Copy link
Member

Choose a reason for hiding this comment

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

I get why people want it, but I want to make sure it's not an X Y solution as it'll get abused and has lots of gotchas, I'm curious about your use case though

Copy link
Member

Choose a reason for hiding this comment

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

I'm also mindful we will want a generic PHP provisioner at some point with templated PHP ini files etc rather than a dedicated hardcoded provisioner for each

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my use case maybe not that generic and/or critical enough, but here it is

  • im trying to match php version in WP CLI to the one that i have on my production server, that way when i do test my script locally ( in vvv ), it ensure script run successfully in that specific php version. Changing PHP version for WP CLI in runtime is not that straightforward, so having the PHP CLI to be statically same with my production server, would avoid me make a wrong judgement.

the solution that i offered here, maybe just too far and/or not actually the solution that i actually need. in my other fork, i have this script in homebin: vvv_change_php_default and that was enough.

#!/bin/bash

DEFAULTPHP=${1:-"7.4"}
echo " * Changing the default PHP CLI version ( ${DEFAULTPHP} )"
update-alternatives --set php "/usr/bin/php${DEFAULTPHP}"
update-alternatives --set phar "/usr/bin/phar${DEFAULTPHP}"
update-alternatives --set phar.phar "/usr/bin/phar.phar${DEFAULTPHP}"
update-alternatives --set phpize "/usr/bin/phpize${DEFAULTPHP}"
update-alternatives --set php-config "/usr/bin/php-config${DEFAULTPHP}"
echo " * Change completed"

i just assumed since its similar code with vvv_restore_php_default, then maybe having it in that script would be better, which is wrong assumption obviously.

Copy link
Member

Choose a reason for hiding this comment

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

hmmm, something I wanted to do was make it auto-switch PHP based on the folder you were in, but it's not as straightforward especially if you've multiple sessions. There's a ticket to add the PHP version into the prompt so it's more obvious for people who aren't aware of all of this and mistakenly run in the wrong version they expect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this also reverted, as you've mentioned that it can cause issues down the road. and my use case is too specific to be having this in default config

@@ -480,7 +480,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|

# Docker use image.
config.vm.provider :docker do |d, override|
d.image = 'pentatonicfunk/vagrant-ubuntu-base-images:20.04'
d.image = 'pentatonicfunk/vagrant-ubuntu-base-images:24.04'
Copy link
Member

Choose a reason for hiding this comment

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

does this trigger a VM rebuild for exsting Ubuntu 22 instances that upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question, i dont know. it would be vagrant territory, no ? the decision whether docker container need rebuild or not would be based on what vagrant flow will be issued. in any case better to test it, i'll try to test

Copy link
Member

Choose a reason for hiding this comment

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

main concerns is if vagrant would kick up a fuss about mismatching images or if it would discard the container ( and the database + data ) to create a new 24 container

Comment on lines 34 to 38
if sudo service "${mysql_service_name[@]}" status > /dev/null; then
sudo service "${mysql_service_name[@]}" restart
else
sudo service "${mysql_service_name[@]}" start
fi
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to wrap all this in a helper function as we have similar stuff in the vagrant up and provision hooks as well as lots of provisioners and it's super repetitive

sudo pip3 install wheel
sudo pip3 install shyaml

local OSVERSION_NUMBER=$(lsb_release lsb_release -sr)
Copy link
Member

Choose a reason for hiding this comment

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

shellcheck will probably flag this and a few other things, it should be something like this:

Suggested change
local OSVERSION_NUMBER=$(lsb_release lsb_release -sr)
local OSVERSION_NUMBER
OSVERSION_NUMBER=$(lsb_release lsb_release -sr)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adjusted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh, this adjustments causing error for some reason

lsb_release: error: No arguments are permitted

https://github.com/Varying-Vagrant-Vagrants/VVV/actions/runs/11580749924/job/32239965581?pr=2739

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted it to the original changes. the checks / linting seems to be fine: https://github.com/Varying-Vagrant-Vagrants/VVV/actions/runs/11581054132/job/32240928199?pr=2739
and we already doing these kind of local syntax previously: https://github.com/Varying-Vagrant-Vagrants/VVV/blob/develop/provision/core/vvv/provision.sh#L62-L64

Copy link
Member

Choose a reason for hiding this comment

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

ah I think the problem is a mistake on my part, lsb_release vs lsb_release lsb_release

if dpkg --compare-versions "${OSVERSION_NUMBER[@]}" ge "24.04"
then
# to make it available globally this is the last workaround, hopefully it doesn't break the system
# TODO: try to find a better alternative way to install
Copy link
Member

Choose a reason for hiding this comment

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

is there somewhere I can read up on why this happens or where the fix came from? Would be good to have it in the comments, I've created a separate PR for shyaml fixes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -731,3 +732,13 @@ function vvv_search_replace_in_file() {
fi
}
export -f vvv_search_replace_in_file

# @description Identify the service name for the MySQL service via /etc/init.d/ contents
Copy link
Member

Choose a reason for hiding this comment

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

in the past both have worked and been alias'd somehow

Copy link
Member

Choose a reason for hiding this comment

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

that wasn't something we did though, but an observation of how it came out the box

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly yea, it seems how mariadb installer works ( not vvv ). i am thinking maybe the mariadb vs mysql thing is a glitch that i experienced one time, as i can't replicate it again. my last provision, it gets correctly mariadb.
i will do 1 more tests, and maybe revert the changes mariadb vs mysql service thing from this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so i've reverted these, it seems the issue is something in my lab machine.

i've been testing the mariadb with multiple vagrant destroy and vagrant up --provision, its consistently working fine with mariadb

vagrant@vvv:~$ sudo service mariadb status
 * /usr/bin/mariadb-admin  Ver 10.0 Distrib 10.11.9-MariaDB, for debian-linux-gnu on x86_64
Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.

Server version          10.11.9-MariaDB-ubu2404
Protocol version        10
Connection              Localhost via UNIX socket
UNIX socket             /run/mysqld/mysqld.sock
Uptime:                 12 sec

Threads: 1  Questions: 63  Slow queries: 0  Opens: 33  Open tables: 26  Queries per second avg: 5.250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants