propose new object API for devices

[jreidinger]

Hi,
I propose new API for network for purpose of new installer and as I hope good enough to replace in future old API, which is really unintuitive.

example usage as I see it for NI simple task ( for each device try to set up DHCP, start it and if it not work, then stop it and remove DHCP config )

def enable_dhcp_devices(device_name)
  device = Network::Device.new(device_name)
  device.configuration = Network::Configuration::DHCP.new
  device.start
  if device.global_address?
     return
  else
    device.stop
    device.configuration = Network::Configuration::None.new
    device.save
  end
end

in future we can improve when there is use case for it (I have in mind modules to extend configuration for specific devices with proper object hierarchy or Factory pattern for device, so it can be much easier cached).

@mchf @mvidner opinions?

[mvidner]

Looking only at the proposed usage so far: using save only in one if branch is surely wrong.

[jreidinger]

@mvidner you mean that before start must be explicit save and do not call it in start? Because for me start with old configuration when there is new one looks wrong. Another option is to remove save and put it in configuration=

[mchf]

Network is not only about devices configuration. It contains also routing, dns, hostname, ...

[jreidinger]

@mchf I know, I want to extend object modul to reflect real state when it is needed. This way it contain only used stuff and allow us to do it evolution way ( so splitting model when it is too big, define it more precise when needed, etc. )

[mchf]

Well, in current state I would prefer (take it as very fresh ideas):
(1) no drastic changes in yast2-network API. Refactor and clean the code as much as possible. As well as reducing public API. Reduce yast2-network's codebase. E.g. piece of code which detects available netcards should be moved into separate module.
(2) reimplement API for lower level modules (NetworkInterfaces). It will be needed bcs new low level modules will be required (wicked, nethw, ...). Make them powerful enought to be able to remove stupid "cache" (LanItems#Items)
(3) somehow persuade others to use yast2-network only for accessing net config. It is very often bypassed and e.g. NetworkInterfaces is often accessed directly. There are inconsistencies in cached data then. An example can be firewall module. It should add device into EXT zone by default. But it uses NetworkInterfaces for listing devices and it is by definition not able to list really all devices in the system (it lists only those which has netconfig files)

As a summary. I would redefine / reimplement network's architecture firstly and then try to identify new API.

[jreidinger]

@mchf - OK, sounds reasonable. So what steps you prefer for my task ( try dhcp for all network cards and keep it for all that have global address )? We can discuss it on IRC or on phone.

[mchf]

@jreidinger in current state you'll need:

• LanItems#Read It creates items for all netcards in the system (configured & unconfigured)
• create dhcp configuration for each device
• write the configuration (LanItems::Write should be enough)
• start all links (LanItems::SetAllLinksUp - it calls bash with ip link set <dev> up)

[jreidinger]

@mchf

LanItems#Read It creates items for all netcards in the system (configured & unconfigured)

OK, sounds reasonable

create dhcp configuration for each device

If I check https://github.com/yast/yast-network/blob/master/src/include/network/lan/cmdline.rb#L240 how CLI do it, then it is around 40 lines of code for such simple task

write the configuration (LanItems::Write should be enough)

It would be enough if it exist :) My checking code looks like I need to call NetworkInterfaces::Write instead

start all links (LanItems::SetAllLinksUp - it calls bash with ip link set <dev> up)

Does it use new configuration? @mvidner propose to use ifup

What is missing in your proposal is

• check what network interface doesn't have scope global
• for such network revert back configuration to unconfigured
• stop such devices

any proposal how achieve that? LanItems.Delete doesn't looks like good call, so maybe NetworkInterface.Delete is better?

[mvidner]

Maybe @mchf 's opposition comes from thinking that the initial example is a total rewrite of Yast::Network? It is not, it is a new top level namespace ::Network wrapping the existing cryptic APIs.

[jreidinger]

OK, lets discuss it in person when @mchf will be in office.

[mvidner]

Re save: @jreidinger explained to me that the original intent was to have it asymmetric: start calling save but stop not calling save because he thought that stopping an interface required the original configuration. AFAIK stopping an interface needs no config (recall "ifdown: no config found for eth0, will stop anyway"). Then the situation can be symmetric: save being called by both start and stop, or by configuration=, or explicitly (I am not sure which is best).

Other than this detail I really like this approach. One of the biggest mistakes in the current code is not having a consistent way of OOP so a clean new API will help a lot (even though underneath it may be messy).

[mvidner]

IIRC we decided in person to postpone this after SLE12 So what are the next steps 1) in the big picture 2) for this PR?

BTW this API was motivated by the need to implement https://github.com/yast/yast-network/blob/6d6f9da7c5324143ca5e3dc04a230f67af544a3f/src/clients/inst_setup_dhcp.rb

[jreidinger]

lets close it for now. We will reopen when there is time for it.