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

LACP support #37

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

LACP support #37

wants to merge 23 commits into from

Conversation

ydirson
Copy link
Contributor

@ydirson ydirson commented Jan 23, 2023

This branch deals with the installer side of things, firstboot data needs to be handled in XAPI, for which we depend on xapi-project/xen-api#4333:

  • configuration from cmdline (covers fetching answerfile and the next items)
  • configuration from answerfile
  • configuration UI for netinstall
  • configuration UI of mgmt interface, writing data for firstboot

Known limitations we may want to address:

  • bad cmdline config could be reported cleanly (we currently get a python backtrace, which gets garbled by the TUI)
  • does not deal with DebStyleInterface, ie. does not support Debian-based installer; shouldn't that code be simply be nuked ? [nuked since]
  • limitations that could be addressed by moving bond creation/management to a preliminar "advanced networking" step similar to "advanced storage" (adding a new F key for this to initial screen may look bad, though):
    • bond interface is not created immediately, so appears in the interface list as "[no link]". Maybe this can be dealt with, by first creating ifcfg-* files without an IP config.
    • cannot change our mind once the choice to create a bond with given eth* has been made

This PR explicitly does not deal with:

  • creating more than 1 bond
  • system upgrade parsing during upgrade (get mgmt-iface config in ExistingInstallation.readSettings()), to which network_db seems to be unable to report about bonding (but the impact seems to be limited to default values). The system's bonds are already preserved on upgrade, though.

What the screens currently look like:

lacp-0-ifaces
lacp-1-create
lacp-2-confirm
lacp-3-ifaces

ydirson and others added 23 commits March 6, 2023 13:36
This will allow for more concise logic.

Signed-off-by: BenjiReis <[email protected]>
Always safer, and will facilitate further splitting

Signed-off-by: Yann Dirson <[email protected]>
This check was already not very useful, as if an eth device is not in
getNetifList(), the ifup command will just fail properly.

But in the case of a bond interface, the bond device will not even exist
before running ifup.

Signed-off-by: Yann Dirson <[email protected]>
This is a prerequisite to allowing more than just eth*

Signed-off-by: Yann Dirson <[email protected]>
Less complexity for the nominal case, and makes it easier to add different
non-nominal cases.

Signed-off-by: Yann Dirson <[email protected]>
…rpart

Don't repeat all parameters on the deprecated version, as we're going to
add more options.

Signed-off-by: Yann Dirson <[email protected]>
Using 1 name for 2 concepts is a bad idea, especially in a single function.

Also nukes one line of dead code.

Signed-off-by: Yann Dirson <[email protected]>
Signed-off-by: Yann Dirson <[email protected]>
(cherry picked from commit 50ecc56)
For bonding support we'll add more fields to NIC, that NetInterface needs
to know about.

Signed-off-by: Yann Dirson <[email protected]>
Note this does not thouch DebStyleInterface, which seems to be dead
code we cannot test.

Signed-off-by: Yann Dirson <[email protected]>
Bonding configuration created from commandline cannot be properly
passed to the UI, which needs it both for netinstall and
host-management, so we have to re-discover the existing configuration
to present it to the user.  This includes identifying the configured
bonding interface, and filtering out their member interfaces from
available choices.

This is not very satisfying, and we may want to improve the overall
installer architecture to make this better in the future, but it would be
way out of scope here.

Signed-off-by: Yann Dirson <[email protected]>
We'll call them from upcoming lacp_bond_ui().

Signed-off-by: BenjiReis <[email protected]>
Making things visible is good (esp. here it allows to check interface
details), especially once we add the ability to create a bond
interface.

Signed-off-by: Yann Dirson <[email protected]>
Works at selectNetif level, and as such applies both to installer and
host-management networt configs.

Signed-off-by: BenjiReis <[email protected]>
Signed-off-by: Yann Dirson <[email protected]>
@stormi
Copy link
Contributor

stormi commented Mar 6, 2023

What's the context of the last force-push?

@ydirson
Copy link
Contributor Author

ydirson commented Mar 6, 2023

What's the context of the last force-push?

A simple rebase on latest release

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