[PATCH rhel7-branch] (v2) network: add support for --no-activate kickstart opton (#1277975)
Resolves: rhbz#1277975
This will allow to override special default of first network command which is --activate. --- dracut/parse-kickstart | 3 ++- pyanaconda/network.py | 2 +- pyanaconda/ui/gui/spokes/network.py | 8 +++++++- pyanaconda/ui/tui/spokes/network.py | 7 ++++++- 4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/dracut/parse-kickstart b/dracut/parse-kickstart index 250274e..bc9cbdd 100755 --- a/dracut/parse-kickstart +++ b/dracut/parse-kickstart @@ -178,7 +178,8 @@ class Network(commands.network.RHEL7_Network, DracutArgsMixin):
# first 'network' line if len(self.network) == 1: - net.activate = True + if net.activate is None: + net.activate = True # Note that there may be no net.device and no ksdevice if inst.ks=file:/ks.cfg # If that is the case, fall into ksnet_to_dracut with net.device=None and let # it handle things. diff --git a/pyanaconda/network.py b/pyanaconda/network.py index f04b1c7..38e3a18 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -1236,7 +1236,7 @@ def apply_kickstart(ksdata): # If we have kickstart ifcfg from initramfs if "Generated by parse-kickstart" in f.read(): # and we should activate the device - if i == 0 or network_data.activate: + if network_data.activate or (i == 0 and network_data.activate is None): ifcfg = IfcfgFile(ifcfg_path) ifcfg.read() con_uuid = ifcfg.get("UUID") diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py index 052a375..e2f3504 100644 --- a/pyanaconda/ui/gui/spokes/network.py +++ b/pyanaconda/ui/gui/spokes/network.py @@ -1673,13 +1673,19 @@ class NetworkStandaloneSpoke(StandaloneSpoke):
def _update_network_data(data, ncb): data.network.network = [] - for dev_cfg in ncb.dev_cfgs: + for i, dev_cfg in enumerate(ncb.dev_cfgs): devname = dev_cfg.get_iface() nd = network.ksdata_from_ifcfg(devname, dev_cfg.con_uuid) if not nd: continue if devname in nm.nm_activated_devices(): nd.activate = True + else: + # First network command defaults to --activate so we must + # use --no-activate explicitly to prevent the default + if i == 0: + nd.activate = False + data.network.network.append(nd) hostname = ncb.hostname network.update_hostname_data(data, hostname) diff --git a/pyanaconda/ui/tui/spokes/network.py b/pyanaconda/ui/tui/spokes/network.py index f60b9ec..b0067fe 100644 --- a/pyanaconda/ui/tui/spokes/network.py +++ b/pyanaconda/ui/tui/spokes/network.py @@ -252,12 +252,17 @@ class NetworkSpoke(FirstbootSpokeMixIn, EditTUISpoke): hostname = self.data.network.hostname
self.data.network.network = [] - for name in nm.nm_devices(): + for i, name in enumerate(nm.nm_devices()): nd = network.ksdata_from_ifcfg(name) if not nd: continue if name in nm.nm_activated_devices(): nd.activate = True + else: + # First network command defaults to --activate so we must + # use --no-activate explicitly to prevent the default + if i == 0: + nd.activate = False self.data.network.network.append(nd)
(valid, error) = network.sanityCheckHostname(self.hostname_dialog.value)
Related: rhbz#1277975
This will allow to override special default of first network command which is --activate. --- pykickstart/commands/network.py | 12 ++++++++---- tests/commands/network.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/pykickstart/commands/network.py b/pykickstart/commands/network.py index 4daeece..a9b12b3 100644 --- a/pykickstart/commands/network.py +++ b/pykickstart/commands/network.py @@ -155,7 +155,7 @@ class F16_NetworkData(F8_NetworkData):
def __init__(self, *args, **kwargs): F8_NetworkData.__init__(self, *args, **kwargs) - self.activate = kwargs.get("activate", False) + self.activate = kwargs.get("activate", None) self.nodefroute = kwargs.get("nodefroute", False) self.wpakey = kwargs.get("wpakey", "")
@@ -276,7 +276,7 @@ class RHEL6_NetworkData(F8_NetworkData):
def __init__(self, *args, **kwargs): F8_NetworkData.__init__(self, *args, **kwargs) - self.activate = kwargs.get("activate", False) + self.activate = kwargs.get("activate", None) self.nodefroute = kwargs.get("nodefroute", False) self.vlanid = kwargs.get("vlanid", "") self.bondslaves = kwargs.get("bondslaves", "") @@ -314,6 +314,8 @@ class RHEL7_NetworkData(F21_NetworkData): retval += " --bridgeslaves=%s" % self.bridgeslaves if self.bridgeopts != "": retval += " --bridgeopts=%s" % self.bridgeopts + if self.activate == False: + retval += " --no-activate"
return retval
@@ -435,7 +437,7 @@ class F16_Network(F9_Network): def _getParser(self): op = F9_Network._getParser(self) op.add_option("--activate", dest="activate", action="store_true", - default=False) + default=None) op.add_option("--nodefroute", dest="nodefroute", action="store_true", default=False) op.add_option("--wpakey", dest="wpakey", action="store", default="") @@ -582,7 +584,7 @@ class RHEL6_Network(F9_Network): def _getParser(self): op = F9_Network._getParser(self) op.add_option("--activate", dest="activate", action="store_true", - default=False) + default=None) op.add_option("--nodefroute", dest="nodefroute", action="store_true", default=False) op.add_option("--vlanid", dest="vlanid") @@ -639,6 +641,8 @@ class RHEL7_Network(F21_Network): default="") op.add_option("--bridgeopts", dest="bridgeopts", action="store", default="") + op.add_option("--no-activate", dest="activate", action="store_false", + default=None) return op
def parse(self, args): diff --git a/tests/commands/network.py b/tests/commands/network.py index cd150d1..73bcc04 100644 --- a/tests/commands/network.py +++ b/tests/commands/network.py @@ -152,6 +152,17 @@ class RHEL7_TestCase(F20_TestCase): "--bridgeopts=priority", KickstartValueError)
+ # activating a device + network_data = self.assert_parse("network --device eth0") + self.assertEquals(network_data.activate, None) + network_data = self.assert_parse("network --device eth0 --no-activate") + self.assertEquals(network_data.activate, False) + network_data = self.assert_parse("network --device eth0 --activate") + self.assertEquals(network_data.activate, True) + network_data = self.assert_parse("network --device eth0 --activate --no-activate") + self.assertEquals(network_data.activate, False) + network_data = self.assert_parse("network --device eth0 --no-activate --activate") + self.assertEquals(network_data.activate, True)
if __name__ == "__main__": unittest.main()
On Tue, 2016-05-31 at 14:35 +0200, Radek Vykydal wrote:
+ network_data = self.assert_parse("network --device eth0 -- activate --no-activate") + self.assertEquals(network_data.activate, False) + network_data = self.assert_parse("network --device eth0 -- no-activate --activate") + self.assertEquals(network_data.activate, True)
Shouldn't this two tests give us an error/exception instead? I see this as a big place to generate errors from missing one of these parameters in a command.
I think good behavior here should be to let user know that these commands can't be used together.
On Wed, 2016-06-01 at 08:51 +0200, Jiří Konečný wrote:
On Tue, 2016-05-31 at 14:35 +0200, Radek Vykydal wrote:
+ network_data = self.assert_parse("network --device eth0 -- activate --no-activate") + self.assertEquals(network_data.activate, False) + network_data = self.assert_parse("network --device eth0 -- no-activate --activate") + self.assertEquals(network_data.activate, True)
Shouldn't this two tests give us an error/exception instead? I see this as a big place to generate errors from missing one of these parameters in a command.
I think good behavior here should be to let user know that these commands can't be used together.
AFAICT, it's basically impossible to check for this now with both options using the 'activate' attribute. And the policy is that the last one wins which should be okay and many command line tools work the same way with such options.
On 1.6.2016 09:16, Vratislav Podzimek wrote:
On Wed, 2016-06-01 at 08:51 +0200, Jiří Konečný wrote:
On Tue, 2016-05-31 at 14:35 +0200, Radek Vykydal wrote:
network_data = self.assert_parse("network --device eth0 --activate --no-activate")
self.assertEquals(network_data.activate, False)network_data = self.assert_parse("network --device eth0 --no-activate --activate")
self.assertEquals(network_data.activate, True)Shouldn't this two tests give us an error/exception instead? I see this as a big place to generate errors from missing one of these parameters in a command.
I think good behavior here should be to let user know that these commands can't be used together.
AFAICT, it's basically impossible to check for this now with both options using the 'activate' attribute. And the policy is that the last one wins which should be okay and many command line tools work the same way with such options.
This is exactly my opinion.
On Wed, 2016-06-01 at 12:47 +0200, Radek Vykydal wrote:
On 1.6.2016 09:16, Vratislav Podzimek wrote:
On Wed, 2016-06-01 at 08:51 +0200, Jiří Konečný wrote:
On Tue, 2016-05-31 at 14:35 +0200, Radek Vykydal wrote:
+ network_data = self.assert_parse("network --device eth0 -- activate --no-activate") + self.assertEquals(network_data.activate, False) + network_data = self.assert_parse("network --device eth0 -- no-activate --activate") + self.assertEquals(network_data.activate, True)
Shouldn't this two tests give us an error/exception instead? I see this as a big place to generate errors from missing one of these parameters in a command.
I think good behavior here should be to let user know that these commands can't be used together.
AFAICT, it's basically impossible to check for this now with both options using the 'activate' attribute. And the policy is that the last one wins which should be okay and many command line tools work the same way with such options.
This is exactly my opinion.
I'm not quite convinced that this behavior is the good one but if it's impossible to check then you have ACK from me too.
Both of these look good to me.
On Tue, 2016-05-31 at 10:31 -0700, Brian C. Lane wrote:
Both of these look good to me.
Same here.
anaconda-patches@lists.fedorahosted.org
-
Brian C. Lane -
Jiří Konečný -
Radek Vykydal -
Vratislav Podzimek