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

ENT-5099: Collect network facts using 'ip' #3284

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Conversation

m-horky
Copy link
Contributor

@m-horky m-horky commented Jun 13, 2023

  • Card ID: ENT-5099

Implementation of new code for network collection using iproute instead of python-ethtool

@cnsnyder cnsnyder requested review from a team and ptoscano and removed request for a team June 13, 2023 07:21
@github-actions
Copy link

github-actions bot commented Jun 13, 2023

Coverage

Coverage (computed on Fedora latest) •
FileStmtsMissCoverMissing
rhsmlib/facts
   all.py190100% 
   hwprobe.py41510175%43, 85, 194–196, 220–221, 237–238, 260, 265, 270–272, 274–277, 279, 324–328, 330–332, 335–338, 340–342, 369, 386, 405, 407, 439, 465, 583, 601–603, 612, 620–623, 633, 636, 638–639, 641, 648, 652–654, 660–662, 666–668, 679–680, 684, 708, 710–711, 713, 715, 717, 719–722, 726–728, 730–731, 734, 744–748, 750–751, 753–754, 756–762, 764–765
   network.py89891%41–46, 72–73
TOTAL18059438475% 

Tests Skipped Failures Errors Time
2632 9 💤 0 ❌ 0 🔥 58.413s ⏱️

@m-horky m-horky force-pushed the mhorky/ENT-5099_ip branch from 4efcb6d to e777c9f Compare June 13, 2023 07:32
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for this, in general it seems solid work; I'll do a thorough review in the upcoming days.

In the meanwhile, I have few small notes, some inline and some global:

  • I'd suggest using
    """
    text
    """
    for the multi-line doc comments, ie the first line in a different line of """
  • in the _list facts, the item must be separated by ", " and not only by a space; see the existing code:
    list_key = key + "_list"
    netinfdict[key] = values[0]
    netinfdict[list_key] = ", ".join(values)

src/rhsmlib/facts/network.py Outdated Show resolved Hide resolved
src/rhsmlib/facts/network.py Outdated Show resolved Hide resolved
src/rhsmlib/facts/network.py Outdated Show resolved Hide resolved
src/rhsmlib/facts/network.py Outdated Show resolved Hide resolved
@m-horky m-horky force-pushed the mhorky/ENT-5099_ip branch 3 times, most recently from 655d2df to 4086f1d Compare June 19, 2023 14:31
@m-horky m-horky force-pushed the mhorky/ENT-5099_ip branch from 4086f1d to 04d6849 Compare June 28, 2023 10:00
* BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2215974
* Card ID: ENT-5099

Implementation of new code for network collection using 'iproute'
instead of 'python-ethtool'.

- Bonding can be created quite easily in VM (provided it has two virtual
  interfaces):

  nmcli connection add type bond ifname bond0 con-name bond0
  nmcli connection add type ethernet ifname enp1s0 master bond0 \
    con-name bond0-1
  nmcli connection add type ethernet ifname enp2s0 master bond0 \
    con-name bond0-2
  nmcli connection up bond0
  nmcli connection up bond0-1
  nmcli connection up bond0-2

- Tunnels can be enabled by running

  ifconfig sit0 up

- Loopback usually is called 'lo', but it is just a name and we should
  not rely on it. It can be renamed by calling

  ip link set down lo
  ip link set lo name lpbck
  ip link set up lpbck

- Network interfaces may not only be ASCII. They can even be emojis such
  as watermelon:

  ip link set eth0 name 🍉

  or even multi-byte complex emojis using joiners (which you can create
  in Python by printing "\N{ADULT}\N{ZERO WIDTH JOINER}\N{ROCKET}"):

  ip link set eth0 name 🧑‍🚀
@m-horky m-horky force-pushed the mhorky/ENT-5099_ip branch from 04d6849 to 4c81d7c Compare June 28, 2023 11:17
@ptoscano ptoscano marked this pull request as ready for review June 29, 2023 15:46
@cnsnyder cnsnyder requested a review from a team June 29, 2023 15:47
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ptoscano ptoscano merged commit fc8979c into main Jun 29, 2023
@ptoscano ptoscano deleted the mhorky/ENT-5099_ip branch June 29, 2023 15:53
@m-horky
Copy link
Contributor Author

m-horky commented Jul 12, 2023

Hi @yselkowitz, thanks for catching that! Should be fixed by #3295.

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