Merge ~hyask/autopkgtest-cloud:skia/vimrc into autopkgtest-cloud:master

[Skia]

Set a default vimrc on autopkgtest-cloud machines.

[Skia (hyask)]

To have a quick look at what this enables, please `vim /tmp/test` on the bastion (production environment).

[Paride Legovini (paride)]

review: Disapprove

[Paride Legovini (paride)]

I misclicked, I don't have an opinion on this one yet.

review: Abstain

[Tim Andersson (andersson123)]

given the vimrc config is duplicated in code in both of the charms with this approach, I think it'd be best to just have it in a file that's shared by both charms rather than have the config duplicated. Plus you can also then just symlink the vimrc (within autopkgtest-cloud) to /etc/vim/vimrc.local

review: Needs Fixing

[Paride Legovini (paride)]

I think this is fine and the settings are sensible, at least until we get a team member that uses terminals with a light background. :)

Very minor nit: there is some alignment issue in the inline comments (on the match ExtraWhitespace line), but maybe that's only happening on Launchpad.

As discussed in a side channel, is it fine to have this system wide as this is going into single purpose systems, and we often need to `sudo vim`, and in that case having the vimrc to the $HOME of the unprivileged user won't work.

+1 but needs CI fix, I see you added the disable=line-too-long, but it's still failing?

[Tim Andersson (andersson123)]

... I use light background, but I'm fine, as I'm clearly the minority here ...

[Skia (hyask)]

I've updated to fix the CI.

Here are my answers:
Tim: I totally agree that there is code duplication, and I don't like that. I couldn't find what mechanism we have in place to avoid that in the charms code. The current status seems to be to duplicate the code, since there are a lot of similar functions in both charms already. Do you know of anything that could help?

Paride: about the misalignment, this is on purpose in the Python code, since we need to backslash escaping. The goal was to have proper alignment in the resulting `vimrc` file.

[Tim Andersson (andersson123)]

The only mechanism I've used in the past is in this MP:
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/457239

You can see config_file_check_common.py is symlinked to the relevant charm directories. The MP is unfinished and I believe the way I symlinked the file obviously isn't appropriate, so I need to fix that when I come back to it.

But yeah, the only suggestion I have is the primitive ape approach I've detailed above, symlink a common file to the relevant directories

[Skia (hyask)]

You mean the local symlink you pushed to the MP, with the whole `/home/tim/canonical/code/autopkgtest-cloud/infrastructure-tests-common/config_file_check_common.py` path? I doubt this is appropriate to push to a repo shared among us :-D

Jokes aside, yes, maybe a relative symlink could be pushed to reduce the copy-pasting, but I think we would spend better time really thinking about a proper solution to improve our charms.

[Tim Andersson (andersson123)]

yes I mean that xD

I agree. We should spend some time coming up with a better solution. If you think it's best to do that at another time and not do anything for this MP, I'm fine with that. I'd much rather have a symlink at the least but I'm not to fussed, do as you please

[Skia (hyask)]

Given your comments, our discussions, and the fact that this is not really dangerous for autopkgtest-cloud itself, I've gone ahead and merged this. :-)

review: Approve

diff --git a/charms/focal/autopkgtest-cloud-worker/layer.yaml b/charms/focal/autopkgtest-cloud-worker/layer.yaml
index 692d9af..70be935 100644
--- a/charms/focal/autopkgtest-cloud-worker/layer.yaml
+++ b/charms/focal/autopkgtest-cloud-worker/layer.yaml
@@ -29,4 +29,5 @@ options:
       - python3-openstackclient
       - python3-swiftclient
       - ssmtp
+      - vim
     include_system_packages: true
diff --git a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
index 4b5b22d..eaf76e4 100644
--- a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
+++ b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
@@ -4,6 +4,7 @@ import json
 import os
 import socket
 import subprocess
+from pathlib import Path
 from textwrap import dedent
 
 import pygit2
@@ -51,6 +52,23 @@ AUTOPKGTEST_PER_PACKAGE_CLONE_LOCATION = (
 RABBITMQ_CRED_PATH = os.path.expanduser("~ubuntu/rabbitmq.cred")
 
 
+@when("apt.installed.vim")
+def configure_vim():
+    status.maintenance("Configuring vim")
+    Path("/etc/vim/vimrc.local").write_text(
+        """" autopkgtest-cloud vimrc, nicer for admins
+" This file is written by the autopkgtest-cloud charm, do not edit manually, or
+" you may loose your changes.
+set background=dark                                             " lighter colors by default
+set number                                                      " show line numbers
+set listchars=eol:¬,tab:>·,trail:~,extends:>,precedes:<,nbsp:‡  " set display of special characters
+set list                                                        " show those special characters
+highlight ExtraWhitespace ctermbg=red guibg=red                 " highlight extra whitespaces
+match ExtraWhitespace /\\s\\+$\\| \\+\\ze\\t/                         " define the extra whitespaces
+    """
+    )
+
+
 @when_not("autopkgtest.autopkgtest_cloud_symlinked")
 def symlink_autopkgtest_cloud():
     status.maintenance("Creating symlink to charmed autopkgtest-cloud code")
diff --git a/charms/focal/autopkgtest-web/layer.yaml b/charms/focal/autopkgtest-web/layer.yaml
index 5c81bdb..6fd70d5 100644
--- a/charms/focal/autopkgtest-web/layer.yaml
+++ b/charms/focal/autopkgtest-web/layer.yaml
@@ -21,4 +21,5 @@ options:
       - python3-flask-openid
       - python3-swiftclient
       - python3-werkzeug
+      - vim
     include_system_packages: true
diff --git a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
index a228a0f..3aad037 100644
--- a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
+++ b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
@@ -42,6 +42,23 @@ def install_apt_packages():
     clear_flag("apt.queued_installs")
 
 
+@when("apt.installed.vim")
+def configure_vim():
+    status.maintenance("Configuring vim")
+    pathlib.Path("/etc/vim/vimrc.local").write_text(
+        """" autopkgtest-cloud vimrc, nicer for admins
+" This file is written by the autopkgtest-cloud charm, do not edit manually, or
+" you may loose your changes.
+set background=dark                                             " lighter colors by default
+set number                                                      " show line numbers
+set listchars=eol:¬,tab:>·,trail:~,extends:>,precedes:<,nbsp:‡  " set display of special characters
+set list                                                        " show those special characters
+highlight ExtraWhitespace ctermbg=red guibg=red                 " highlight extra whitespaces
+match ExtraWhitespace /\\s\\+$\\| \\+\\ze\\t/                         " define the extra whitespaces
+    """
+    )
+
+
 @when_not("autopkgtest-web.autopkgtest_web_symlinked")
 def symlink_autopkgtest_cloud():
     status.maintenance("Creating symlink to charmed autopkgtest-web code")