[node-patches] Change in ovirt-node[master]: ntp: include peerntp_no karg

[fabiand at redhat.com]

Fabian Deutsch has posted comments on this change.

Change subject: ntp: include peerntp_no karg
......................................................................


Patch Set 2: Code-Review-1

(7 comments)

https://gerrit.ovirt.org/#/c/40406/2/scripts/ovirt-init-functions.sh.in
File scripts/ovirt-init-functions.sh.in:

Line 787: peerntp_no
Please use a argument like peerntp=no, instead of a flag peerntp_no that fit's better to what we have today


https://gerrit.ovirt.org/#/c/40406/2/src/ovirt/node/config/defaults.py
File src/ovirt/node/config/defaults.py:

Line 430:                 # peerntp_no karg and do not include in any
Line 431:                 # ifcfg file the PEERNTP=yes. This will avoid
Line 432:                 # any update in ntp.conf by the nic interfaces
Line 433:                 aug = utils.AugeasWrapper()
Line 434:                 sys_network = aug.get("/files/etc/sysconfig/network/PEERNTP")
Once the function I mention below is created, you can use it here as well.
Line 435:                 if sys_network and "no" in sys_network:
Line 436:                     cfg.peerntp = "no"
Line 437:                 else:
Line 438:                     cfg.peerntp = "yes"


Line 431:                 # ifcfg file the PEERNTP=yes. This will avoid
Line 432:                 # any update in ntp.conf by the nic interfaces
Line 433:                 aug = utils.AugeasWrapper()
Line 434:                 sys_network = aug.get("/files/etc/sysconfig/network/PEERNTP")
Line 435:                 if sys_network and "no" in sys_network:
The function for the global ntp setting should return a boolean, then there is no need for testing against "no".
Line 436:                     cfg.peerntp = "no"
Line 437:                 else:
Line 438:                     cfg.peerntp = "yes"
Line 439: 


Line 912: 
Line 913:     def configure(self, servers):
Line 914:         self.update(servers)
Line 915: 
Line 916:     def set_peerntp(self, option):
This logic needs to be split into two:

- One for the /etc/sysconfig/network file
- One for the ifcfg files.

The logic for the ifcfg files can go into ovirt.node.config.network.NIC and the logic for the global option can go into a new function, similar to ovirt.node.config.network.timeservers().

That new function shall take and return a boolean (True/False), not "yes"/"no" - because that would not be very pythonic.
Line 917:         """
Line 918:         Update PEERNTP option in:
Line 919:           /etc/sysconfig/network-scripts/
Line 920:           /etc/sysconfig/network


Line 925:         """
Line 926:         NETWORK_SCRIPTS = "/etc/sysconfig/network-scripts/"
Line 927:         NETWORK_FILE = "/files/etc/sysconfig/network/PEERNTP"
Line 928: 
Line 929:         aug = utils.AugeasWrapper()
This is one function (ovirt.node.config.network.peerntp())
Line 930:         if aug.get(NETWORK_FILE):
Line 931:             aug.set(NETWORK_FILE, option)
Line 932: 
Line 933:         for name in os.listdir(NETWORK_SCRIPTS):


Line 929:         aug = utils.AugeasWrapper()
Line 930:         if aug.get(NETWORK_FILE):
Line 931:             aug.set(NETWORK_FILE, option)
Line 932: 
Line 933:         for name in os.listdir(NETWORK_SCRIPTS):
This needs to go into the ovirt.node.config.network.NIC class.
Line 934:             sysconf_net_script = "/files" + NETWORK_SCRIPTS + "/PEERNTP"
Line 935:             if os.path.isfile(os.path.join(NETWORK_SCRIPTS,
Line 936:                               name)) and aug.get(sysconf_net_script):
Line 937:                 aug.set(sysconf_net_script, option)


https://gerrit.ovirt.org/#/c/40406/2/src/ovirt/node/setup/core/network_page.py
File src/ovirt/node/setup/core/network_page.py:

Line 379:             self.logger.info("Setting new timeservers: %s" % timeservers)
Line 380:             timesrv = defaults.Timeservers()
Line 381: 
Line 382:             # Validate if user removed both NTP entries in TUI
Line 383:             if not timeservers[0] and not timeservers[1]:
What about doing:

    if len(timeservers):
        True
    else:
        Flase
Line 384:                 timesrv.set_peerntp("no")
Line 385:             else:
Line 386:                 timesrv.set_peerntp("yes")
Line 387: 


-- 
To view, visit https://gerrit.ovirt.org/40406
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I127a78df38c21899f37fab2a55aea415f256540a
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes