Add support to set static IPs, gateway, DNS to wired/wireless networks #5
Conversation
Create piaware-config.txt for debian-bookworm
setting static IP setting gateway setting netmask setting nameservers
…, dns Added unit test for generate-network-config-bookworm.service
nikeflight
changed the title
replace default values for wireless-type and wired-type to "network-manager"
…er NetworkManager is started
Add start-network.service, start_network.py to up the connections aft…
… the connection profile before NM spins up Change network-manager back to dhcp in piaware_config.py
…st will ignore any setting value for a particular key from that file.
debian-bookworm/piaware-support.generate-network-config.service
Outdated
Show resolved
Hide resolved
| Wants=config-ready.target | ||
| DefaultDependencies=no | ||
|
|
||
| [Service] | ||
| Type=oneshot | ||
| ExecStart=/usr/bin/python3 /usr/lib/piaware-support/generate-network-config-bookworm.py | ||
| ExecStart=/usr/bin/python3 /usr/lib/piaware-support/generate_network_config_bookworm.py |
There was a problem hiding this comment.
I think we want a shebang interpreter here, not explicitly refer to python3 in the service file
There was a problem hiding this comment.
As in:
Add #!/usr/bin/evn python3 to generate_network_config_bookworm.py.
Make it executable (chmod +x). I think we can do this in the rules folder.
Call it in this file like ExecStart=/usr/lib/piaware-support/generate_network_config_bookworm.py
Or is there a simpler way to do this.
There was a problem hiding this comment.
Yes I think the goal is to be able to call it in the .service file without python3 and the .py extension...i.e. ExecStart=/usr/lib/piaware-support/generate_network_config_bookworm
So you could just rename the file without the .py extension, add the shebang, and chmod +x it.
| ipv4 += configure_static_network("wired", config) | ||
| ipv4 += "method=manual\n" | ||
| except ValueError: | ||
| ipv4 += "method=auto\n" |
There was a problem hiding this comment.
This hides a multitude of errors in a non-obvious way. Bad config should really result in user-visible errors.
| if not config.get("wireless-network"): | ||
| print("wireless-network set to no") | ||
| return | ||
|
|
There was a problem hiding this comment.
If you change from wireless-network yes to wireless-network no, then without a cleanup step, this will leave behind the old wireless.nmconnection with the old config.
I mentioned this in slack earlier and I thought you said the files were written unconditionally? If not then you need cleanup.
There was a problem hiding this comment.
Yes I got my wires crossed.
We can write the files unconditionally. The autoconnect field will determine if the wireless/wired network will be enabled/disabled.
scripts/piaware_config.py
Outdated
| self.settings["wired-address"] = MetadataSettings(sdonly=True, network=True, setting_type="str") | ||
| self.settings["wired-netmask"] = MetadataSettings(sdonly=True, network=True, setting_type="str") | ||
| # Setting broadcast address directly through boot/firmare/piaware-config.txt has been deprecated. | ||
| self.settings["wired-broadcast"] = MetadataSettings(sdonly=True, network=True, setting_type="str", ignore_list=[BOOT_PIAWARE_CONF]) |
There was a problem hiding this comment.
Why is this setting ignored, but only from specific files?
There was a problem hiding this comment.
I added this as a way to deprecate or ignore settings.
I guess it makes sense to just have the logic be ignore any settings for this value from all files.
scripts/piaware_config.py
Outdated
| self.settings["wireless-gateway"] = MetadataSettings(sdonly=True, network=True) | ||
| self.settings["wireless-nameservers"] = MetadataSettings(default = ["8.8.8.8", "8.8.4.4"], sdonly=True, network=True) | ||
| self.settings["wireless-address"] = MetadataSettings(sdonly=True, network=True, setting_type="str") | ||
| self.settings["wireless-broadcast"] = MetadataSettings(sdonly=True, network=True, setting_type="str", ignore_list=[BOOT_PIAWARE_CONF]) |
| @@ -210,6 +214,13 @@ def validate_uuid(self, val: str, version=4) -> bool: | |||
| def validate_gain(self, val: str) -> bool: | |||
| return self.validate_double(val) or (isinstance(val, str) and val == "max") | |||
|
|
|||
| def check_if_file_in_ignore_list(self, key, file_name) -> bool: | |||
There was a problem hiding this comment.
If you defaulted ignore_list to [] then this function becomes just return (file_name in self.settings[key].ignore_list) which is brief enough you may not even need a function for it.
Moved MetadataSettings to be a dict literal Change subprocess calls to use os module We write unconditionally to connection files. We set autoconnect variable in order to enable/disable the network Remove debian-bookworm/piaware-config.txt
Fix unit tests Fix dict initialization
Keep the wired-network logic simple.
If eth is plugged in, enable ethernet.
If not, disable ethernet.
a) Remove the wired-network option. No point in having it since it's based on whether an ethernet cable is physically plugged in.
b) Remove mention of dhc.