Skip to content

Commit

Permalink
Merge pull request #121 from yast/not_write_when_syncing
Browse files Browse the repository at this point in the history
Not write when syncing and save the user status according to the user preferences
  • Loading branch information
teclator committed Nov 8, 2018
2 parents e07579b + 2911814 commit bfe634c
Show file tree
Hide file tree
Showing 5 changed files with 261 additions and 68 deletions.
9 changes: 9 additions & 0 deletions package/yast2-ntp-client.changes
@@ -1,3 +1,12 @@
-------------------------------------------------------------------
Fri Sep 7 15:20:26 UTC 2018 - dgonzalez@suse.com

- Save the service status according to the user preferences
(bsc#1075039)
- Only write the configuration once, and do not save changes when
we are only synchronizing the date. (bsc#1108497)
- 3.2.17

-------------------------------------------------------------------
Tue Apr 17 07:03:46 UTC 2018 - jreidinger@suse.com

Expand Down
2 changes: 1 addition & 1 deletion package/yast2-ntp-client.spec
Expand Up @@ -17,7 +17,7 @@


Name: yast2-ntp-client
Version: 3.2.16
Version: 3.2.17
Release: 0
Summary: YaST2 - NTP Client Configuration
License: GPL-2.0+
Expand Down
123 changes: 56 additions & 67 deletions src/clients/ntp-client_proposal.rb
@@ -1,13 +1,13 @@
# encoding: utf-8

# File: clients/ntp-client_proposal.ycp
# Summary: Installation client for ntp configuration
# Author: Bubli <kmachalkova@suse.cz>
#
# This is used as the general interface between yast2-country
# (time,timezone) and yast2-ntp-client.
require "yast"

module Yast
# This is used as the general interface between yast2-country
# (time,timezone) and yast2-ntp-client.
class NtpClientProposalClient < Client
include Logger

def main
Yast.import "UI"
textdomain "ntp-client"
Expand Down Expand Up @@ -339,76 +339,47 @@ def WriteNtpSettings(ntp_servers, ntp_server, run_service)
true
end

# params:
# server (taken from UI if empty)
# servers (intended to use all of opensuse.pool.ntp.org,
# but I did not have time to make it work)
# run_service (set to true if empty)
# write_only (bnc#589296)
# ntpdate_only (TODO rename to onetime)
# return:
# `success, `invalid_hostname or `ntpdate_failed
def Write(param)
ntp_servers = param["servers"] || []
ntp_server = param["server"] || ""
run_service = param.fetch("run_service", true)
if ntp_server == ""
# get the value from UI only when it wasn't given as a parameter
ntp_server = UI.QueryWidget(Id(:ntp_address), :Value)
end
return :invalid_hostname if !ValidateSingleServer(ntp_server)
# Writes the NTP settings
#
# @param [Hash] params
# @option params [String] "server" The NTP server address, taken from the UI if empty
# @option params [Array<String>] "servers" A collection of NTP servers
# @option params [Boolean] "run_service" Whether service should be active and enable
# @option params [Boolean] "write_only" If only is needed to write the settings, (bnc#589296)
# @option params [Boolean] "ntpdate_only" ? TODO: rename to onetime
#
# @return [Symbol] :invalid_hostname, when a not valid ntp_server is given
# :ntpdate_failed, when the ntp sychronization fails
# :success, when settings (and sync if proceed) were performed successfully
def Write(params)
log.info "ntp client proposal Write with #{params.inspect}"

WriteNtpSettings(ntp_servers, ntp_server, run_service)
return :success if param["write_only"]
# clean params
params.delete_if { |_, v| v.nil? }

# One-time adjusment without running the ntp daemon
# Meanwhile, ntpdate was replaced by sntp
ntpdate_only = param["ntpdate_only"]
ntp_server = params.fetch("server", "")
ntp_servers = params.fetch("servers", [])
run_service = params.fetch("run_service", NtpClient.run_service)

required_package = "ntp"
# Get the ntp_server value from UI only if isn't present (probably wasn't given as parameter)
ntp_server = UI.QueryWidget(Id(:ntp_address), :Value) if ntp_server.strip.empty?

# In 1st stage, schedule packages for installation
if Stage.initial
Yast.import "Packages"
Packages.addAdditionalPackage(required_package)
# Otherwise, prompt user for confirming pkg installation
elsif !PackageSystem.CheckAndInstallPackages([required_package])
Report.Error(
Builtins.sformat(
_(
"Synchronization with NTP server is not possible\nwithout package %1 installed."
),
required_package
)
)
end
return :invalid_hostname unless ValidateSingleServer(ntp_server)

add_or_install_required_package unless params["write_only"]

WriteNtpSettings(ntp_servers, ntp_server, run_service) unless params["ntpdate_only"]

return :success if params["write_only"]

ret = 0
# Only if network is running try to synchronize the ntp server
if NetworkService.isNetworkRunning
# Only if network is running try to synchronize the ntp server
Popup.ShowFeedback("", _("Synchronizing with NTP server..."))

Builtins.y2milestone("Running sntp to sync with %1", ntp_server)

# -S: do set the system time
# -t 5: timeout of 5 seconds
# -K /dev/null: use /dev/null as KoD history file (if not specified,
# /var/db/ntp-kod will be used and it doesn't exist)
# -l <file>: log to a file to not mess text mode installation
# -c: causes all IP addresses to which ntp_server resolves to be queried in parallel
ret = SCR.Execute(
path(".target.bash"),
"/usr/sbin/sntp -S -K /dev/null -l /var/log/YaST2/sntp.log " \
"-t 5 -c '#{String.Quote(ntp_server)}'"
)
Builtins.y2milestone("'sntp %1' returned %2", ntp_server, ret)
exit_code = NtpClient.sync_once(ntp_server)
Popup.ClearFeedback
end

return :ntpdate_failed if ret != 0

# User wants to more than running sntp (synchronize on boot)
WriteNtpSettings(ntp_servers, ntp_server, run_service) if !ntpdate_only
return :ntpdate_failed unless exit_code.zero?
end

:success
end
Expand Down Expand Up @@ -485,6 +456,24 @@ def ui_try_save
# success, exit
true
end

private

def add_or_install_required_package
# In 1st stage, schedule packages for installation
if Stage.initial
Yast.import "Packages"
Packages.addAdditionalPackage(NtpClientClass::REQUIRED_PACKAGE)
# Otherwise, prompt user for confirming pkg installation
elsif !PackageSystem.CheckAndInstallPackages([NtpClientClass::REQUIRED_PACKAGE])
Report.Error(
Builtins.sformat(
_("Synchronization with NTP server is not possible\nwithout package %1 installed."),
NtpClientClass::REQUIRED_PACKAGE
)
)
end
end
end
end

Expand Down
26 changes: 26 additions & 0 deletions src/modules/NtpClient.rb
Expand Up @@ -37,6 +37,9 @@ class NtpClientClass < Module

NTP_FILE = "/etc/ntp.conf".freeze

# Package which is needed for saving NTP configuration into system
REQUIRED_PACKAGE = "ntp".freeze

def main
Yast.import "UI"
textdomain "ntp-client"
Expand Down Expand Up @@ -180,6 +183,29 @@ def progress?
Mode.normal
end

# Synchronize against specified server only one time and does not modify
# any configuration
# @param server [String] to sync against
# @return [Integer] exit code of sync command
def sync_once(server)
log.info("Running sntp to sync with #{server}")

# -S: do set the system time
# -t 5: timeout of 5 seconds
# -K /dev/null: use /dev/null as KoD history file (if not specified,
# /var/db/ntp-kod will be used and it doesn't exist)
# -l <file>: log to a file to not mess text mode installation
# -c: causes all IP addresses to which ntp_server resolves to be queried in parallel
ret = SCR.Execute(
path(".target.bash"),
"/usr/sbin/sntp -S -K /dev/null -l /var/log/YaST2/sntp.log " \
"-t 5 -c '#{String.Quote(server)}'"
)
log.info("'sntp #{server}' returned #{ret}")

ret["exit"]
end

# Reads and returns all known countries with their country codes
#
# @return [Hash{String => String}] of known contries
Expand Down
169 changes: 169 additions & 0 deletions test/ntp_client_proposal_test.rb
@@ -0,0 +1,169 @@
require_relative "test_helper"

require_relative "../src/clients/ntp-client_proposal.rb"

Yast.import "Packages"

describe Yast::NtpClientProposalClient do
subject do
client = described_class.new
client.main
client
end

describe "#Write" do
let(:ntp_server) { "fake.pool.ntp.org" }
let(:write_only) { false }
let(:ntpdate_only) { false }
let(:params) do
{
"server" => ntp_server,
"write_only" => write_only,
"ntpdate_only" => ntpdate_only
}
end
let(:ntp_client) { Yast::NtpClient }
let(:initial_stage) { false }
let(:network_running) { false }

before do
allow(subject).to receive(:WriteNtpSettings)

allow(Yast::Stage).to receive(:initial).and_return(initial_stage)
allow(Yast::PackageSystem).to receive(:CheckAndInstallPackages)
allow(Yast::Report).to receive(:Error)
allow(Yast::NetworkService).to receive(:isNetworkRunning).and_return(network_running)
end

context "with a not valid hostname" do
let(:ntp_server) { nil }

it "does not write settings" do
expect(subject).to_not receive(:WriteNtpSettings)

subject.Write(params)
end

it "returns :invalid_hostname" do
expect(subject.Write(params)).to eq(:invalid_hostname)
end
end

context "with valid hostname" do
before do
allow(subject).to receive(:ValidateSingleServer).and_return(true)
end

context "but in 'write_only' mode" do
let(:write_only) { true }

it "only writes settings" do
expect(subject).to receive(:WriteNtpSettings).once
expect(Yast::Stage).to_not receive(:initial)
expect(Yast::NetworkService).to_not receive(:isNetworkRunning)

subject.Write(params)
end

it "returns :success" do
expect(subject.Write(params)).to eq(:success)
end
end

context "but 'run_service' param is not given" do
it "uses the current value of NtpClient.run_service" do
ntp_client.run_service = true
expect(subject).to receive(:WriteNtpSettings).with(anything, anything, true)
subject.Write({})

ntp_client.run_service = false
expect(subject).to receive(:WriteNtpSettings).with(anything, anything, false)
subject.Write({})
end
end

context "and is in the initial stage" do
let(:initial_stage) { true }

it "imports Yast::Packages" do
allow(Yast).to receive(:import).and_call_original
expect(Yast).to receive(:import).with("Packages")

subject.Write(params)
end

it "adds the additional package" do
expect(Yast::Packages).to receive(:addAdditionalPackage)

subject.Write(params)
end
end

context "and is not in the initial stage" do
it "asks user to confirm the package installation" do
expect(Yast::PackageSystem).to receive(:CheckAndInstallPackages)

subject.Write(params)
end

context "but user rejects the package installation" do
before do
allow(Yast::PackageSystem).to receive(:CheckAndInstallPackages).and_return(false)
end

it "reports an error" do
expect(Yast::Report).to receive(:Error).with(/Synchronization with NTP server is not/)

subject.Write(params)
end
end
end

context "and network is not available" do
it "does not performs the ntp syncronization" do
expect(Yast::NtpClient).to_not receive(:sync_once)

subject.Write(params)
end

it "returns :success" do
expect(subject.Write(params)).to eq(:success)
end
end

context "and network is available" do
let(:network_running) { true }

it "returns :ntpdate_failed if synchronization fails" do
allow(Yast::NtpClient).to receive(:sync_once).with(ntp_server).and_return(1)

expect(subject.Write(params)).to eq(:ntpdate_failed)
end

it "returns :success if syncronization was successfully" do
allow(Yast::NtpClient).to receive(:sync_once).with(ntp_server).and_return(0)

expect(subject.Write(params)).to eq(:success)
end
end

context "and user only wants to syncronize date" do
let(:ntpdate_only) { true }

it "does not write settings" do
expect(subject).to_not receive(:WriteNtpSettings)

subject.Write(params)
end
end

context "and user wants to syncronize on boot" do
it "writes settings only once" do
expect(subject).to receive(:WriteNtpSettings).once

subject.Write(params)
end
end
end
end
end

0 comments on commit bfe634c

Please sign in to comment.