vyos_net_name: T6544: Updated the vyos_net_name script (backport from current)
#3806
Conversation
zdc
requested review from
dmbaturin,
sarthurdev,
jestabro,
sever-sever,
c-po and
nicolas-fort
|
👍 |
|
❌ |
|
@Mergifyio backport circinus |
🟠 Waiting for conditions to match
|
| if re.match("^e[0-9]+$", ifname): | ||
| intf = ifname.split("e") | ||
| """Check interface with names eX and return ifname on the next format eth{ifindex} - 2""" | ||
| if re.match('^e[0-9]+$', ifname): |
There was a problem hiding this comment.
It's not a big performance problem for short strings, but I'd still do a match first, then check if the match object was None and used the match groups if it wasn't, instead of calling .split() on it, since re.match() result can already give us the substring.
There was a problem hiding this comment.
This is old code, just reformatted according to new rules. I believe code optimization is a separate task.
| @@ -95,11 +89,12 @@ def get_biosdevname(ifname: str) -> str: | |||
| try: | |||
| biosname = cmd(f'/sbin/biosdevname --policy all_ethN -i {ifname}') | |||
There was a problem hiding this comment.
Should we add the missing --allow-vm argument here, if we are using biosdevname for now?
There was a problem hiding this comment.
IIRC this is the only reason we keep a fork of biosdevname? If so, we should add it.
|
It should first be in the current -> circinus > sagitta |
|
FIY: the PR to deprecate biosdevname is in: #3821 |
Sometimes we need a reliable way to lock an execution until some other operation
is not done.
This commit introduces locking util, which can be used as a common lock, even
between different processes.
Usage example:
```
from vyos.utils.locking import Lock
lock = Lock('my_lock_id')
lock.acquire(timeout=10)
print(f'Lock acquired: {lock.is_locked}')
lock.release()
```
Improvements in the `vyos_net_name`: - Used a new locking system, to be sure that multiple running scripts will not try to perform operations at the same time. - Replace logging from a file to syslog. This is common with all the rest logs, and additionally gives a better view of actions done during a boot. - Small bug fix in `get_configfile_interfaces()`: exit with an error in case a config file cannot be parsed. This resolves potentially an unbound `config` object. - Minor formatting fixes to follow our requirements.
|
@Mergifyio dequeue |
☑️ The pull request is not queued |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
@zdc could you rebase or solve conflicts? |
|
Backported to the circinus our org |
Change Summary
Updated the
vyos_net_namescriptTypes of changes
Related Task(s)
Related PR(s)
Component(s) name
udev, vyos_net_name
Proposed changes
Improvements in the
vyos_net_name:get_configfile_interfaces(): exit with an error in case a config file cannot be parsed. This resolves potentially an unboundconfigobject.How to test
Boot with a config like this on a device where
eth0andeth1interfaces defined in it are connected via PCIe, and there are at least two extra onboard NICs :The easiest way to see a difference is to check logs - without a new locking system they are out of order:
With an updated file, everything is in order:
Smoketest result
N/A
Checklist: