Bug 1367378 - vdsm can not handle all ifcfg files created by NM
Summary: vdsm can not handle all ifcfg files created by NM
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: vdsm
Classification: oVirt
Component: Core
Version: 4.18.10
Hardware: All
OS: Linux
high
medium
Target Milestone: ovirt-4.0.5
: 4.18.15.1
Assignee: Edward Haas
QA Contact: Michael Burman
URL:
Whiteboard:
: 1362353 (view as bug list)
Depends On: 1368764
Blocks: 1304509
TreeView+ depends on / blocked
 
Reported: 2016-08-16 10:01 UTC by jianwu
Modified: 2017-01-18 07:39 UTC (History)
20 users (show)

Fixed In Version: v4.18.15.1
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-01-18 07:39:23 UTC
oVirt Team: Network
Embargoed:
rule-engine: ovirt-4.0.z+
ylavi: planning_ack+
fdeutsch: devel_ack+
cshao: testing_ack+


Attachments (Terms of Use)
The bond0 setup info on cockpit Web UI (212.41 KB, image/png)
2016-08-16 10:01 UTC, jianwu
no flags Details
The bond0's ifcfg file name display in /etc/sysconfig/network-scripts (135.55 KB, image/png)
2016-08-16 10:02 UTC, jianwu
no flags Details
The related log info packages about bond0 setup (6.74 MB, application/x-gzip)
2016-08-16 10:05 UTC, jianwu
no flags Details
bond_creation.png (147.21 KB, image/png)
2016-08-17 02:24 UTC, Ryan Barry
no flags Details
bond_deletion.png (71.23 KB, image/png)
2016-08-17 02:24 UTC, Ryan Barry
no flags Details


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 63990 0 master MERGED net: Consume ifcfg files that have a non vdsm standard name 2016-09-23 10:06:43 UTC
oVirt gerrit 64095 0 ovirt-4.0 NEW net: Expose owned_device method through the configurator 2016-09-25 11:51:15 UTC
oVirt gerrit 64096 0 ovirt-4.0 MERGED net: Consume ifcfg files that have a non vdsm standard name 2016-09-29 10:38:08 UTC
oVirt gerrit 65304 0 ovirt-4.0.5 MERGED net: Consume ifcfg files that have a non vdsm standard name 2016-10-11 11:53:46 UTC

Description jianwu 2016-08-16 10:01:24 UTC
Created attachment 1191183 [details]
The bond0 setup info on cockpit Web UI

Description of problem:
Setup bond0 over two NICs with active-backup mode, the bond0' IP display unsuitable ifcfg file name when check it under /etc/sysconfig/network-scripts.

It should show as : ifcfg-bond0 instead of ifcfg-dd880d3b-b3b5-4968-9f89-db9d9407714e

How reproducible:
100%


Steps to Reproduce:
1. Install RHVH rhvh-4.0-0.20160812.0
2. Login cockpit via "ip:9090"
3. Enter to Networking page, setup bond0 with active-backup mode over two NICs(eno01,enp1s0).And eno01 is primary NIC, "Connect automatically".
4. check info in /etc/sysconfig/network-scripts

Actual results:
1. After step4, the bond0 ifcfg file name show as "ifcfg-3b1378a6-b417-48ed-87d8-7f3e5ecf05d3"


Expected results:
1. After step4, bond0 ifcfg file name should show as "ifcfg-bond0"

Additional info:

Comment 1 Red Hat Bugzilla Rules Engine 2016-08-16 10:01:34 UTC
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

Comment 2 jianwu 2016-08-16 10:02:48 UTC
Created attachment 1191184 [details]
The bond0's ifcfg file name display in /etc/sysconfig/network-scripts

Comment 3 jianwu 2016-08-16 10:05:40 UTC
Created attachment 1191186 [details]
The related log info packages about bond0 setup

Comment 4 Fabian Deutsch 2016-08-16 12:57:28 UTC
This might be the normal behavior of NM when creatign bond ifcfg files.

Is the actual problem just the name of the ifcfg, or are there functional side effects?

Comment 5 Dan Kenigsberg 2016-08-16 13:05:33 UTC
Vdsm assumes the stadard initscripts' naming of ifcfg files. Changing this at this stage of ovirt-4.0.z is unacceptable.

dperpeet, can you tell what could have caused this?

Comment 6 Ondřej Svoboda 2016-08-16 13:46:19 UTC
NM documentation doesn't indicate we could control the way the ifcfg-rh plugin names ifcfg-* files.

1) In https://developer.gnome.org/NetworkManager/stable/NetworkManager.conf.html other plugins' settings are documented there but the ifcfg-rh plugin appears to have no extra settings.

2) There is no explanation of file names even in https://developer.gnome.org/NetworkManager/unstable/nm-settings-ifcfg-rh.html -- only the various extra keys that NM may store in ifcfg file.

Comment 7 Ondřej Svoboda 2016-08-16 13:49:31 UTC
As for the reason why "ifcfg-UUID" format is used here, we may speculate that this is because NM handles multiple "connections", or profiles.

The ifcfg-rh plugin doesn't use any other means of persistence of such profiles, and so the "ifcfg-DEV" would result in clashes.

Comment 8 Fabian Deutsch 2016-08-16 16:29:20 UTC
It seems to be a regression of cockpit 114.

With the following commit a UUID is used as a the ID, which is causing the ifcfg-<uuid> naming.

This patch was part of cockpit-114, which landed in the last RHVH build.
Before we were carrying cockpit-112.

Author: Marius Vollmer <mvollmer>  2014-06-25 13:40:56
Committer: Stef Walter <stefw>  2014-06-30 17:58:43
Parent: 0e1ee43b621ecf9b3f052e004aec7d2f3751b4db (shell: Removed unused boolbox function)
Child:  e6b8563a460b82be9af88343c18193359d934a82 (shell: Support deletion of deletable network interfaces)
Branches: master, os-release, remotes/origin/master
Follows: 0.13
Precedes: 0.14

    shell: Support for adding/removing bond slaves
    
    Reviewed-by: Stef Walter <stefw>

Comment 9 Fabian Deutsch 2016-08-16 19:20:36 UTC
A few clarifications:

The ID is used for naming the ifcfg devices. In cockpit a UUID is used for the NM ID (there is an additional NM internal UUID). So this bug is caused when Cockpit is used.

Also, it might not actually be a regression but rather a long standing way of how cockpit is doing this - I see it's done the same way in Cockpit 0.96.

My guess is that this was not discovered before because it was hidden by bug 1329954.

Based on the current state I see the following implications

With this naming, vdsm will not be able to use the ifcfg files correctly (IIUIC).

This means

At GA a user can
- still drop to shell to configure a bond by a) writing ifcfg files b) using nmtui and nmcli

At GA a user can _NOT_
- use Cockpit for creating bonds, bridges (it affects bridges as well), vlans (needs to be verified)


I also see a few options how we can address this bug
- Change the Cockpit default to use "iface" instead of "uuid" as the NM ID
- Add additional fields to the Cockpit interface to allow setting the NM ID

The two approaches above are probably not doable for GA.

Adventures hack: Let's have a service watching ifcfg scripts, and atomically rename ifcfg files to match vdsm's expectation.

Thoughts?

Comment 10 Fabian Deutsch 2016-08-16 19:30:53 UTC
Comment 9 is based on reading code, and looking at cockpit 0.96, thus it's an assumption and I did not verify my understanding by patching Cockpit to see if the files are then named correctly.

Relevant lines: <cockpit-0.114>/pkg/networkmanager/interfaces.js:1565:

                connection: {
                    id: uuid,               <-- HERE
                    autoconnect: false,
                    type: "bond",
                    uuid: uuid,
                    interface_name: iface
                },

Comment 11 Yaniv Kaul 2016-08-16 20:47:09 UTC
The first option makes more sense to me.
But we need to see how much we'll need to hack this.
For example, is it just for bond0?
Consult with the cockpit team.

For rn, don't do bond via cockpit sounds OK to me.

Comment 12 Ryan Barry 2016-08-17 02:23:34 UTC
(In reply to Fabian Deutsch from comment #9)
> A few clarifications:
> 
> The ID is used for naming the ifcfg devices. In cockpit a UUID is used for
> the NM ID (there is an additional NM internal UUID). So this bug is caused
> when Cockpit is used.
>
> At GA a user can _NOT_
> - use Cockpit for creating bonds, bridges (it affects bridges as well),
> vlans (needs to be verified)

Also verified that this affects vlans.


> I also see a few options how we can address this bug
> - Change the Cockpit default to use "iface" instead of "uuid" as the NM ID
> - Add additional fields to the Cockpit interface to allow setting the NM ID
> 
> The two approaches above are probably not doable for GA.
> 
> Adventures hack: Let's have a service watching ifcfg scripts, and atomically
> rename ifcfg files to match vdsm's expectation.
> 
> Thoughts?

So, this works:

https://gist.github.com/evol262/5f694f655b3b8c3ebb9e045bcfdc1e92

Verified working with bridges, bonds, and vlans.

We need to wait a second or so before removing the file, otherwise cockpit gets unhappy.

Is it possible to make vdsm ignore symlinks? If so, the gist above works perfectly.

If not, we can remove the symlinking and just go with os.rename, but cockpit expects to be able to read ifcfg-${uuid} directly for bond creation/deletion, and we get some unfriendly errors...

Comment 13 Ryan Barry 2016-08-17 02:24:07 UTC
Created attachment 1191450 [details]
bond_creation.png

Comment 14 Ryan Barry 2016-08-17 02:24:30 UTC
Created attachment 1191451 [details]
bond_deletion.png

Comment 15 Ryan Barry 2016-08-17 02:25:43 UTC
(In reply to Yaniv Kaul from comment #11)
> The first option makes more sense to me.
> But we need to see how much we'll need to hack this.
> For example, is it just for bond0?
> Consult with the cockpit team.
> 
> For rn, don't do bond via cockpit sounds OK to me.

FYI, the ifcfg-${uuid} issue applies to:

bonds
vlans
bridges

So any/all of those configurations result in ifcfg filenames vdsm does not like.

Comment 16 Ryan Barry 2016-08-17 03:17:15 UTC
Small update:

patching doesn't seem to be a reliable solution, but needs more testing.

Since bundle.min.js is compiled/minimized, it takes a PCRE (I'm using a perl onliner, but could be anything) to appropriately substitute the names.

However, this works for bonds and bridges. Not vlans. VLANS have:

interface_name: ""

Which throws an error in cockpit (connection.id: property is empty).

I'll see if I get a more clever patch in the morning.

Comment 17 Yaniv Lavi 2016-08-17 06:21:25 UTC
Can we please approach the Cockpit team with this issue?

Comment 18 Marius Vollmer 2016-08-17 07:28:41 UTC
Cockpit has always used the UUID as the ID when creating new connection settings.

The reason is weak: I didn't want to worry about using the same ID twice.

Using the UUID as the ID is annoying if you drop to the NetworkManager level, and I agree that using a more standard ID is strictly better.  So let's change that in Cockpit:

    https://github.com/cockpit-project/cockpit/issues/4888

Comment 19 Marius Vollmer 2016-08-17 07:29:09 UTC
> So, this works:
> 
> https://gist.github.com/evol262/5f694f655b3b8c3ebb9e045bcfdc1e92
> 
> Verified working with bridges, bonds, and vlans.
> 
> We need to wait a second or so before removing the file, otherwise cockpit
> gets unhappy.
> 
> Is it possible to make vdsm ignore symlinks? If so, the gist above works
> perfectly.
> 
> If not, we can remove the symlinking and just go with os.rename, but cockpit
> expects to be able to read ifcfg-${uuid} directly for bond
> creation/deletion, and we get some unfriendly errors...

Can you elaborate about the unhappiness and unfriendly errors?  Does "nmcli connection reload" help after changing the files?  It's probably worth filing some bugs about this.

Comment 20 Marius Vollmer 2016-08-17 07:31:39 UTC
(In reply to Marius Vollmer from comment #19)

> Can you elaborate about the unhappiness and unfriendly errors?

Ahh, found the screenshots, sorry about the noise.

Comment 21 Marius Vollmer 2016-08-17 07:38:59 UTC
(In reply to Dan Kenigsberg from comment #5)
> Vdsm assumes the stadard initscripts' naming of ifcfg files.

What is the standard naming?  NetworkManager sometimes uses IDs like "Wired Connection 5" with a file named "ifcfg-Wired_connection_5".  Are these also problematic?

My current plan is to use the interface name as the ID, and add "-1", "-2", etc suffixes until it is unique.  Would that work?

Comment 22 Fabian Deutsch 2016-08-17 07:59:03 UTC
The current assumption of vdsm is to have the ifcfg files named ifcfg-<ifname> - i.e. ifcfg-bond0 ifcfg-bond42 ifcfg-br0
I don't recall the naming for vlans from the top of my head.

Comment 23 Dan Kenigsberg 2016-08-17 08:20:53 UTC
(In reply to Marius Vollmer from comment #21)
> (In reply to Dan Kenigsberg from comment #5)
> 
> My current plan is to use the interface name as the ID, and add "-1", "-2",
> etc suffixes until it is unique.  Would that work?

Sounds good. Using interface names (e.g. bond0.200 for a vlan device on top of a bonding device) is what I am asking for.

How can these be non-unique with Cockpit? Can Cockpit ever create two connections on top of a single device?

Comment 24 Marius Vollmer 2016-08-17 08:27:20 UTC
IDs don't seem to need to be unique, so let's go with this:

    https://github.com/cockpit-project/cockpit/pull/4889

Comment 25 Marius Vollmer 2016-08-17 08:50:20 UTC
(In reply to Dan Kenigsberg from comment #23)
 
> How can these be non-unique with Cockpit? Can Cockpit ever create two
> connections on top of a single device?

Using Cockpit exclusively should be no problem, but people might create connections in other ways.

In any case, unique connection ids are not necessary.  I misunderstood this.

Comment 26 Fabian Deutsch 2016-08-17 10:41:53 UTC
Considering that we'll pull in bug 1367669 to disable the network page in Cockpit, this bug becomes irrelevant, and can thus be moved out of 4.0.

Comment 27 Fabian Deutsch 2016-08-19 10:24:02 UTC
*** Bug 1362353 has been marked as a duplicate of this bug. ***

Comment 28 Dan Kenigsberg 2016-08-21 09:00:38 UTC
With bug 1368764, cockpit would attempt to create interface-named NM connections.

However, this can fail if a preexisting same-named interface already exists, or if an oblivious nmcli user creates them.

When consuming NM-controlled interfaces, Vdsm should not care about ifcfg filenames.

Comment 29 Dominik Perpeet 2016-09-07 18:56:27 UTC
https://github.com/cockpit-project/cockpit/pull/4889 was released in cockpit 118

Comment 30 Edward Haas 2016-09-11 15:11:39 UTC
We will complement the fix by adding the following logic in VDSM ifcfg configurator:

- On request to touch a device, when the default ifcfg file is missing (ifcfg-<dev name>), VDSM will check an alternative ifcfg file.
- If an alternative ifcfg file has been found, it will rename it to the default name and proceed with the regular flow.

We will need to consider what to do in case multiple connections have been defined for the same device.

Comment 31 Michael Burman 2016-10-05 12:53:21 UTC
Not sure this should be verified, vdsm should have been remove all other connections(as agreed with edy and as i saw when tested the patches for this fix)after add host.

Although vdsm manage to handle the 3 different connections for 1 device -

[root@camel-vdsa ~]# cat /etc/sysconfig/network-scripts/ifcfg-enp4s0
# Generated by parse-kickstart
IPV6INIT=no
DEVICE="enp4s0"
ONBOOT="yes"
UUID="759dfa0f-0a1f-4093-96bf-a027d3f96cae"
TYPE=Ethernet
NAME="System enp4s0"
[root@camel-vdsa ~]# cat /etc/sysconfig/network-scripts/ifcfg-enp4s0-1
TYPE=Ethernet
BOOTPROTO=dhcp
DEFROUTE=yes
IPV4_FAILURE_FATAL=no
IPV6INIT=yes
IPV6_AUTOCONF=yes
IPV6_DEFROUTE=yes
IPV6_FAILURE_FATAL=no
IPV6_ADDR_GEN_MODE=stable-privacy
NAME=enp4s0-1
UUID=f3cf6619-20db-4d94-87e4-9635552ee43b
DEVICE=enp4s0
ONBOOT=yes
PEERDNS=yes
PEERROUTES=yes
IPV6_PEERDNS=yes
IPV6_PEERROUTES=yes
[root@camel-vdsa ~]# cat /etc/sysconfig/network-scripts/ifcfg-enp4s0-1
ifcfg-enp4s0-1    ifcfg-enp4s0-1-1  
[root@camel-vdsa ~]# cat /etc/sysconfig/network-scripts/ifcfg-enp4s0-1-1 
TYPE=Ethernet
BOOTPROTO=dhcp
DEFROUTE=yes
PEERDNS=yes
PEERROUTES=yes
IPV4_FAILURE_FATAL=no
IPV6INIT=yes
IPV6_AUTOCONF=yes
IPV6_DEFROUTE=yes
IPV6_PEERDNS=yes
IPV6_PEERROUTES=yes
IPV6_FAILURE_FATAL=no
IPV6_ADDR_GEN_MODE=stable-privacy
NAME=enp4s0-1
UUID=e54c9211-39f7-45f8-8bfa-d9dd1cd996dc
DEVICE=enp4s0
ONBOOT=yes

- After that add host succeeded over enp4s0, we still remain with the other connections, but we shouldn't, vdsm should remove them, no? 


[root@camel-vdsa ~]# cat /etc/sysconfig/network-scripts/ifcfg-enp4s0
ifcfg-enp4s0      ifcfg-enp4s0-1    ifcfg-enp4s0-1-1  

# Generated by VDSM version 4.18.15-1.el7ev
DEVICE=enp4s0
BRIDGE=ovirtmgmt
ONBOOT=yes
MTU=1500
NM_CONTROLLED=no
IPV6INIT=no
[root@camel-vdsa ~]# cat /etc/sysconfig/network-scripts/ifcfg-enp4s0-1
TYPE=Ethernet
BOOTPROTO=dhcp
DEFROUTE=yes
IPV4_FAILURE_FATAL=no
IPV6INIT=yes
IPV6_AUTOCONF=yes
IPV6_DEFROUTE=yes
IPV6_FAILURE_FATAL=no
IPV6_ADDR_GEN_MODE=stable-privacy
NAME=enp4s0-1
UUID=f3cf6619-20db-4d94-87e4-9635552ee43b
DEVICE=enp4s0
ONBOOT=yes
PEERDNS=yes
PEERROUTES=yes
IPV6_PEERDNS=yes
IPV6_PEERROUTES=yes
[root@camel-vdsa ~]# cat /etc/sysconfig/network-scripts/ifcfg-enp4s0-1-1
TYPE=Ethernet
BOOTPROTO=dhcp
DEFROUTE=yes
PEERDNS=yes
PEERROUTES=yes
IPV4_FAILURE_FATAL=no
IPV6INIT=yes
IPV6_AUTOCONF=yes
IPV6_DEFROUTE=yes
IPV6_PEERDNS=yes
IPV6_PEERROUTES=yes
IPV6_FAILURE_FATAL=no
IPV6_ADDR_GEN_MODE=stable-privacy
NAME=enp4s0-1
UUID=e54c9211-39f7-45f8-8bfa-d9dd1cd996dc
DEVICE=enp4s0
ONBOOT=yes

Comment 32 Dan Kenigsberg 2016-10-05 13:28:03 UTC
sorry, but vdsm-4.18.15 does not contain the fix. Only 4.18.16 would.

Comment 33 Michael Burman 2016-10-26 08:31:38 UTC
Verified with vdsm-4.18.15.2-1.el7ev.x86_64


Note You need to log in before you can comment on or make changes to this bug.