Skip to content

Add support to assign an interface or match settings to a connection#723

Merged
teclator merged 12 commits intomasterfrom
matching_settings
Sep 13, 2023
Merged

Add support to assign an interface or match settings to a connection#723
teclator merged 12 commits intomasterfrom
matching_settings

Conversation

@teclator
Copy link
Copy Markdown
Contributor

@teclator teclator commented Aug 29, 2023

Problem

Agama allows to add/update network connections. However, it relies on NetworkManager to decide which interface to use. Alternatively, Agama should allow the user to assign a connection to an specific interface.

Instead of using just an interface name, Agama should offer some matching capabilities, just like NetworkManager. How the API should look like is up to who implements the feature but the interface-name, the path and the driver should be supported.

Solution

It has been added support to specify to which interfaces the connection should be bond to supporting it directly through the interface name attribute or through a set of match settings as them are supported by NetworkManager.

"network": {
    "connections": [
      {
        "id": "Ethernet network device 1",
        "method": "manual",
        "interface": "eth0",
        "addresses": [
          "192.168.122.100/24"
        ],
        "gateway": "192.168.122.1",
        "nameservers": [
          "192.168.122.1"
        ]
      }
    ]
  }

Testing

  • Added a new unit test
  • Tested manually

@coveralls
Copy link
Copy Markdown

coveralls commented Sep 11, 2023

Coverage Status

coverage: 72.814% (-0.05%) from 72.863% when pulling c3f1bd9 on matching_settings into 9c310af on master.

@teclator teclator requested a review from imobachgs September 12, 2023 09:02
Copy link
Copy Markdown
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

In general, it looks good. Just a few suggestions.


match_config_dbus.insert("interface-name", interfaces);

match_config_dbus
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can do:

    HashMap::from([
        ("driver", drivers),
        ("kernel-command-line", kernels),
        ("path", paths),
        ("interface-name", interfaces),
    ])

It looks more readable to me and it is easy to find out what is included in the HashMap.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, originally did so, then added some logic to inserting only the key if it was not empty but as we are inserting it always now it makes sense to do so. Thanks

@teclator teclator force-pushed the matching_settings branch 2 times, most recently from 4b60739 to ed83dad Compare September 13, 2023 11:46
Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>
@teclator teclator merged commit f7fa3ad into master Sep 13, 2023
@teclator teclator deleted the matching_settings branch September 13, 2023 12:10
@imobachgs imobachgs mentioned this pull request Sep 26, 2023
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.

3 participants