PR#4804: Improvements to Vagrant VM provisioning and operation - pagure

pagure

#4804 Improvements to Vagrant VM provisioning and operation

Opened 6 months ago by sudoman. Modified 5 months ago

sudoman/pagure vagrant-improvements into master

Fix service autostarting on Vagrant VM

Andrew Engelbrecht • 6 months ago

2b65ddc

Increase amount of memory alloted to Vagrant VM

Andrew Engelbrecht • 6 months ago

406c657

Fix SSH for Vagrant when personal keys installed

Andrew Engelbrecht • 6 months ago

b7cb024

dev/Vagrantfile.example

file modified

+2 -1

		`@@ -27,6 +27,7 @@`
		`# config.vm.synced_folder ".", "/vagrant", type: "nfs", nfs_version: 4, nfs_udp: false`
		`config.vm.synced_folder ".", "/vagrant", disabled: true`
		`config.vm.synced_folder ".", "/home/vagrant/devel",`
		`+ ssh_opts_append: "-o IdentitiesOnly=yes",`
		`type: "sshfs"`

		# To cache update packages (which is helpful if frequently doing `vagrant destroy && vagrant up`)
		`@@ -54,7 +55,7 @@`
		`# Season to taste`
		`domain.cpus = 4`
		`domain.graphics_type = "spice"`
		`- domain.memory = 2048`
		`+ domain.memory = 3072`
ngompa commented 6 months ago Can we raise this to 4096 instead? That gives us a bit more wiggle room...
		`domain.video_type = "qxl"`

		`# Uncomment the following line if you would like to enable libvirt's unsafe cache`

dev/ansible/roles/pagure-dev/files/pagure-docs.service

file modified

		`@@ -8,5 +8,8 @@`
		`/home/vagrant/devel/rundocserver.py --host 0.0.0.0`
		`Type=simple`

		`+ Restart=on-failure`
		`+ RestartSec=5s`
		`+`
		`[Install]`
		`WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure.service

file modified

		`@@ -7,5 +7,8 @@`
		`ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/python %h/devel/runserver.py --host 0.0.0.0 --debug`
		`Type=simple`

		`+ Restart=on-failure`
		`+ RestartSec=5s`
		`+`
		`[Install]`
		`WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure_ci.service

file modified

		`@@ -8,5 +8,8 @@`
		`ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/celery worker -A pagure.lib.tasks --loglevel=info -Q pagure_ci`
		`Type=simple`

		`+ Restart=on-failure`
		`+ RestartSec=5s`
		`+`
		`[Install]`
		`WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure_ev.service

file modified

		`@@ -9,5 +9,8 @@`
		`/home/vagrant/devel/pagure-ev/pagure_stream_server.py`
		`Type=simple`

		`+ Restart=on-failure`
		`+ RestartSec=5s`
		`+`
		`[Install]`
		`WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure_webhook.service

file modified

		`@@ -8,5 +8,8 @@`
		`ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/celery worker -A pagure.lib.tasks --loglevel=info -Q pagure_webhook`
		`Type=simple`

		`+ Restart=on-failure`
		`+ RestartSec=5s`
		`+`
		`[Install]`
		`WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure_worker.service

file modified

		`@@ -7,7 +7,9 @@`
		`ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/celery worker -A pagure.lib.tasks --loglevel=info`
		`Environment="PAGURE_CONFIG=/home/vagrant/pagure.cfg"`
		`Type=simple`
		`+`
		`Restart=on-failure`
		`+ RestartSec=5s`

		`[Install]`
		`WantedBy=default.target`

sudoman commented 6 months ago

These commits fix a few of the issues I ran into while trying to get the Vagrant based deployment up and running. There is more work to do, but once the Vagrant based installation is working, it should lower the bar to contributing to Pagure.

ngompa commented on line 14 of dev/Vagrantfile.example 6 months ago

Can we raise this to 4096 instead? That gives us a bit more wiggle room...

sudoman commented 6 months ago

That is an option, but I'm concerned about users who may not have much RAM on their computers. At 3 GB total RAM, that leaves an additional 1 GB for Pagure to make use of after it's already running.

ngompa commented 6 months ago

Hmm, that's fair.

ngompa commented 6 months ago

:thumbsup:

sudoman commented 6 months ago

Now that I think of it, I'm not sure if restarting services every 5 seconds upon failure is necessarily a great idea. If a developer patches Pagure, and a service consistently crashes right away, it will get restarted rather frequently.

This could produce a lot of debug output. I'm not sure if frequently restarting would also break how other services interact with the one that may only work for a few seconds at a time.

I think that in most cases this kind of behavior would be fine. However there could be some edge cases. Since the Vagrant based deployment is geared towards dev environments, that may be less of a concern, and more of an annoyance. In any case, without this change, or another solution, the Vagrant deployment isn't working yet.

pingou commented 6 months ago

Now that I think of it, I'm not sure if restarting services every 5 seconds upon failure is necessarily a great idea. If a developer patches Pagure, and a service consistently crashes right away, it will get restarted rather frequently.

I'm wondering a bit about this as well, I fear that 5seconds may not be enough for a developer to find which service did not behave as expected and the longer they take to figure which service crashed the longer they may end up needing to scroll up to find the traceback.

jlanda commented 6 months ago

@sudoman I read the other day on irc that you had problems to run systemctl --user using su...

that's because the vagrant user is not linger enabled. systemctl users session is launched after user's first login by default. If you enable lingering for that user it will start on boot.

a task on the provisioning ansible that runs loginctl linger-enable vagrantas root should fix that and allow you to use systemct --user from a su session.

Edited 6 months ago by jlanda

pingou commented 5 months ago

@sudoman with the recent changes we've made to the vagrant box, do we still need these? If so, could you rebase? It seems it conflicts now :)

pingou commented 5 months ago

@sudoman ping?

sudoman commented 5 months ago

Sorry about the dropped ping. The only part that may be useful still is the following:

https://pagure.io/fork/sudoman/pagure/c/b7cb024a8bec696287b1313c9e35eeaaa8602926

That should resolve an issue with running 'vagrant up' after adding one's SSH key to the Pagure user dashboard. I'll investigate and create a new PR.

pingou commented 5 months ago

@sudoman :thumbsup: for me, just drop the other commit from your branch and force push and it'll update this PR