Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions rust/agama-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ async fn install(manager: &ManagerClient<'_>, max_attempts: u8) -> anyhow::Resul
println!("Agama's manager is busy. Waiting until it is ready...");
}

// Make sure that the manager is ready
manager.wait().await?;

if !manager.can_install().await? {
return Err(CliError::ValidationError)?;
}

// Display the progress (if needed) and makes sure that the manager is ready
manager.wait().await?;

let progress = task::spawn(async { show_progress().await });
// Try to start the installation up to max_attempts times.
let mut attempts = 1;
Expand Down
7 changes: 7 additions & 0 deletions service/lib/agama/dbus/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ def busy_services
backend.busy_services
end

# Redefines #service_status to use the one from the backend
#
# @return [DBus::ServiceStatus]
def service_status
backend.service_status
end

private

# @return [Agama::Manager]
Expand Down
15 changes: 15 additions & 0 deletions service/lib/agama/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
require "agama/with_progress"
require "agama/installation_phase"
require "agama/service_status_recorder"
require "agama/dbus/service_status"
require "agama/dbus/clients/locale"
require "agama/dbus/clients/software"
require "agama/dbus/clients/storage"
Expand All @@ -50,6 +51,9 @@ class Manager
# @return [InstallationPhase]
attr_reader :installation_phase

# @return [DBus::ServiceStatus]
attr_reader :service_status

# Constructor
#
# @param logger [Logger]
Expand All @@ -58,18 +62,22 @@ def initialize(config, logger)
@logger = logger
@installation_phase = InstallationPhase.new
@service_status_recorder = ServiceStatusRecorder.new
@service_status = DBus::ServiceStatus.new.busy
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like the manager will only rarely be idle and safe_run will fail more often?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would not say rarely, but this ensures that it waits for startup phase, not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's the point. It starts as busy, but it is set as idle as soon as the startup_phase finishes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok then

end

# Runs the startup phase
def startup_phase
service_status.busy
installation_phase.startup
config_phase if software.selected_product

logger.info("Startup phase done")
service_status.idle
end

# Runs the config phase
def config_phase
service_status.busy
installation_phase.config

start_progress(2)
Expand All @@ -80,11 +88,14 @@ def config_phase
rescue StandardError => e
logger.error "Startup error: #{e.inspect}. Backtrace: #{e.backtrace}"
# TODO: report errors
ensure
service_status.idle
end

# Runs the install phase
# rubocop:disable Metrics/AbcSize
def install_phase
service_status.busy
installation_phase.install
start_progress(7)

Expand All @@ -108,6 +119,10 @@ def install_phase
end

logger.info("Install phase done")
rescue StandardError => e
logger.error "Installation error: #{e.inspect}. Backtrace: #{e.backtrace}"
ensure
service_status.idle
end
# rubocop:enable Metrics/AbcSize

Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Mon Aug 21 11:15:50 UTC 2023 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

- Set the manager service as busy during the startup phase
(bsc#1213194).

-------------------------------------------------------------------
Fri Aug 18 14:17:13 UTC 2023 - Knut Anderssen <kanderssen@suse.com>

Expand Down
16 changes: 9 additions & 7 deletions service/test/agama/dbus/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@
subject { described_class.new(backend, logger) }

let(:logger) { Logger.new($stdout, level: :warn) }
let(:service_status) { Agama::DBus::ServiceStatus.new.idle }

let(:backend) do
instance_double(Agama::Manager,
installation_phase: installation_phase,
software: software_client,
on_services_status_change: nil,
valid?: true)
valid?: true,
service_status: service_status)
end

let(:installation_phase) { Agama::InstallationPhase.new }
Expand Down Expand Up @@ -98,7 +100,7 @@
describe "#config_phase" do
context "when the service is idle" do
before do
subject.service_status.idle
service_status.idle
end

it "runs the config phase, setting the service as busy meanwhile" do
Expand All @@ -112,19 +114,19 @@

context "when the service is busy" do
before do
subject.service_status.busy
service_status.busy
end

it "raises a D-Bus error" do
expect { subject.config_phase }.to raise_error(::DBus::Error)
expect { subject.config_phase }.to raise_error(DBus::Error)
end
end
end

describe "#install_phase" do
context "when the service is idle" do
before do
subject.service_status.idle
service_status.idle
end

it "runs the install phase, setting the service as busy meanwhile" do
Expand All @@ -148,11 +150,11 @@

context "when the service is busy" do
before do
subject.service_status.busy
service_status.busy
end

it "raises a D-Bus error" do
expect { subject.install_phase }.to raise_error(::DBus::Error)
expect { subject.install_phase }.to raise_error(DBus::Error)
end
end
end
Expand Down