[RFC] First impressions of the current network configuration status #729
Conversation
teclator
commented
Feb 25, 2019
teclator
commented
- https://trello.com/c/MGUrYrXo/699-3-research-identify-the-worst-places-in-yast-network-for-refactoring
doc/status-current-code.md
Outdated
| we could start splitting at least the presentation part from the different | ||
| models, and move some responsabilities that currently are part of the | ||
| dialogs to other place. (See for example controllers / actions in | ||
| storage-ng) |
There was a problem hiding this comment.
There are many nice examples on how storage-ng separates responsibilities and layers: models / dialogs / clients in the Y2Storage part or actions / dialogs / widgets in the Y2Partitioner part to name a couple... But the not-so-obvious relationships between actions and controllers is definitely not the best example of well engineered separation. 😉
doc/status-current-code.md
Outdated
|
|
||
| The match between these different objects / hashes is based on the interface | ||
| or device name, and as we can see it has been one of me most common point of | ||
| failures in network configuration. |
There was a problem hiding this comment.
can you please add links to some well known bugs in network so we know which use cases we need to have in mind? Also one issue which is not mentioned here is problem that not all hwinfo need to have ifcfg file and so one, which sometimes causes problem.
There was a problem hiding this comment.
Sure, that is the main difference between virtual interfaces and phisical ones, and already described in Michal Zugec email link. So, will try to expand here in a way that the problems behind that are easy to expose and understand.
I will add also a section with the installation overview (not having a separation between the inst-sys network configuration and the proposal for the system being installed).
There was a problem hiding this comment.
moreover Items use a useless layer of abstraction (item_id). Designing a structure where devices would be more easily accessible according its names would have bigger benefit (now we search such device with O(n) complexity)
doc/status-current-code.md
Outdated
| - NetworkInterfaces (yast2): represent mainly the configured interfaces and is in | ||
| charge of writing the ifcfg-giles. The numbers of bugs in this area are not | ||
| so big, we have been dealing mostly with some normalization of the config or | ||
| better defaults like for wireless configuration. |
There was a problem hiding this comment.
Since fixing device renaming, NetworkInterfaces is calm place ... however it needs some love. Some methods should be revised if it should live in an upper layer. Some features should be redesigned (filtering devices according to name when filtering to device groups)
There was a problem hiding this comment.
On the other hand this module will become less important in the future when we should support exporting device configuration in wicked's native xml.
There was a problem hiding this comment.
So in the end we will need a support for sysconfig way of configuration (NetworkInterfaces), wicked's xml, ... so we can even think of at least partly supporting NetworkManager
doc/status-current-code.md
Outdated
|
|
||
| So basically each LanItems Item has the information about its (hwinfo, ifcfg | ||
| file, [udev rules](udev-rules-implementation.org), and table description | ||
| (the overview)) |
There was a problem hiding this comment.
and for example storing complete hwinfo for each interface is a bit overkill and causes a huge footprint. It is sad bcs in reality only two or three items are actively used.
|
Currently a big pain of networking code in my POV is that it mixes backend (controller) and UI (view) code together. It leads to methods with hundred lines with multiple exceptional handling like "if we need only backend functionality and no GUI, then don't show this popup" |
doc/status-current-code.md
Outdated
| reason of the new predictable network interfaces naming. Something we are | ||
| not prepared yet but which is already used in TW (we still offer to rename | ||
| the interfaces based on [udev rules](udev_rules.md) even when does not work | ||
| on this scenario) |
There was a problem hiding this comment.
well not prepared ... there is only one problematic part ... and that is transition (or creating) of configuration from instsys to installed target.
doc/status-current-code.md
Outdated
|
|
||
| 3. Duplication of code and bad methods naming, making it difficult to reuse or | ||
| understand if you do not know too deep about it. (It is also described in | ||
| the udev status document) [3] |
There was a problem hiding this comment.
naming is not that big problem for me, however some methods are definitely too complex and some still have undocumented side effects.
doc/status-current-code.md
Outdated
|
|
||
| 4. Some configuration workflow or interfaces should be revised or redesigned | ||
| as it is not clear enough how to set some of the most common options like | ||
| in wireless config. Not so far hostname setup has been already modified [4]. |
There was a problem hiding this comment.
I'd vote for revising methods like ReadHardware. This beasts are really huge and just works ... somehow, but no one knows why ... and if it is still needed to do that way.
doc/status-current-code.md
Outdated
| fragile and specially when remote installations are in use we do not want to | ||
| break current connections (for that maybe having a separation could help, but | ||
| this is only an idea that needs to be ellaborated / discussed than a real | ||
| problem). |
There was a problem hiding this comment.
a lot of configuration is stored in strange "global" variables which are wrongly maintained, for example it already happened that some variables was not cleared when configuration of device was canceled ... and configuration got (partly) written when whole module was closed by clicking [OK]
|
Outdated and not relevant anymore. |
