From 709d4a9ca3e2f62aef9542dd77fc036a8aa2503b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 23 Aug 2023 11:24:20 +0100 Subject: [PATCH 01/30] Add a document explaining the Agama startup process --- doc/startup.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 doc/startup.md diff --git a/doc/startup.md b/doc/startup.md new file mode 100644 index 0000000000..ddffc15936 --- /dev/null +++ b/doc/startup.md @@ -0,0 +1,40 @@ +# Agama Startup Process + +This document summarizes how Agama's startup process. + +## Overview + +As described in the [README](../README.md#architecture), Agama is composed by a set of D-Bus +services, a web client and a command-line interface. The startup process aims to get those D-Bus +services up and running and make the web UI available. Additionally, the auto-installation procedure +could be started if required by the user. + +The startup process is handled by systemd and D-Bus. The only exception is starting the local +browser in the Agama Live image. + +## Starting the D-Bus Services + +[agama.service](../service/share/agama.service) is responsible for starting up Agama's D-Bus daemon +process using the `agamactl --daemon` command. This process uses a [specific +configuration](../service/share/dbus.conf). + +Once the daemon process is running, each D-Bus service will be automatically activated when +required. The definitions of those services are located in `/usr/share/dbus-1/agama-services`, +although you can find the sources in the [repository](../service/share) +(`org.opensuse.Agama*.service` files). + +## Auto-installation + +If the `agama.auto` option is specified in the kernel command-line, the +[agama-auto.service](../service/share/systemd/agama-auto.service) comes into play. It runs after the +`agama.service` so the D-Bus daemon is ready and the services can be activated as needed. + +## Web UI + +When discussing the web UI, we can distinguish two sides: the server process and the web browser. +Regarding the server, Agama's web UI is implemented as a Cockpit module, so the only requisite is +that the `cockpit.socket` is enabled. Then, you can connect to the UI using the +`https://$SERVER_IP:9090/cockpit/@localhost/agama/index.html`. + +When using Agama Live, a local web browser is automatically started. In the default image, it is +launched using an Icewm startup script. From c1ec99c380cd82455a4204c362b75c10f42ba0a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 23 Aug 2023 11:24:25 +0100 Subject: [PATCH 02/30] Update README * Rely more on systemd commands. --- README.md | 46 +++++++++++++--------------------------------- 1 file changed, 13 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 15f9c4fba0..f1e6add9e6 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,7 @@ communication. ![Architecture](./doc/images/architecture.png) -Agama consists on a set of D-Bus services and a web client (an experimental CLI is also available). The services use YaST-based libraries under the hood, reusing a lot logic already provided by YaST. Currently Agama comes with six separate services, although the list can increase in the future: +Agama consists on a set of D-Bus services, a web client and a command-line interface. The services use YaST-based libraries under the hood, reusing a lot logic already provided by YaST. Currently Agama comes with six separate services, although the list can increase in the future: * Agama service: it is the main service which manages and controls the installation process. * Software service: configures the product and software to install. @@ -136,50 +136,30 @@ Then point your browser to http://localhost:9090/cockpit/@localhost/agama/index. The [setup.sh](./setup.sh) script installs the required dependencies to build and run the project and it also configures the Agama services -and cockpit. It uses `sudo` to install packages and files to system locations. +and Cockpit. It uses `sudo` to install packages and files to system locations. The script is well commented so we refer you to it instead of repeating its steps here. -Alternatively you can run a development server which works as a proxy for -the cockpit server. See more details [in the documentation]( -web/README.md#using-a-development-server). +Regarding the web user interface, alternatively you can run a development +server which works as a proxy for the cockpit server. See more details [in the +documentation]( web/README.md#using-a-development-server). -Another alternative is to run source checkout inside container so system is not -affected by doing testing run beside real actions really done by installer. -See more details [in the documentation](doc/testing_using_container.md). - -* Start the services: - * beware that Agama must run as root (like YaST does) to do - hardware probing, partition the disks, install the software and so on. - * Note that `setup.sh` sets up D-Bus activation so starting manually is - only needed when you prefer to see the log output upfront. +To start or stop Agama D-Bus services at any time, use the `agama` systemd service: ```console -$ cd service -$ sudo bundle exec bin/agama +sudo systemctl start agama ``` -* Check that Agama services are working with a tool like -[busctl](https://www.freedesktop.org/wiki/Software/dbus/) or -[D-Feet](https://wiki.gnome.org/Apps/DFeet) if you prefer a graphical one: - +If something goes wrong, you can use `journalctl` to get Agama logs: ```console -$ busctl --address=unix:path=/run/agama/bus \ - call \ - org.opensuse.Agama1 \ - /org/opensuse/Agama1/Manager \ - org.opensuse.Agama1.Manager \ - CanInstall - -$ busctl --address=unix:path=/run/agama/bus \ - call \ - org.opensuse.Agama.Locale1 \ - /org/opensuse/Agama/Locale1 \ - org.freedesktop.DBus.Properties \ - GetAll s org.opensuse.Agama.Locale1 +sudo journalctl -u agama ``` +Another alternative is to run source checkout inside container so system is not +affected by doing testing run beside real actions really done by installer. +See more details [in the documentation](doc/testing_using_container.md). + ## How to Contribute If you want to contribute to Agama, then please open a pull request or report an issue. You can also have a look to our [road-map](https://github.com/orgs/yast/projects/1/views/1). From 0d606c44a836197f993f4b695d23b1d1f76782f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 23 Aug 2023 11:39:12 +0100 Subject: [PATCH 03/30] Install ppc64-diag in PPC architectures --- service/etc/agama.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/service/etc/agama.yaml b/service/etc/agama.yaml index d160523856..268cd66690 100644 --- a/service/etc/agama.yaml +++ b/service/etc/agama.yaml @@ -159,6 +159,8 @@ ALP-Dolomite: - package: fde-tools # Needed for FDE with TPM, hardcoded here temporarily archs: aarch64, x86_64 - package: libtss2-tcti-device0 # Same than fde-tools + - package: ppc64-diag # Needed for hardware-based installations + archs: ppc optional_packages: null base_product: ALP-Dolomite From 9829f1c3551917ea8cce6a860f553fa722a10559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 23 Aug 2023 11:40:44 +0100 Subject: [PATCH 04/30] Update the changes file --- service/package/rubygem-agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index 89f861fbf7..8118a71c03 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Wed Aug 23 10:39:46 UTC 2023 - Imobach Gonzalez Sosa + +- Install the ppc64-diag package when running on PowerPC (related + to bsc#1206898). + ------------------------------------------------------------------- Mon Aug 21 11:15:50 UTC 2023 - Imobach Gonzalez Sosa From 2d39b2042b8319aa6b8d119534a687482045f165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 23 Aug 2023 13:49:34 +0100 Subject: [PATCH 05/30] Update from doc review --- doc/startup.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/startup.md b/doc/startup.md index ddffc15936..627e203025 100644 --- a/doc/startup.md +++ b/doc/startup.md @@ -32,9 +32,11 @@ If the `agama.auto` option is specified in the kernel command-line, the ## Web UI When discussing the web UI, we can distinguish two sides: the server process and the web browser. -Regarding the server, Agama's web UI is implemented as a Cockpit module, so the only requisite is +Regarding the server, Agama's web UI is implemented as a Cockpit module, so the only requirement is that the `cockpit.socket` is enabled. Then, you can connect to the UI using the `https://$SERVER_IP:9090/cockpit/@localhost/agama/index.html`. When using Agama Live, a local web browser is automatically started. In the default image, it is -launched using an Icewm startup script. +launched using an Icewm startup script[^1]. + +[^1]: Check the `root.tar` file from the [agama-live](https://build.opensuse.org/package/show/systemsmanagement:Agama:Devel/agama-live) sources. From fbf7fb57ad62d332ac19d58380ce78161f98fa69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 23 Aug 2023 14:00:49 +0100 Subject: [PATCH 06/30] Use ppc64 to filter ppc64-diag installation --- service/etc/agama.yaml | 2 +- service/package/rubygem-agama.changes | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/service/etc/agama.yaml b/service/etc/agama.yaml index 268cd66690..a7c330cdde 100644 --- a/service/etc/agama.yaml +++ b/service/etc/agama.yaml @@ -160,7 +160,7 @@ ALP-Dolomite: archs: aarch64, x86_64 - package: libtss2-tcti-device0 # Same than fde-tools - package: ppc64-diag # Needed for hardware-based installations - archs: ppc + archs: ppc64 optional_packages: null base_product: ALP-Dolomite diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index 8118a71c03..292cb806d8 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,7 +1,7 @@ ------------------------------------------------------------------- Wed Aug 23 10:39:46 UTC 2023 - Imobach Gonzalez Sosa -- Install the ppc64-diag package when running on PowerPC (related +- Install the ppc64-diag package when running on ppc64le (related to bsc#1206898). ------------------------------------------------------------------- From b9be75bb3b311b226efd6795903ffb7f63cd439f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 23 Aug 2023 14:29:24 +0100 Subject: [PATCH 07/30] Apply suggestions from doc review Co-authored-by: Martin Vidner --- doc/startup.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/startup.md b/doc/startup.md index 627e203025..ba485977fe 100644 --- a/doc/startup.md +++ b/doc/startup.md @@ -1,10 +1,10 @@ # Agama Startup Process -This document summarizes how Agama's startup process. +This document summarizes Agama's startup process. ## Overview -As described in the [README](../README.md#architecture), Agama is composed by a set of D-Bus +As described in the [README](../README.md#architecture), Agama is composed of a set of D-Bus services, a web client and a command-line interface. The startup process aims to get those D-Bus services up and running and make the web UI available. Additionally, the auto-installation procedure could be started if required by the user. @@ -15,7 +15,7 @@ browser in the Agama Live image. ## Starting the D-Bus Services [agama.service](../service/share/agama.service) is responsible for starting up Agama's D-Bus daemon -process using the `agamactl --daemon` command. This process uses a [specific +process using the `agamactl --daemon` command. This process uses a dedicated bus with a [specific configuration](../service/share/dbus.conf). Once the daemon process is running, each D-Bus service will be automatically activated when From f35db4b0a1d1e54f3bd6b05e31fa6e84c85fafc6 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 21 Aug 2023 08:14:32 +0100 Subject: [PATCH 08/30] Copy proxy config to the target system --- service/lib/agama/network.rb | 3 +++ service/lib/agama/proxy_setup.rb | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/service/lib/agama/network.rb b/service/lib/agama/network.rb index c8c4435a72..23eef44e37 100644 --- a/service/lib/agama/network.rb +++ b/service/lib/agama/network.rb @@ -23,6 +23,7 @@ require "yast" require "yast2/systemd/service" require "y2network/proposal_settings" +require "agama/proxy_setup" Yast.import "Installation" @@ -41,6 +42,8 @@ def initialize(logger) def install copy_files enable_service + + ProxySetup.instance.install end private diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index 9852db68ca..80075c63c8 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -33,6 +33,8 @@ class ProxySetup CMDLINE_PATH = "/proc/cmdline" CMDLINE_MENU_CONF = "/etc/cmdline-menu.conf" + PACKAGES = ["microos-tools"].freeze + CONFIG_PATH = "/etc/sysconfig/proxy" # @return [URI::Generic] attr_accessor :proxy @@ -105,5 +107,21 @@ def write Proxy.Write end + + def install + return unless proxy + + copy_files + add_packages + end + + def add_packages + log.info "Selecting these packages for installation: #{PACKAGES}" + Yast::PackagesProposal.SetResolvables(PROPOSAL_ID, :package, PACKAGES) + end + + def copy_files + FileUtils.cp(CONFIG_PATH, File.join(Yast::Installation.destdir, CONFIG_PATH)) + end end end From 2e4dcadafd0d62dd4b91c40ad887868f921fd1aa Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 21 Aug 2023 12:17:03 +0100 Subject: [PATCH 09/30] Read the current proxy configuration --- service/lib/agama/proxy_setup.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index 80075c63c8..ef8a7a5141 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -109,7 +109,8 @@ def write end def install - return unless proxy + Proxy.Read + return unless Proxy.enabled copy_files add_packages @@ -121,6 +122,7 @@ def add_packages end def copy_files + log.info "Copying proxy configuration to the target system" FileUtils.cp(CONFIG_PATH, File.join(Yast::Installation.destdir, CONFIG_PATH)) end end From 178630106ce942a721ccfd10e9114957d8f96107 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 22 Aug 2023 09:36:31 +0100 Subject: [PATCH 10/30] Make install method public --- service/lib/agama/proxy_setup.rb | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index ef8a7a5141..15b852746d 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -21,6 +21,7 @@ require "yast" require "uri" +require "fileutils" module Agama # This class is responsible of parsing the proxy url from the kernel cmdline or configured @@ -35,6 +36,7 @@ class ProxySetup CMDLINE_MENU_CONF = "/etc/cmdline-menu.conf" PACKAGES = ["microos-tools"].freeze CONFIG_PATH = "/etc/sysconfig/proxy" + PROPOSAL_ID = "network_proposal" # @return [URI::Generic] attr_accessor :proxy @@ -42,6 +44,8 @@ class ProxySetup # Constructor def initialize Yast.import "Proxy" + Yast.import "Installation" + Yast.import "PackagesProposal" end def run @@ -49,6 +53,14 @@ def run write end + def install + Proxy.Read + return unless Proxy.enabled + + copy_files + add_packages + end + private def read @@ -108,14 +120,6 @@ def write Proxy.Write end - def install - Proxy.Read - return unless Proxy.enabled - - copy_files - add_packages - end - def add_packages log.info "Selecting these packages for installation: #{PACKAGES}" Yast::PackagesProposal.SetResolvables(PROPOSAL_ID, :package, PACKAGES) @@ -123,7 +127,7 @@ def add_packages def copy_files log.info "Copying proxy configuration to the target system" - FileUtils.cp(CONFIG_PATH, File.join(Yast::Installation.destdir, CONFIG_PATH)) + ::FileUtils.cp(CONFIG_PATH, File.join(Yast::Installation.destdir, CONFIG_PATH)) end end end From 3a89c8924f50ac728dc8d55734c36168d925040f Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 22 Aug 2023 10:11:59 +0100 Subject: [PATCH 11/30] Added ProxySetup#install unit test --- service/test/agama/proxy_setup_test.rb | 51 +++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/service/test/agama/proxy_setup_test.rb b/service/test/agama/proxy_setup_test.rb index 484541296a..14b0e08b96 100644 --- a/service/test/agama/proxy_setup_test.rb +++ b/service/test/agama/proxy_setup_test.rb @@ -28,15 +28,15 @@ proxy.proxy = nil end + before do + allow(Yast::Proxy).to receive(:Read) + allow(Yast::Proxy).to receive(:Write) + end + describe "#run" do let(:file_content) { "proxy=#{proxy_url}" } let(:proxy_url) { "https://yast:1234@192.168.122.1:3128" } - before do - allow(Yast::Proxy).to receive(:Read) - allow(Yast::Proxy).to receive(:Write) - end - context "when some configuration is given through the kernel command line" do before do allow(proxy).to receive(:proxy_from_cmdline).and_return(URI(proxy_url)) @@ -75,4 +75,45 @@ end end end + + describe "#install" do + let(:config) do + { + "enabled" => false + } + end + + before do + Yast::Proxy.Import(config) + allow(Yast::Installation).to receive(:destdir).and_return("/mnt") + end + + it "reads the current Proxy configuration from the inst-sys" do + expect(Yast::Proxy).to receive(:Read) + + proxy.install + end + + context "when the use of proxy is disabled" do + it "does not copy the configuration to the target system" do + expect(FileUtils).to_not receive(:cp) + proxy.install + end + end + + context "when the use of proxy is enabled" do + let(:config) do + { + "enabled" => true, + "http_proxy" => "http://192.168.122.1:3128" + } + end + + it "copies the configuration to the target system" do + expect(FileUtils).to receive(:cp).with(described_class::CONFIG_PATH, + File.join("/mnt", described_class::CONFIG_PATH)) + proxy.install + end + end + end end From 7ea9c0fc4b71e4209a61893ee3299c71268fc655 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 22 Aug 2023 11:39:45 +0100 Subject: [PATCH 12/30] Ensure the read is done out of the chroot --- service/lib/agama/helpers.rb | 19 +++++++++++++++++++ service/lib/agama/manager.rb | 1 + service/lib/agama/proxy_setup.rb | 12 ++++++++---- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/service/lib/agama/helpers.rb b/service/lib/agama/helpers.rb index e8cd993b11..d60bbbd107 100644 --- a/service/lib/agama/helpers.rb +++ b/service/lib/agama/helpers.rb @@ -45,5 +45,24 @@ def on_target(&block) Yast::WFM.SCRClose(handle) end end + + # Run a block in the local system + # + # @param block [Proc] Block to run on the local system + def on_local(&block) + Yast.import "WFM" + old_handle = Yast::WFM.SCRGetDefault + handle = Yast::WFM.SCROpen("chroot=/:scr", false) + Yast::WFM.SCRSetDefault(handle) + + begin + block.call + rescue StandardError => e + logger.error "Error while running on target tasks: #{e.inspect}" + ensure + Yast::WFM.SCRSetDefault(old_handle) + Yast::WFM.SCRClose(handle) + end + end end end diff --git a/service/lib/agama/manager.rb b/service/lib/agama/manager.rb index 6eea3dba66..187b402c4e 100644 --- a/service/lib/agama/manager.rb +++ b/service/lib/agama/manager.rb @@ -22,6 +22,7 @@ require "yast" require "agama/config" require "agama/network" +require "agama/proxy_setup" require "agama/with_progress" require "agama/installation_phase" require "agama/service_status_recorder" diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index 15b852746d..cdc32a252d 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -22,6 +22,7 @@ require "yast" require "uri" require "fileutils" +require "agama/helpers" module Agama # This class is responsible of parsing the proxy url from the kernel cmdline or configured @@ -31,6 +32,7 @@ class ProxySetup include Singleton include Yast include Logger + include Helpers CMDLINE_PATH = "/proc/cmdline" CMDLINE_MENU_CONF = "/etc/cmdline-menu.conf" @@ -54,11 +56,13 @@ def run end def install - Proxy.Read - return unless Proxy.enabled + on_local do + Proxy.Read + return unless Proxy.enabled - copy_files - add_packages + copy_files + add_packages + end end private From b92528e37a6f26ecfc234c05ff50628c23cd795b Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 23 Aug 2023 11:36:48 +0100 Subject: [PATCH 13/30] Add the needed packages before installing the software --- service/lib/agama/manager.rb | 2 ++ service/lib/agama/proxy_setup.rb | 23 +++++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/service/lib/agama/manager.rb b/service/lib/agama/manager.rb index 187b402c4e..429652d1a4 100644 --- a/service/lib/agama/manager.rb +++ b/service/lib/agama/manager.rb @@ -109,6 +109,8 @@ def install_phase software.propose end + ProxySetup.instance.propose + progress.step("Installing Software") { software.install } on_target do diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index cdc32a252d..46704f47b1 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -48,6 +48,8 @@ def initialize Yast.import "Proxy" Yast.import "Installation" Yast.import "PackagesProposal" + + Proxy.Read end def run @@ -55,13 +57,16 @@ def run write end + def propose + on_target { add_packages } if Proxy.enabled + end + def install - on_local do - Proxy.Read - return unless Proxy.enabled + return unless Proxy.enabled + on_local do copy_files - add_packages + enable_services end end @@ -133,5 +138,15 @@ def copy_files log.info "Copying proxy configuration to the target system" ::FileUtils.cp(CONFIG_PATH, File.join(Yast::Installation.destdir, CONFIG_PATH)) end + + def enable_services + service = Yast2::Systemd::Service.find("setup-systemd-proxy-env") + if service.nil? + logger.error "setup-systemd-proxy-env service was not found" + return + end + + Yast::Execute.on_target!("systemctl", "enable", "setup-systemd-proxy-env.path") + end end end From bab9875d180852bd23a258e41884b844c4955bf8 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 24 Aug 2023 10:25:08 +0100 Subject: [PATCH 14/30] Move the proxy propose before software proposal is done --- service/lib/agama/manager.rb | 10 ++++++++-- service/lib/agama/proxy_setup.rb | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/service/lib/agama/manager.rb b/service/lib/agama/manager.rb index 429652d1a4..dfb2210de3 100644 --- a/service/lib/agama/manager.rb +++ b/service/lib/agama/manager.rb @@ -104,13 +104,12 @@ def install_phase progress.step("Partitioning") do storage.install + proxy.propose # propose software after /mnt is already separated, so it uses proper # target software.propose end - ProxySetup.instance.propose - progress.step("Installing Software") { software.install } on_target do @@ -140,6 +139,13 @@ def software end end + # ProxySetup instance + # + # @return [ProxySetup] + def proxy + ProxySetup.instance + end + # Language manager # # @return [DBus::Clients::Locale] diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index 46704f47b1..4b7e72d993 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -58,7 +58,7 @@ def run end def propose - on_target { add_packages } if Proxy.enabled + add_packages if Proxy.enabled end def install From 6821ba258acb82e44279b6652339e2c4f8e30a46 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 24 Aug 2023 10:55:09 +0100 Subject: [PATCH 15/30] Logger fix and more unit tests --- service/lib/agama/proxy_setup.rb | 4 +++- service/test/agama/manager_test.rb | 10 ++++++++ service/test/agama/proxy_setup_test.rb | 32 ++++++++++++++++++++++---- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index 4b7e72d993..cfb59e26df 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -43,6 +43,8 @@ class ProxySetup # @return [URI::Generic] attr_accessor :proxy + alias_method :logger, :log + # Constructor def initialize Yast.import "Proxy" @@ -142,7 +144,7 @@ def copy_files def enable_services service = Yast2::Systemd::Service.find("setup-systemd-proxy-env") if service.nil? - logger.error "setup-systemd-proxy-env service was not found" + log.error "setup-systemd-proxy-env service was not found" return end diff --git a/service/test/agama/manager_test.rb b/service/test/agama/manager_test.rb index cf57345473..b050600515 100644 --- a/service/test/agama/manager_test.rb +++ b/service/test/agama/manager_test.rb @@ -34,6 +34,9 @@ end let(:config) { Agama::Config.from_file(config_path) } let(:logger) { Logger.new($stdout, level: :warn) } + let(:proxy) do + instance_double(Agama::ProxySetup, propose: nil, install: nil) + end let(:software) do instance_double( @@ -61,6 +64,7 @@ before do allow(Agama::Network).to receive(:new).and_return(network) + allow(Agama::ProxySetup).to receive(:instance).and_return(proxy) allow(Agama::DBus::Clients::Locale).to receive(:new).and_return(locale) allow(Agama::DBus::Clients::Software).to receive(:new).and_return(software) allow(Agama::DBus::Clients::Storage).to receive(:new).and_return(storage) @@ -115,6 +119,12 @@ expect(subject.installation_phase.install?).to eq(true) end + it "calls #propose on proxy and software modules" do + expect(proxy).to receive(:propose) + expect(software).to receive(:propose) + subject.install_phase + end + it "calls #install (or #write) method of each module" do expect(network).to receive(:install) expect(software).to receive(:install) diff --git a/service/test/agama/proxy_setup_test.rb b/service/test/agama/proxy_setup_test.rb index 14b0e08b96..c4246921dc 100644 --- a/service/test/agama/proxy_setup_test.rb +++ b/service/test/agama/proxy_setup_test.rb @@ -76,7 +76,7 @@ end end - describe "#install" do + describe "#propose" do let(:config) do { "enabled" => false @@ -88,10 +88,34 @@ allow(Yast::Installation).to receive(:destdir).and_return("/mnt") end - it "reads the current Proxy configuration from the inst-sys" do - expect(Yast::Proxy).to receive(:Read) + context "when the use of proxy is enabled" do + let(:config) do + { + "enabled" => true, + "http_proxy" => "http://192.168.122.1:3128" + } + end + + it "adds microos-tools package to the set of resolvables" do + expect(Yast::PackagesProposal).to receive(:SetResolvables) do |_, _, packages| + expect(packages).to contain_exactly("microos-tools") + end + + proxy.propose + end + end + end + + describe "#install" do + let(:config) do + { + "enabled" => false + } + end - proxy.install + before do + Yast::Proxy.Import(config) + allow(Yast::Installation).to receive(:destdir).and_return("/mnt") end context "when the use of proxy is disabled" do From 1b91c2444f897fd1fc466fefb7ab2ea5a05851a9 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 24 Aug 2023 13:00:07 +0100 Subject: [PATCH 16/30] Enable service in target --- service/lib/agama/proxy_setup.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/service/lib/agama/proxy_setup.rb b/service/lib/agama/proxy_setup.rb index cfb59e26df..00ae9e3f88 100644 --- a/service/lib/agama/proxy_setup.rb +++ b/service/lib/agama/proxy_setup.rb @@ -66,10 +66,8 @@ def propose def install return unless Proxy.enabled - on_local do - copy_files - enable_services - end + on_local { copy_files } + enable_services end private @@ -148,6 +146,7 @@ def enable_services return end + Yast::Execute.on_target!("systemctl", "enable", "setup-systemd-proxy-env.service") Yast::Execute.on_target!("systemctl", "enable", "setup-systemd-proxy-env.path") end end From 393e0213d8856f6cc27468b57cf413fa281ea870 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 28 Aug 2023 09:01:33 +0100 Subject: [PATCH 17/30] Added changelog --- service/package/rubygem-agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index 292cb806d8..fd7da96943 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Aug 28 07:59:26 UTC 2023 - Knut Anderssen + +- Copy the proxy configuration to the target system when needed + (bsc#1212677, gh#openSUSE/agama#711). + ------------------------------------------------------------------- Wed Aug 23 10:39:46 UTC 2023 - Imobach Gonzalez Sosa From ed125459f35761159e80c5f778120a4f78cbab3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Mon, 28 Aug 2023 10:19:51 +0200 Subject: [PATCH 18/30] weblate-merge-po.yml - fixed PO file validation Each PO file needs to be checked individually otherwise msgfmt complains about duplicate messages. --- .github/workflows/weblate-merge-po.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/weblate-merge-po.yml b/.github/workflows/weblate-merge-po.yml index 8700618681..244e9b2aa9 100644 --- a/.github/workflows/weblate-merge-po.yml +++ b/.github/workflows/weblate-merge-po.yml @@ -65,7 +65,7 @@ jobs: - name: Validate the PO files working-directory: ./agama - run: msgfmt --check-format -o /dev/null web/po/*.po + run: ls web/po/*.po | xargs -n1 msgfmt --check-format -o /dev/null # any changes besides the timestamps in the PO files? - name: Check changes From 82080ec3ad6c38f811d3083dbe5e302ad38ee06b Mon Sep 17 00:00:00 2001 From: YaST Bot Date: Mon, 28 Aug 2023 12:03:32 +0000 Subject: [PATCH 19/30] Update PO files Agama-weblate commit: 066daea6dbbac08530ffd2d74dfac4eddd9ce51d --- web/po/ru.po | 33 +++++++++++++++++++++++++++++++++ web/po/uk.po | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 web/po/ru.po create mode 100644 web/po/uk.po diff --git a/web/po/ru.po b/web/po/ru.po new file mode 100644 index 0000000000..ebfd652be8 --- /dev/null +++ b/web/po/ru.po @@ -0,0 +1,33 @@ +# SOME DESCRIPTIVE TITLE. +# Copyright (C) YEAR SuSE Linux Products GmbH, Nuernberg +# This file is distributed under the same license as the PACKAGE package. +# FIRST AUTHOR , YEAR. +# +msgid "" +msgstr "" +"Project-Id-Version: PACKAGE VERSION\n" +"Report-Msgid-Bugs-To: \n" +"POT-Creation-Date: 2023-07-19 12:50+0000\n" +"PO-Revision-Date: 2023-08-06 21:15+0000\n" +"Last-Translator: Milachew \n" +"Language-Team: Russian \n" +"Language: ru\n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=UTF-8\n" +"Content-Transfer-Encoding: 8bit\n" +"Plural-Forms: nplurals=3; plural=n%10==1 && n%100!=11 ? 0 : n%10>=2 && n" +"%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2;\n" +"X-Generator: Weblate 4.9.1\n" + +#: src/components/core/FileViewer.jsx:67 +msgid "Reading file..." +msgstr "Чтение файла..." + +#: src/components/core/FileViewer.jsx:73 +msgid "Cannot read the file" +msgstr "Невозможно прочитать файл" + +#: src/components/core/FileViewer.jsx:82 +msgid "Close" +msgstr "Закрыть" diff --git a/web/po/uk.po b/web/po/uk.po new file mode 100644 index 0000000000..baa996d551 --- /dev/null +++ b/web/po/uk.po @@ -0,0 +1,33 @@ +# SOME DESCRIPTIVE TITLE. +# Copyright (C) YEAR SuSE Linux Products GmbH, Nuernberg +# This file is distributed under the same license as the PACKAGE package. +# FIRST AUTHOR , YEAR. +# +msgid "" +msgstr "" +"Project-Id-Version: PACKAGE VERSION\n" +"Report-Msgid-Bugs-To: \n" +"POT-Creation-Date: 2023-07-19 12:50+0000\n" +"PO-Revision-Date: 2023-08-06 21:15+0000\n" +"Last-Translator: Milachew \n" +"Language-Team: Ukrainian \n" +"Language: uk\n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=UTF-8\n" +"Content-Transfer-Encoding: 8bit\n" +"Plural-Forms: nplurals=3; plural=n%10==1 && n%100!=11 ? 0 : n%10>=2 && n" +"%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2;\n" +"X-Generator: Weblate 4.9.1\n" + +#: src/components/core/FileViewer.jsx:67 +msgid "Reading file..." +msgstr "Читання файлу..." + +#: src/components/core/FileViewer.jsx:73 +msgid "Cannot read the file" +msgstr "Неможливо прочитати файл" + +#: src/components/core/FileViewer.jsx:82 +msgid "Close" +msgstr "Закрити" From 3e8707efe6a91cc8057d6124a3cd26cb669daf86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 11 Aug 2023 12:33:02 +0100 Subject: [PATCH 20/30] Fix the connect() function error --- rust/agama-lib/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/agama-lib/src/lib.rs b/rust/agama-lib/src/lib.rs index c0ba6866e3..a3c055b55a 100644 --- a/rust/agama-lib/src/lib.rs +++ b/rust/agama-lib/src/lib.rs @@ -50,6 +50,6 @@ pub async fn connection_to(address: &str) -> Result Date: Fri, 11 Aug 2023 12:32:19 +0100 Subject: [PATCH 21/30] Make NetworkSystem generic over the Adapter --- .../src/network/dbus/service.rs | 9 ++-- rust/agama-dbus-server/src/network/system.rs | 48 +++++++------------ 2 files changed, 21 insertions(+), 36 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/service.rs b/rust/agama-dbus-server/src/network/dbus/service.rs index 32513b884e..2b746dc0d0 100644 --- a/rust/agama-dbus-server/src/network/dbus/service.rs +++ b/rust/agama-dbus-server/src/network/dbus/service.rs @@ -1,7 +1,7 @@ //! Network D-Bus service. //! //! This module defines a D-Bus service which exposes Agama's network configuration. -use crate::network::NetworkSystem; +use crate::network::{nm::NetworkManagerAdapter, NetworkSystem}; use agama_lib::connection_to; use std::error::Error; @@ -15,10 +15,11 @@ impl NetworkService { pub async fn start(address: &str) -> Result<(), Box> { const SERVICE_NAME: &str = "org.opensuse.Agama.Network1"; - let connection = connection_to(address).await?; - let mut network = NetworkSystem::from_network_manager(connection.clone()) + let adapter = NetworkManagerAdapter::from_system() .await - .expect("Could not read network state"); + .expect("Could not connect to NetworkManager to read the configuration."); + let connection = connection_to(address).await?; + let mut network = NetworkSystem::new(connection.clone(), adapter); async_std::task::spawn(async move { network diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index d7b06c1f8e..6597f28ea1 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -1,53 +1,36 @@ -use crate::network::{ - dbus::Tree, model::Connection, nm::NetworkManagerAdapter, Action, Adapter, NetworkState, -}; -use agama_lib::error::ServiceError; +use crate::network::{dbus::Tree, model::Connection, Action, Adapter, NetworkState}; use async_std::channel::{unbounded, Receiver, Sender}; use std::error::Error; -/// Represents the network system, wrapping a [NetworkState] and setting up the D-Bus tree. -pub struct NetworkSystem { +/// Represents the network system using holding the state and setting up the D-Bus tree. +pub struct NetworkSystem { /// Network state pub state: NetworkState, /// Side of the channel to send actions. actions_tx: Sender, actions_rx: Receiver, tree: Tree, + /// Adapter to read/write the network state. + adapter: T, } -impl NetworkSystem { - pub fn new(state: NetworkState, conn: zbus::Connection) -> Self { +impl NetworkSystem { + pub fn new(conn: zbus::Connection, adapter: T) -> Self { let (actions_tx, actions_rx) = unbounded(); let tree = Tree::new(conn, actions_tx.clone()); Self { - state, + state: NetworkState::default(), actions_tx, actions_rx, tree, + adapter, } } - /// Reads the network configuration using the NetworkManager adapter. - /// - /// * `conn`: connection where self will be exposed. Another connection will be made internally - /// to talk with NetworkManager (which may be on a different bus even). - pub async fn from_network_manager( - conn: zbus::Connection, - ) -> Result> { - let adapter = NetworkManagerAdapter::from_system() - .await - .expect("Could not connect to NetworkManager to read the configuration."); - let state = adapter.read()?; - Ok(Self::new(state, conn)) - } - - /// Writes the network configuration to NetworkManager. - pub async fn to_network_manager(&mut self) -> Result<(), Box> { - let adapter = NetworkManagerAdapter::from_system() - .await - .expect("Could not connect to NetworkManager to write the changes."); - adapter.write(&self.state)?; - self.state = adapter.read()?; + /// Writes the network configuration. + pub async fn write(&mut self) -> Result<(), Box> { + self.adapter.write(&self.state)?; + self.state = self.adapter.read()?; Ok(()) } @@ -58,7 +41,8 @@ impl NetworkSystem { } /// Populates the D-Bus tree with the known devices and connections. - pub async fn setup(&mut self) -> Result<(), ServiceError> { + pub async fn setup(&mut self) -> Result<(), Box> { + self.state = self.adapter.read()?; self.tree .set_connections(&mut self.state.connections) .await?; @@ -93,7 +77,7 @@ impl NetworkSystem { self.state.remove_connection(&id)?; } Action::Apply => { - self.to_network_manager().await?; + self.write().await?; // TODO: re-creating the tree is kind of brute-force and it sends signals about // adding/removing interfaces. We should add/update/delete objects as needed. self.tree From ea6cf3b4e2d68f169c1d191aaf0b20e085c28218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 25 Aug 2023 12:03:10 +0100 Subject: [PATCH 22/30] Allow to inject a different adapter in the NetworkSystem --- rust/agama-dbus-server/src/main.rs | 4 ++-- rust/agama-dbus-server/src/network.rs | 7 +++++++ rust/agama-dbus-server/src/network/dbus/service.rs | 10 +++++----- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/rust/agama-dbus-server/src/main.rs b/rust/agama-dbus-server/src/main.rs index 6b3914d56c..a84b77c107 100644 --- a/rust/agama-dbus-server/src/main.rs +++ b/rust/agama-dbus-server/src/main.rs @@ -1,4 +1,4 @@ -use agama_dbus_server::{locale, network::NetworkService, questions}; +use agama_dbus_server::{locale, network, questions}; use log::LevelFilter; use std::future::pending; @@ -28,7 +28,7 @@ async fn main() -> Result<(), Box> { log::info!("Started questions interface"); let _conn = locale::start_service(ADDRESS).await?; log::info!("Started locale interface"); - NetworkService::start(ADDRESS).await?; + network::start_service(ADDRESS).await?; log::info!("Started network interface"); // Do other things or go to wait forever diff --git a/rust/agama-dbus-server/src/network.rs b/rust/agama-dbus-server/src/network.rs index 93e8dfdc8d..e7fa1d3673 100644 --- a/rust/agama-dbus-server/src/network.rs +++ b/rust/agama-dbus-server/src/network.rs @@ -51,3 +51,10 @@ pub use adapter::Adapter; pub use dbus::NetworkService; pub use model::NetworkState; pub use system::NetworkSystem; + +pub async fn start_service(address: &str) -> Result<(), Box> { + let adapter = NetworkManagerAdapter::from_system() + .await + .expect("Could not connect to NetworkManager to read the configuration."); + NetworkService::start(address, adapter).await +} diff --git a/rust/agama-dbus-server/src/network/dbus/service.rs b/rust/agama-dbus-server/src/network/dbus/service.rs index 2b746dc0d0..37057882b1 100644 --- a/rust/agama-dbus-server/src/network/dbus/service.rs +++ b/rust/agama-dbus-server/src/network/dbus/service.rs @@ -1,7 +1,7 @@ //! Network D-Bus service. //! //! This module defines a D-Bus service which exposes Agama's network configuration. -use crate::network::{nm::NetworkManagerAdapter, NetworkSystem}; +use crate::network::{Adapter, NetworkSystem}; use agama_lib::connection_to; use std::error::Error; @@ -12,12 +12,12 @@ pub struct NetworkService; impl NetworkService { /// Starts listening and dispatching events on the D-Bus connection. - pub async fn start(address: &str) -> Result<(), Box> { + pub async fn start( + address: &str, + adapter: T, + ) -> Result<(), Box> { const SERVICE_NAME: &str = "org.opensuse.Agama.Network1"; - let adapter = NetworkManagerAdapter::from_system() - .await - .expect("Could not connect to NetworkManager to read the configuration."); let connection = connection_to(address).await?; let mut network = NetworkSystem::new(connection.clone(), adapter); From 2835ca556f0f338d5cf6ce4558d94117d7f61e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 14 Aug 2023 09:44:46 +0100 Subject: [PATCH 23/30] Write agama-dbus-server integration tests --- rust/agama-dbus-server/src/network.rs | 1 + rust/agama-dbus-server/src/network/model.rs | 2 +- rust/agama-dbus-server/tests/common/mod.rs | 49 +++++++++++ rust/agama-dbus-server/tests/network.rs | 95 +++++++++++++++++++++ rust/agama-lib/src/network.rs | 2 +- rust/share/dbus-test.conf | 82 ++++++++++++++++++ 6 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 rust/agama-dbus-server/tests/common/mod.rs create mode 100644 rust/agama-dbus-server/tests/network.rs create mode 100644 rust/share/dbus-test.conf diff --git a/rust/agama-dbus-server/src/network.rs b/rust/agama-dbus-server/src/network.rs index e7fa1d3673..1e6009ccea 100644 --- a/rust/agama-dbus-server/src/network.rs +++ b/rust/agama-dbus-server/src/network.rs @@ -50,6 +50,7 @@ pub use action::Action; pub use adapter::Adapter; pub use dbus::NetworkService; pub use model::NetworkState; +pub use nm::NetworkManagerAdapter; pub use system::NetworkSystem; pub async fn start_service(address: &str) -> Result<(), Box> { diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 34937d20b3..43c6ef25c7 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -13,7 +13,7 @@ use std::{ }; use thiserror::Error; -#[derive(Default)] +#[derive(Default, Clone)] pub struct NetworkState { pub devices: Vec, pub connections: Vec, diff --git a/rust/agama-dbus-server/tests/common/mod.rs b/rust/agama-dbus-server/tests/common/mod.rs new file mode 100644 index 0000000000..9f1849cd68 --- /dev/null +++ b/rust/agama-dbus-server/tests/common/mod.rs @@ -0,0 +1,49 @@ +use std::process::{Child, Command}; +use uuid::Uuid; + +pub struct DBusServer { + child: Option, + pub address: String, +} + +impl DBusServer { + pub fn start_server() -> Self { + let mut server = Self::new(); + server.start(); + println!("Starting the server at {}", &server.address); + server + } + + pub fn new() -> Self { + let uuid = Uuid::new_v4(); + Self { + address: format!("unix:path=/tmp/agama-tests-{uuid}"), + child: None, + } + } + + pub fn start(&mut self) { + let child = Command::new("/usr/bin/dbus-daemon") + .args([ + "--config-file", + "../share/dbus-test.conf", + "--address", + &self.address, + ]) + .spawn() + .expect("to start the testing D-Bus daemon"); + self.child = Some(child); + } + + pub fn stop(&mut self) { + if let Some(mut child) = self.child.take() { + child.kill().unwrap(); + } + } +} + +impl Drop for DBusServer { + fn drop(&mut self) { + self.stop(); + } +} diff --git a/rust/agama-dbus-server/tests/network.rs b/rust/agama-dbus-server/tests/network.rs new file mode 100644 index 0000000000..b79e18cba3 --- /dev/null +++ b/rust/agama-dbus-server/tests/network.rs @@ -0,0 +1,95 @@ +mod common; + +use self::common::DBusServer; +use agama_dbus_server::network::{self, model, Adapter, NetworkService, NetworkState}; +use agama_lib::{ + connection_to, + network::{settings, types::DeviceType, NetworkClient}, +}; +use async_std::test; + +#[derive(Default)] +pub struct NetworkTestAdapter(network::NetworkState); + +impl Adapter for NetworkTestAdapter { + fn read(&self) -> Result> { + Ok(self.0.clone()) + } + + fn write(&self, _network: &network::NetworkState) -> Result<(), Box> { + unimplemented!("Not used in tests"); + } +} + +#[test] +async fn test_read_connections() { + let server = DBusServer::start_server(); + + let device = model::Device { + name: String::from("eth0"), + type_: DeviceType::Ethernet, + }; + let eth0 = model::Connection::new("eth0".to_string(), DeviceType::Ethernet); + let state = NetworkState::new(vec![device], vec![eth0]); + let adapter = NetworkTestAdapter(state); + + // TODO: Find a better way to detect when the server started + let ten_millis = std::time::Duration::from_millis(1000); + std::thread::sleep(ten_millis); + + let _service = NetworkService::start(&server.address, adapter) + .await + .unwrap(); + + let ten_millis = std::time::Duration::from_millis(1000); + std::thread::sleep(ten_millis); + + let connection = connection_to(&server.address).await.unwrap(); + let client = NetworkClient::new(connection.clone()).await.unwrap(); + let conns = client.connections().await.unwrap(); + assert_eq!(conns.len(), 1); + let dbus_eth0 = conns.first().unwrap(); + assert_eq!(dbus_eth0.id, "eth0"); + assert_eq!(dbus_eth0.device_type(), DeviceType::Ethernet); +} + +#[test] +async fn test_add_connection() { + let server = DBusServer::start_server(); + + let state = NetworkState::default(); + let adapter = NetworkTestAdapter(state); + + // TODO: Find a better way to detect when the server started + let ten_millis = std::time::Duration::from_millis(100); + std::thread::sleep(ten_millis); + + let _service = NetworkService::start(&server.address, adapter) + .await + .unwrap(); + + let ten_millis = std::time::Duration::from_millis(1000); + std::thread::sleep(ten_millis); + + let connection = connection_to(&server.address).await.unwrap(); + let client = NetworkClient::new(connection.clone()).await.unwrap(); + + let wlan0 = settings::NetworkConnection { + id: "wlan0".to_string(), + method: Some("auto".to_string()), + wireless: Some(settings::WirelessSettings { + password: "123456".to_string(), + security: "wpa-psk".to_string(), + ssid: "TEST".to_string(), + mode: "infrastructure".to_string(), + }), + ..Default::default() + }; + client.add_or_update_connection(&wlan0).await.unwrap(); + + let conns = client.connections().await.unwrap(); + assert_eq!(conns.len(), 1); + let conn = conns.first().unwrap(); + assert_eq!(conn.id, "wlan0"); + assert_eq!(conn.device_type(), DeviceType::Wireless); +} diff --git a/rust/agama-lib/src/network.rs b/rust/agama-lib/src/network.rs index 11bd289408..0f0d65674a 100644 --- a/rust/agama-lib/src/network.rs +++ b/rust/agama-lib/src/network.rs @@ -2,7 +2,7 @@ mod client; mod proxies; -mod settings; +pub mod settings; mod store; pub mod types; diff --git a/rust/share/dbus-test.conf b/rust/share/dbus-test.conf new file mode 100644 index 0000000000..7cc5329c72 --- /dev/null +++ b/rust/share/dbus-test.conf @@ -0,0 +1,82 @@ + + + + org.opensuse.Agama + + + + + unix:tmpdir=/tmp + + + EXTERNAL + + + + + + + + + + + + + + + + + + + + + + + + + + + + contexts/dbus_contexts + + + + + 1000000000 + 250000000 + 1000000000 + 250000000 + 1000000000 + + 120000 + 240000 + 150000 + 100000 + 10000 + 100000 + 10000 + 50000 + 50000 + 50000 + From 8f8c5d07bc6b15d82f24b26c4118d51fcc87740e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 25 Aug 2023 12:18:39 +0100 Subject: [PATCH 24/30] Document a potential bug in the network service --- rust/agama-lib/src/network/client.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index a849c1b164..8e0fe74351 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -123,6 +123,7 @@ impl<'a> NetworkClient<'a> { self.connections_proxy .add_connection(&conn.id, conn.device_type() as u8) .await?; + // FIXME: this method might not work because the object might time some time to appear. Ok(self.connections_proxy.get_connection(&conn.id).await?) } From 00840d15354c8cb9c99a554a4daa6d9e93263bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 25 Aug 2023 12:49:26 +0100 Subject: [PATCH 25/30] Fix NetworkClient to wait for the connection to appear --- rust/agama-lib/src/network/client.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index 8e0fe74351..cd1d5b11e5 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -123,8 +123,7 @@ impl<'a> NetworkClient<'a> { self.connections_proxy .add_connection(&conn.id, conn.device_type() as u8) .await?; - // FIXME: this method might not work because the object might time some time to appear. - Ok(self.connections_proxy.get_connection(&conn.id).await?) + self.wait_for_connection(&conn.id).await } /// Updates a network connection. @@ -182,4 +181,19 @@ impl<'a> NetworkClient<'a> { proxy.set_password(&wireless.password).await?; Ok(()) } + + async fn wait_for_connection(&self, id: &str) -> Result { + loop { + let mut retries = 0; + match self.connections_proxy.get_connection(&id).await { + Ok(path) => return Ok(path), + Err(e) => { + retries += 1; + if retries > 3 { + return Err(e.into()); + }; + } + } + } + } } From 85ec299dc05fd95887b71492c5afbb88788a8d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 25 Aug 2023 14:17:22 +0100 Subject: [PATCH 26/30] Use a signal to detect when a connection is added --- .../src/network/dbus/interfaces.rs | 10 ++++- .../src/network/dbus/tree.rs | 23 +++++++++- rust/agama-dbus-server/src/network/system.rs | 2 +- rust/agama-lib/src/network/client.rs | 43 ++++++++++++------- rust/agama-lib/src/network/proxies.rs | 4 ++ 5 files changed, 63 insertions(+), 19 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 6af95d197f..b8055fa156 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -19,6 +19,7 @@ use std::net::{AddrParseError, Ipv4Addr}; use zbus::{ dbus_interface, zvariant::{ObjectPath, OwnedObjectPath}, + SignalContext, }; /// D-Bus interface for the network devices collection @@ -126,7 +127,7 @@ impl Connections { pub async fn add_connection(&mut self, id: String, ty: u8) -> zbus::fdo::Result<()> { let actions = self.actions.lock().await; actions - .send(Action::AddConnection(id, ty.try_into()?)) + .send(Action::AddConnection(id.clone(), ty.try_into()?)) .await .unwrap(); Ok(()) @@ -163,6 +164,13 @@ impl Connections { actions.send(Action::Apply).await.unwrap(); Ok(()) } + + #[dbus_interface(signal)] + pub async fn connection_added( + ctxt: &SignalContext<'_>, + id: &str, + path: &str, + ) -> zbus::Result<()>; } /// D-Bus interface for a network connection diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index 438086b607..9e09c02aa2 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -79,9 +79,15 @@ impl Tree { /// Adds a connection to the D-Bus tree. /// /// * `connection`: connection to add. - pub async fn add_connection(&self, conn: &mut Connection) -> Result<(), ServiceError> { + /// * `notify`: whether to notify the added connection + pub async fn add_connection( + &self, + conn: &mut Connection, + notify: bool, + ) -> Result<(), ServiceError> { let mut objects = self.objects.lock().await; + let orig_id = conn.id().to_owned(); let (id, path) = objects.register_connection(conn); if id != conn.id() { conn.set_id(&id) @@ -106,6 +112,10 @@ impl Tree { .await?; } + if notify { + self.notify_connection_added(&orig_id, &path).await?; + } + Ok(()) } @@ -127,7 +137,7 @@ impl Tree { /// * `connections`: list of connections. async fn add_connections(&self, connections: &mut [Connection]) -> Result<(), ServiceError> { for conn in connections.iter_mut() { - self.add_connection(conn).await?; + self.add_connection(conn, false).await?; } self.add_interface( @@ -182,6 +192,15 @@ impl Tree { let object_server = self.connection.object_server(); Ok(object_server.at(path, iface).await?) } + + /// Notify that a new connection has been added + async fn notify_connection_added(&self, id: &str, path: &str) -> Result<(), ServiceError> { + let object_server = self.connection.object_server(); + let iface_ref = object_server + .interface::<_, interfaces::Connections>(CONNECTIONS_PATH) + .await?; + Ok(interfaces::Connections::connection_added(iface_ref.signal_context(), id, path).await?) + } } /// Objects paths for known devices and connections diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index 6597f28ea1..5a1fa40879 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -66,7 +66,7 @@ impl NetworkSystem { match action { Action::AddConnection(name, ty) => { let mut conn = Connection::new(name, ty); - self.tree.add_connection(&mut conn).await?; + self.tree.add_connection(&mut conn, true).await?; self.state.add_connection(conn)?; } Action::UpdateConnection(conn) => { diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index cd1d5b11e5..a281355de7 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -3,6 +3,7 @@ use super::types::SSID; use crate::error::ServiceError; use super::proxies::{ConnectionProxy, ConnectionsProxy, IPv4Proxy, WirelessProxy}; +use async_std::stream::StreamExt; use zbus::zvariant::OwnedObjectPath; use zbus::Connection; @@ -120,10 +121,21 @@ impl<'a> NetworkClient<'a> { &self, conn: &NetworkConnection, ) -> Result { + let mut stream = self.connections_proxy.receive_connection_added().await?; + self.connections_proxy .add_connection(&conn.id, conn.device_type() as u8) .await?; - self.wait_for_connection(&conn.id).await + + loop { + let signal = stream.next().await.unwrap(); + let (id, _path): (String, String) = signal.body().unwrap(); + if id == conn.id { + break; + }; + } + + Ok(self.connections_proxy.get_connection(&conn.id).await?) } /// Updates a network connection. @@ -182,18 +194,19 @@ impl<'a> NetworkClient<'a> { Ok(()) } - async fn wait_for_connection(&self, id: &str) -> Result { - loop { - let mut retries = 0; - match self.connections_proxy.get_connection(&id).await { - Ok(path) => return Ok(path), - Err(e) => { - retries += 1; - if retries > 3 { - return Err(e.into()); - }; - } - } - } - } + // Replace with a better implemenation based on signals. + // async fn wait_for_connection(&self, id: &str) -> Result { + // loop { + // let mut retries = 0; + // match self.connections_proxy.get_connection(&id).await { + // Ok(path) => return Ok(path), + // Err(e) => { + // retries += 1; + // if retries > 3 { + // return Err(e.into()); + // }; + // } + // } + // } + // } } diff --git a/rust/agama-lib/src/network/proxies.rs b/rust/agama-lib/src/network/proxies.rs index 5b787ca677..4166abeaa8 100644 --- a/rust/agama-lib/src/network/proxies.rs +++ b/rust/agama-lib/src/network/proxies.rs @@ -28,6 +28,10 @@ trait Connections { /// RemoveConnection method fn remove_connection(&self, uuid: &str) -> zbus::Result<()>; + + /// ConnectionAdded signal + #[dbus_proxy(signal)] + fn connection_added(&self, id: &str, path: &str) -> zbus::Result<()>; } #[dbus_proxy( From f22a960b371f7d4e6928ce2b53e20b54141eef0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 28 Aug 2023 13:23:30 +0100 Subject: [PATCH 27/30] Tests: detect when a service is started --- rust/agama-dbus-server/tests/common/mod.rs | 62 ++++++++++++++++++++-- rust/agama-dbus-server/tests/network.rs | 20 ++----- 2 files changed, 62 insertions(+), 20 deletions(-) diff --git a/rust/agama-dbus-server/tests/common/mod.rs b/rust/agama-dbus-server/tests/common/mod.rs index 9f1849cd68..883f4ade47 100644 --- a/rust/agama-dbus-server/tests/common/mod.rs +++ b/rust/agama-dbus-server/tests/common/mod.rs @@ -1,28 +1,42 @@ +use agama_lib::{connection_to, error::ServiceError}; +use async_std::stream::StreamExt; use std::process::{Child, Command}; use uuid::Uuid; +use zbus::{MatchRule, MessageStream, MessageType}; +/// D-Bus server to be used on tests. +/// +/// This struct takes care of starting, stopping and monitoring dbus-daemon to be used on +/// integration tests. Each server uses a different socket, so they do not collide. pub struct DBusServer { child: Option, + messages: Option, pub address: String, } impl DBusServer { - pub fn start_server() -> Self { + /// Builds and starts a server. + pub async fn start_server() -> Result { let mut server = Self::new(); - server.start(); + server.start().await?; println!("Starting the server at {}", &server.address); - server + Ok(server) } + /// Builds a new server. + /// + /// To start the server, check the `start` function. pub fn new() -> Self { let uuid = Uuid::new_v4(); Self { address: format!("unix:path=/tmp/agama-tests-{uuid}"), child: None, + messages: None, } } - pub fn start(&mut self) { + /// Starts the server. + pub async fn start(&mut self) -> Result<(), ServiceError> { let child = Command::new("/usr/bin/dbus-daemon") .args([ "--config-file", @@ -33,12 +47,52 @@ impl DBusServer { .spawn() .expect("to start the testing D-Bus daemon"); self.child = Some(child); + self.wait(); + self.messages = Some(self.build_message_stream().await?); + Ok(()) } + /// Stops the server. pub fn stop(&mut self) { if let Some(mut child) = self.child.take() { child.kill().unwrap(); } + self.messages = None; + } + + /// Waits for a server to be available. + /// + /// * `name`: service name (e.g., "org.opensuse.Agama.Network1"). + pub async fn wait_for_service(&mut self, name: &str) { + let Some(ref mut messages) = self.messages else { + return; + }; + + loop { + let signal = messages.next().await.unwrap().unwrap(); + let (sname, _, _): (String, String, String) = signal.body().unwrap(); + if &sname == name { + return; + } + } + } + + /// Waits until the D-Bus daemon is ready. + // TODO: implement proper waiting instead of just using a sleep + fn wait(&self) { + let wait_time = std::time::Duration::from_millis(500); + std::thread::sleep(wait_time); + } + + /// Builds a message stream. + async fn build_message_stream(&self) -> Result { + let conn = connection_to(&self.address).await?; + let rule = MatchRule::builder() + .msg_type(MessageType::Signal) + .sender("org.freedesktop.DBus")? + .member("NameOwnerChanged")? + .build(); + Ok(MessageStream::for_match_rule(rule, &conn, None).await?) } } diff --git a/rust/agama-dbus-server/tests/network.rs b/rust/agama-dbus-server/tests/network.rs index b79e18cba3..43d6877aa6 100644 --- a/rust/agama-dbus-server/tests/network.rs +++ b/rust/agama-dbus-server/tests/network.rs @@ -23,7 +23,7 @@ impl Adapter for NetworkTestAdapter { #[test] async fn test_read_connections() { - let server = DBusServer::start_server(); + let mut server = DBusServer::start_server().await.unwrap(); let device = model::Device { name: String::from("eth0"), @@ -33,16 +33,10 @@ async fn test_read_connections() { let state = NetworkState::new(vec![device], vec![eth0]); let adapter = NetworkTestAdapter(state); - // TODO: Find a better way to detect when the server started - let ten_millis = std::time::Duration::from_millis(1000); - std::thread::sleep(ten_millis); - let _service = NetworkService::start(&server.address, adapter) .await .unwrap(); - - let ten_millis = std::time::Duration::from_millis(1000); - std::thread::sleep(ten_millis); + server.wait_for_service("org.opensuse.Agama.Network1").await; let connection = connection_to(&server.address).await.unwrap(); let client = NetworkClient::new(connection.clone()).await.unwrap(); @@ -55,21 +49,15 @@ async fn test_read_connections() { #[test] async fn test_add_connection() { - let server = DBusServer::start_server(); + let mut server = DBusServer::start_server().await.unwrap(); let state = NetworkState::default(); let adapter = NetworkTestAdapter(state); - // TODO: Find a better way to detect when the server started - let ten_millis = std::time::Duration::from_millis(100); - std::thread::sleep(ten_millis); - let _service = NetworkService::start(&server.address, adapter) .await .unwrap(); - - let ten_millis = std::time::Duration::from_millis(1000); - std::thread::sleep(ten_millis); + server.wait_for_service("org.opensuse.Agama.Network1").await; let connection = connection_to(&server.address).await.unwrap(); let client = NetworkClient::new(connection.clone()).await.unwrap(); From c867656b152f6f8497857e51da5a00e48e1c30b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 28 Aug 2023 13:50:38 +0100 Subject: [PATCH 28/30] Run "cargo fmt" happy according to the CI --- rust/agama-dbus-server/src/network/dbus/tree.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index 9e09c02aa2..6878c78a96 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -125,7 +125,7 @@ impl Tree { pub async fn remove_connection(&mut self, id: &str) -> Result<(), ServiceError> { let mut objects = self.objects.lock().await; let Some(path) = objects.connection_path(id) else { - return Ok(()) + return Ok(()); }; self.remove_connection_on(path.as_str()).await?; objects.deregister_connection(id).unwrap(); From 4dd1ad035c7585beea1e8dc612aa481128a8d4a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 28 Aug 2023 14:10:42 +0100 Subject: [PATCH 29/30] Remove commented (and outdated) code --- rust/agama-lib/src/network/client.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index a281355de7..240d0359f0 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -193,20 +193,4 @@ impl<'a> NetworkClient<'a> { proxy.set_password(&wireless.password).await?; Ok(()) } - - // Replace with a better implemenation based on signals. - // async fn wait_for_connection(&self, id: &str) -> Result { - // loop { - // let mut retries = 0; - // match self.connections_proxy.get_connection(&id).await { - // Ok(path) => return Ok(path), - // Err(e) => { - // retries += 1; - // if retries > 3 { - // return Err(e.into()); - // }; - // } - // } - // } - // } } From 601135f8f3da5da27bcdd369a7c745b6c32b6f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 28 Aug 2023 14:26:06 +0100 Subject: [PATCH 30/30] Update from code review --- rust/agama-dbus-server/tests/common/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rust/agama-dbus-server/tests/common/mod.rs b/rust/agama-dbus-server/tests/common/mod.rs index 883f4ade47..2fb8031397 100644 --- a/rust/agama-dbus-server/tests/common/mod.rs +++ b/rust/agama-dbus-server/tests/common/mod.rs @@ -1,6 +1,7 @@ use agama_lib::{connection_to, error::ServiceError}; use async_std::stream::StreamExt; use std::process::{Child, Command}; +use std::time::Duration; use uuid::Uuid; use zbus::{MatchRule, MessageStream, MessageType}; @@ -80,8 +81,8 @@ impl DBusServer { /// Waits until the D-Bus daemon is ready. // TODO: implement proper waiting instead of just using a sleep fn wait(&self) { - let wait_time = std::time::Duration::from_millis(500); - std::thread::sleep(wait_time); + const WAIT_TIME: Duration = Duration::from_millis(500); + std::thread::sleep(WAIT_TIME); } /// Builds a message stream.