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

dbus: Allow Set without origin hint #294

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marex
Copy link

@marex marex commented Nov 5, 2022

Allow Set "s" signature, where the origin hint is left out entirely instead of requiring Set "ss" signature where the origin hint is an empty string.

The following become identical:
$ busctl call io.netplan.Netplan /io/netplan/Netplan/config/ULJIU0 io.netplan.Netplan.Config Set ss "ethernets.eth0={dhcp4: false, dhcp6: true}" ""
$ busctl call io.netplan.Netplan /io/netplan/Netplan/config/ULJIU0 io.netplan.Netplan.Config Set s "ethernets.eth0={dhcp4: false, dhcp6: true}"

Description

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

Allow Set "s" signature, where the origin hint is left out entirely
instead of requiring Set "ss" signature where the origin hint is an
empty string.

The following become identical:
$ busctl call io.netplan.Netplan /io/netplan/Netplan/config/ULJIU0 io.netplan.Netplan.Config Set ss "ethernets.eth0={dhcp4: false, dhcp6: true}" ""
$ busctl call io.netplan.Netplan /io/netplan/Netplan/config/ULJIU0 io.netplan.Netplan.Config Set s "ethernets.eth0={dhcp4: false, dhcp6: true}"
Copy link
Collaborator

@slyon slyon left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution to Netplan!

I like the overall approach of this PR, and I remember that I wanted to do something similar in the first place, when this feature was first implemented, but hit some roadblocks back then..

I wonder if this is the same issue we're seeing here in the failing tests (conflict in vtable)?

Could you please have a look at that and fix the tests (+coverage, if needed)?

Comment on lines +710 to +711
SD_BUS_METHOD("Set", "s", "b", method_config_set_simple, 0),
SD_BUS_METHOD("Set", "ss", "b", method_config_set_full, 0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

There seems to be a conflict in the vtable here, which makes the tests fail...
I'm not sure if we can have two methods using a different signature. Could you please investigate this?

$ NETPLAN_GENERATE_PATH=build/src/generate NETPLAN_DBUS_CMD=build/dbus/netplan-dbus LD_LIBRARY_PATH=build/src PYTHONPATH=. pytest-3 -v tests/dbus/
================================================================= test session starts =================================================================
platform linux -- Python 3.10.6, pytest-6.2.5, py-1.10.0, pluggy-0.13.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/lukas/canonical/netplan
collected 25 items                                                                                                                                    

tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_apply_in_snap_calls_busctl PASSED                                                        [  4%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_apply_in_snap_calls_busctl_err PASSED                                                    [  8%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_apply_in_snap_calls_busctl_ret130 PASSED                                                 [ 12%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_apply_in_snap_uses_dbus PASSED                                                           [ 16%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config FAILED                                                                       [ 20%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_apply FAILED                                                                 [ 24%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_cancel FAILED                                                                [ 28%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_get FAILED                                                                   [ 32%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set FAILED                                                                   [ 36%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set_invalidate FAILED                                                        [ 40%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set_uninvalidate FAILED                                                      [ 44%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set_uninvalidate_timeout FAILED                                              [ 48%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_apply FAILED                                                             [ 52%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_cancel FAILED                                                            [ 56%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_cb FAILED                                                                [ 60%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_config_try FAILED                                                        [ 64%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_no_ready_signal FAILED                                                   [ 68%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_generate PASSED                                                                     [ 72%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_happy PASSED                                                                        [ 76%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_info PASSED                                                                         [ 80%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_no_such_command PASSED                                                              [ 84%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_noroot PASSED                                                                       [ 88%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_generate_in_snap_calls_busctl PASSED                                                     [ 92%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_generate_in_snap_calls_busctl_err PASSED                                                 [ 96%]
tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_generate_in_snap_calls_busctl_ret130 PASSED                                              [100%]

====================================================================== FAILURES =======================================================================
[...]
=============================================================== short test summary info ===============================================================
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config - subprocess.CalledProcessError: Command '['busctl', 'call', '--system', '...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_apply - subprocess.CalledProcessError: Command '['busctl', 'call', '--syst...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_cancel - subprocess.CalledProcessError: Command '['busctl', 'call', '--sys...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_get - subprocess.CalledProcessError: Command '['busctl', 'call', '--system...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set - subprocess.CalledProcessError: Command '['busctl', 'call', '--system...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set_invalidate - subprocess.CalledProcessError: Command '['busctl', 'call'...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set_uninvalidate - subprocess.CalledProcessError: Command '['busctl', 'cal...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_set_uninvalidate_timeout - subprocess.CalledProcessError: Command '['busct...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_apply - subprocess.CalledProcessError: Command '['busctl', 'call', '--...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_cancel - subprocess.CalledProcessError: Command '['busctl', 'call', '-...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_cb - subprocess.CalledProcessError: Command '['busctl', 'call', '--sys...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_config_try - subprocess.CalledProcessError: Command '['busctl', 'call'...
FAILED tests/dbus/test_dbus.py::TestNetplanDBus::test_netplan_dbus_config_try_no_ready_signal - subprocess.CalledProcessError: Command '['busctl', '...
===================================================== 13 failed, 12 passed, 7 warnings in 28.93s ======================================================

@slyon slyon added the needswork Change looks good but requires a bit more work label Nov 23, 2022
@slyon slyon added the community This PR has been proposed by somebody outside of the Netplan team and roadmap commitments. label Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community This PR has been proposed by somebody outside of the Netplan team and roadmap commitments. needswork Change looks good but requires a bit more work
Projects
None yet
2 participants