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

Fix cc_keyboard in mantic #4361

Merged
merged 2 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cloudinit/distros/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ def manage_service(
cmd = list(init_cmd) + list(cmds[action])
return subp.subp(cmd, capture=True, rcs=rcs)

def set_keymap(self, layout, model, variant, options):
def set_keymap(self, layout: str, model: str, variant: str, options: str):
if self.uses_systemd():
subp.subp(
[
Expand Down
2 changes: 1 addition & 1 deletion cloudinit/distros/alpine.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def _read_hostname(self, filename, default=None):
def _get_localhost_ip(self):
return "127.0.1.1"

def set_keymap(self, layout, model, variant, options):
def set_keymap(self, layout: str, model: str, variant: str, options: str):
if not layout:
msg = "Keyboard layout not specified."
LOG.error(msg)
Expand Down
35 changes: 31 additions & 4 deletions cloudinit/distros/debian.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,38 @@ def update_package_sources(self):
def get_primary_arch(self):
return util.get_dpkg_architecture()

def set_keymap(self, layout, model, variant, options):
# Let localectl take care of updating /etc/default/keyboard
distros.Distro.set_keymap(self, layout, model, variant, options)
# Workaround for localectl not applying new settings instantly
def set_keymap(self, layout: str, model: str, variant: str, options: str):
# localectl is broken on some versions of Debian. See
# https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/2030788 and
# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1038762
#
# Instead, write the file directly. According to the keyboard(5) man
# page, this file is shared between both X and the console.

contents = "\n".join(
[
"# This file was generated by cloud-init",
"",
f'XKBMODEL="{model}"',
f'XKBLAYOUT="{layout}"',
f'XKBVARIANT="{variant}"',
f'XKBOPTIONS="{options}"',
"",
'BACKSPACE="guess"', # This is provided on default installs
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a nitpick... Is it possible to allow the user to configure this, but still default to "guess" incase they do not configure this option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

While a good thought, the implementation is highly debian-specific given that this implementation is writing directly to /etc/default/keyboard. Surfacing such and option is something that won't be supportable easily on any other distribution that is calling localectl directly, because that command doesn't support setting backspace values.

"",
]
)
util.write_file(
filename="/etc/default/keyboard",
content=contents,
mode=644,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mode here needs to be octal not decimal, this resulted in some very bad behavior on mantic when cloud-init wrote the file with perms 1204 :)

root@testlocale-manticdaily:~# localectl status
Could not get properties: Access Denied

root@testlocale-manticdaily:~# ls -l /etc/default/keyboard 
-rw-r----T 1 root root 120 Aug 25 02:31 /etc/default/keyboard

More details in the bug

omode="w",
)

# Due to
# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=926037
# if localectl can be used in the future, this line may still
# be needed
self.manage_service("restart", "console-setup")


Expand Down
50 changes: 30 additions & 20 deletions tests/unittests/config/test_cc_keyboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,33 +93,43 @@ def setUp(self):
@mock.patch("cloudinit.distros.Distro.uses_systemd")
@mock.patch("cloudinit.distros.subp.subp")
def test_systemd_linux_cmd(self, m_subp, m_uses_systemd, *args):
"""Ubuntu runs localectl"""
"""Non-Debian systems run localectl"""
cfg = {"keyboard": {"layout": "us", "variant": "us"}}
layout = "us"
model = "pc105"
variant = "us"
options = ""
m_uses_systemd.return_value = True
cloud = get_cloud("ubuntu")
cloud = get_cloud("fedora")
cc_keyboard.handle("cc_keyboard", cfg, cloud, [])
locale_calls = [
mock.call(
[
"localectl",
"set-x11-keymap",
layout,
model,
variant,
options,
]
),
mock.call(
["systemctl", "restart", "console-setup"],
capture=True,
rcs=None,
),
]
m_subp.assert_has_calls(locale_calls)
locale_call = mock.call(
[
"localectl",
"set-x11-keymap",
layout,
model,
variant,
options,
]
)
assert m_subp.call_args == locale_call

@mock.patch("cloudinit.util.write_file")
@mock.patch("cloudinit.distros.subp.subp")
def test_debian_linux_cmd(self, m_subp, m_write_file):
"""localectl is broken on Debian-based systems so write conf file"""
cfg = {"keyboard": {"layout": "gb", "variant": "dvorak"}}
cloud = get_cloud("debian")
cc_keyboard.handle("cc_keyboard", cfg, cloud, [])

m_content = m_write_file.call_args[1]["content"]
assert 'XKBMODEL="pc105"' in m_content
assert 'XKBLAYOUT="gb"' in m_content
assert 'XKBVARIANT="dvorak"' in m_content
assert "/etc/default/keyboard" == m_write_file.call_args[1]["filename"]
m_subp.assert_called_with(
["service", "console-setup", "restart"], capture=True, rcs=None
)

@mock.patch("cloudinit.distros.subp.subp")
def test_alpine_linux_cmd(self, m_subp, *args):
Expand Down
Loading