Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a NtpSetup installation dialog #791

Merged
merged 8 commits into from
Apr 3, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions package/yast2-installation.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Fri Mar 29 12:58:51 UTC 2019 - David Diaz <dgonzalez@suse.com>

- Add a new installation dialog which allows to setup the NTP
servers (bsc#1129095).
- 4.2.1

-------------------------------------------------------------------
Tue Mar 19 09:19:10 UTC 2019 - David Díaz <dgonzalez@suse.com>

Expand Down
5 changes: 4 additions & 1 deletion package/yast2-installation.spec
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#

Name: yast2-installation
Version: 4.2.0
Version: 4.2.1
Release: 0

BuildRoot: %{_tmppath}/%{name}-%{version}-build
Expand Down Expand Up @@ -94,6 +94,9 @@ Requires: yast2-users >= 3.2.8
BuildRequires: yast2-country >= 3.3.1
Requires: yast2-country >= 3.3.1

# NtpSetup dialog (bsc#1129095)
BuildRequires: yast2-ntp-client

# Pkg::SourceProvideSignedFile Pkg::SourceProvideDigestedFile
# pkg-bindings are not directly required
Conflicts: yast2-pkg-bindings < 2.17.25
Expand Down
5 changes: 5 additions & 0 deletions src/clients/inst_ntp_setup.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require "yast"

require "installation/dialogs/ntp_setup"

::Installation::Dialogs::NtpSetup.run
121 changes: 121 additions & 0 deletions src/lib/installation/dialogs/ntp_setup.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# encoding: utf-8

# ------------------------------------------------------------------------------
# Copyright (c) 2019 SUSE LLC
#
#
# This program is free software; you can redistribute it and/or modify it under
# the terms of version 2 of the GNU General Public License as published by the
# Free Software Foundation.
#
# This program is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License along with
# this program; if not, contact SUSE.
#
# To contact SUSE about this file by physical or electronic mail, you may find
# current contact information at www.suse.com.
# ------------------------------------------------------------------------------

require "yast"
require "cwm/dialog"
require "installation/widgets/ntp_server"

module Installation
module Dialogs
# Simple dialog for settings the NTP server names
class NtpSetup < CWM::Dialog
def initialize
textdomain "installation"

Yast.import "Lan"
Yast.import "LanItems"
Yast.import "Product"
Yast.import "ProductFeatures"

super
end

# The dialog title
#
# @return [String] the title
def title
# TRANSLATORS: dialog title
_("NTP Setup")
end

def contents
return @content if @content

@content = HSquash(
MinWidth(
50,
# preselect the servers from the DHCP response
Widgets::NtpServer.new(ntp_servers)
)
)
end

private

# Propose the NTP servers from the DHCP response, fallback to a random
# machine from the ntp.org pool if enabled in control.xml.
#
# @return [Array<String>] proposed NTP servers, empty if nothing suitable found
def ntp_servers
# TODO: use Yast::NtpClient.ntp_conf if configured
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand this TODO comment. I mean, it is not clear to me if we can do what it is stated there or simply keep the current behavior and also de TODO comment.

@lslezak it seems that you wrote it originally. Any suggestion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC that value is set in the time zone configuration (if you go to the details and configure the NTP server there), so there a small duplication.

For now I'd just move it without further changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, keep it as it is, right?

# to better handle going back
servers = dhcp_ntp_servers
servers = [ntp_fallback] if servers.empty? && default_ntp_setup_enabled?

servers
end

# List of NTP servers from DHCP
#
# @return [Array<String>] List of servers (IP or host names), empty if not provided
def dhcp_ntp_servers
# When proposing NTP servers we need to know
#
# 1) list of (dhcp) interfaces
# 2) network service in use
#
# We can either use networking submodule for network service handling and get list of
# interfaces e.g. using a bash command or initialize whole networking module.
Yast::Lan.ReadWithCacheNoGUI

Yast::LanItems.dhcp_ntp_servers.values.flatten.uniq
end

# Whether the a default (fallback) NTP setup is enabled in the control.xml
#
# @return [Boolean]
def default_ntp_setup_enabled?
Yast::ProductFeatures.GetBooleanFeature("globals", "default_ntp_setup")
end

# The fallback servers for NTP configuration
#
# It propose a random pool server in range 0..3
#
# @return [String] the fallback servers
def ntp_fallback
"#{rand(4)}.#{ntp_host}.pool.ntp.org"
end

def ntp_host
# copied from timezone/dialogs.rb:
base_products = Yast::Product.FindBaseProducts

if base_products.any? { |p| p["name"] =~ /openSUSE/i }
"opensuse"
else
# TODO: use a SUSE server when available in the future
schubi2 marked this conversation as resolved.
Show resolved Hide resolved
"novell"
end
end
end
end
end
145 changes: 145 additions & 0 deletions src/lib/installation/widgets/ntp_server.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
# encoding: utf-8

# ------------------------------------------------------------------------------
# Copyright (c) 2019 SUSE LLC
#
#
# This program is free software; you can redistribute it and/or modify it under
# the terms of version 2 of the GNU General Public License as published by the
# Free Software Foundation.
#
# This program is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License along with
# this program; if not, contact SUSE.
#
# To contact SUSE about this file by physical or electronic mail, you may find
# current contact information at www.suse.com.
# ------------------------------------------------------------------------------

require "yast"
require "cwm/widget"
require "installation/system_role"

Yast.import "CWM"
Yast.import "Popup"
Yast.import "Label"
Yast.import "IP"
Yast.import "Hostname"
Yast.import "NtpClient"

module Installation
module Widgets
# This widget is responsible of validating and storing the NTP server to use.
class NtpServer < CWM::InputField
extend Yast::I18n

# @return [Array<String>] List of default servers
attr_reader :default_servers

# Constructor
#
# @param default_servers [Array<String>] List of servers
def initialize(default_servers = [])
textdomain "installation"

@default_servers = default_servers
end

# @return [String] Widget's label
def label
# TRANSLATORS: input field label
_("N&TP Servers (comma or space separated)")
end

# Store the value of the input field if validates
def store
if servers.empty?
# we need to reset the previous settings after going back
Yast::NtpClient.ntp_selected = false
Yast::NtpClient.modified = false

return
end

Yast::NtpClient.ntp_selected = true
Yast::NtpClient.modified = true
Yast::NtpClient.ntp_conf.clear_pools
servers.each { |server| Yast::NtpClient.ntp_conf.add_pool(server) }
# run NTP as a service (not via cron)
Yast::NtpClient.run_service = true
Yast::NtpClient.synchronize_time = false
end

# Initializes the widget's value
def init
saved_servers = (role && role["ntp_servers"]) || default_servers
self.value = saved_servers.join(" ")
end

NOT_VALID_SERVERS_MESSAGE = N_("Not valid location for the NTP servers:\n%{servers}" \
"\n\nPlease, enter a valid IP or Hostname").freeze
# Validate input
#
# * All specified IPs or hostnames should be valid
# * If no server is specified, ask the user whether proceed with installation or not
#
# @return [Boolean] true if value is valid; false otherwise.
def validate
return skip_ntp_server? if servers.empty?
invalid_servers = servers.reject { |v| Yast::IP.Check(v) || Yast::Hostname.CheckFQ(v) }
return true if invalid_servers.empty?
Yast::Popup.Error(
format(_(NOT_VALID_SERVERS_MESSAGE), servers: invalid_servers.join(", "))
)

false
end

def help
# TRANSLATORS: a help text for the NTP server input field
_("<h3>NTP Servers</h3>") +
# TRANSLATORS: a help text for the NTP server input field
_("<p>Enter the host name or the IP address of the NTP server which will be used for " \
"synchronizing the time on this machine</p>") +
# TRANSLATORS: a help text for the NTP server input field
_("<p>Use comma (,) or space to separate multiple values.</p>")
end

private

# Parse the widget's value an return the potential list of hostnames/addresses
#
# @return [Array<String>] List of hostnames/addresses
def servers
value.to_s.tr(",", " ").split(" ")
end

# Check if the user wants to intentionally skip the NTP server configuration
#
# @return [Boolean] true if user wants to skip it; false otherwise.
def skip_ntp_server?
# TODO: improve this "Kubic specific" wording because probably it is not appropiate for any
# possible openSUSE use case. See https://github.com/yast/yast-installation/pull/791#pullrequestreview-220944366
Yast::Popup.AnyQuestion(
_("NTP Servers"),
# TRANSLATORS: error message for invalid ntp server name/address
_("You have not configured an NTP server. This may lead to\n" \
"your cluster not functioning properly or at all.\n" \
dgdavid marked this conversation as resolved.
Show resolved Hide resolved
"Proceed with caution and at your own risk.\n\n" \
"Would you like to continue with the installation?"),
Yast::Label.YesButton,
Yast::Label.NoButton,
:yes
)
end

# Return the current role
def role
::Installation::SystemRole.current_role
end
end
end
end
55 changes: 55 additions & 0 deletions test/dialogs/ntp_setup_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#! /usr/bin/env rspec

require_relative "../test_helper.rb"
require "cwm/rspec"

require "installation/dialogs/ntp_setup"

Yast.import "CWM"
Yast.import "Lan"
Yast.import "Wizard"

describe ::Installation::Dialogs::NtpSetup do
describe "#run" do
let(:ntp_servers) { [] }

before do
allow(Yast::Wizard).to receive(:CreateDialog)
allow(Yast::Wizard).to receive(:CloseDialog)
allow(Yast::CWM).to receive(:show).and_return(:next)
allow(Yast::Lan).to receive(:ReadWithCacheNoGUI)
allow(Yast::LanItems).to receive(:dhcp_ntp_servers).and_return({})
allow(Yast::ProductFeatures).to receive(:GetBooleanFeature)
end

include_examples "CWM::Dialog"

context "when some NTP server is detected via DHCP" do
let(:ntp_servers) { ["ntp.example.com"] }

it "proposes to use it by default" do
expect(Yast::LanItems).to receive(:dhcp_ntp_servers).and_return("eth0" => ntp_servers)
expect(::Installation::Widgets::NtpServer).to receive(:new)
.with(ntp_servers).and_call_original
subject.run
end
end

context "no NTP server set in DHCP and default NTP is enabled in control.xml" do
before do
allow(Yast::ProductFeatures).to receive(:GetBooleanFeature)
.with("globals", "default_ntp_setup").and_return(true)
allow(Yast::Product).to receive(:FindBaseProducts)
.and_return(["name" => "openSUSE-Tumbleweed-Kubic"])
end

it "proposes to use a random openSUSE pool server" do
expect(::Installation::Widgets::NtpServer).to receive(:new).and_wrap_original do |original, arg|
expect(arg.first).to match(/\A[0-3]\.opensuse\.pool\.ntp\.org\z/)
original.call(arg)
end
subject.run
end
end
end
end
Loading