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

[rust] improvements in agama logs store #1762

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
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: 1 addition & 5 deletions rust/agama-cli/src/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
// find current contact information at www.suse.com.

use agama_lib::base_http_client::BaseHTTPClient;
use agama_lib::logs::set_archive_permissions;
use agama_lib::manager::http_client::ManagerHTTPClient as HTTPClient;
use clap::Subcommand;
use std::io;
Expand Down Expand Up @@ -51,10 +50,7 @@ pub async fn run(client: BaseHTTPClient, subcommand: LogsCommands) -> anyhow::Re
let result = client
.store(dst_file.as_path())
.await
.map_err(|_| anyhow::Error::msg("Downloading of logs failed"))?;

set_archive_permissions(result.clone())
.map_err(|_| anyhow::Error::msg("Cannot store the logs"))?;
.map_err(|e| anyhow::Error::new(e))?;

println!("{}", result.clone().display());

Expand Down
12 changes: 10 additions & 2 deletions rust/agama-lib/src/base_http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,19 @@ impl BaseHTTPClient {
/// Returns raw reqwest::Response. Use e.g. in case when response content is not
/// JSON body but e.g. binary data
pub async fn get_raw(&self, path: &str) -> Result<Response, ServiceError> {
self.client
let raw: Result<_, ServiceError> = self
.client
.get(self.url(path))
.send()
.await
.map_err(|e| e.into())
.map_err(|e| e.into());
let response = raw?;

if response.status().is_success() {
Ok(response)
} else {
Err(self.build_backend_error(response).await)
}
}

/// POST/PUT/PATCH an object to a given path and returns server response.
Expand Down
39 changes: 31 additions & 8 deletions rust/agama-lib/src/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,30 +32,32 @@ use std::process::Command;
use tempfile::TempDir;
use utoipa::ToSchema;

const DEFAULT_COMMANDS: [(&str, &str); 3] = [
const DEFAULT_COMMANDS: [(&str, &str); 5] = [
// (<command to be executed>, <file name used for storing result of the command>)
("journalctl -u agama", "agama"),
("journalctl -u agama-auto", "agama-auto"),
("journalctl -u agama-web-server", "agama-web-server"),
("journalctl --dmesg", "dmesg"),
("rpm -qa", "rpm-qa"),
];

const DEFAULT_PATHS: [&str; 14] = [
// logs
"/var/log/YaST2",
"/var/log/zypper.log",
"/var/log/zypper/history*",
"/var/log/zypper/pk_backend_zypp",
"/var/log/pbl.log",
"/var/log/linuxrc.log",
"/var/log/wickedd.log",
"/var/log/NetworkManager",
"/var/log/messages",
"/var/log/boot.msg",
"/var/log/udev.log",
"/run/agama/dbus.log",
// config
"/etc/install.inf",
"/etc/os-release",
"/linuxrc.config",
"/.packages.root",
];

const DEFAULT_RESULT: &str = "/run/agama/agama-logs";
Expand Down Expand Up @@ -143,7 +145,22 @@ impl LogItem for LogPath {
let options = CopyOptions::new();
// fs_extra's own Error doesn't implement From trait so ? operator is unusable
match copy_items(&[self.src_path.as_str()], dst_path, &options) {
Ok(_p) => Ok(()),
Ok(_p) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

One match and three nested "ifs" look too much to me :-) What about something like:

copy_items(&[self.src_path.as_str()], dst_path, &options)
    .map_err(|_e| io::Error::new(io::ErrorKind::Other, "Copying of a file failed"))?;

if let Some(file_name) = dst_file.file_name().and_then(|fname| fname.to_str()) {
    if file_name.starts_with(".") {
        let name = file_name.trim_start_matches(".");
        let _ = fs::rename(dst_file.clone(), dst_file.with_file_name(name));
    }
}

Ok(())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're suppressing my creativity ;-)

I'll look into it ...

// turns "invisible" files to visible ones (.packages.root -> packages.root)
if let Some(fname) = dst_file.file_name() {
if let Some(name) = fname.to_str().and_then(|n| {
if n.starts_with(".") {
Some(n.trim_start_matches("."))
} else {
None
}
}) {
let _ = fs::rename(dst_file.clone(), dst_file.with_file_name(name));
}
}

Ok(())
}
Err(_e) => Err(io::Error::new(
io::ErrorKind::Other,
"Copying of a file failed",
Expand Down Expand Up @@ -172,11 +189,17 @@ impl LogItem for LogCmd {
let output = Command::new(cmd_parts[0])
.args(cmd_parts[1..].iter())
.output()?;
let mut file_stdout = File::create(format!("{}.out.log", file_path.display()))?;
let mut file_stderr = File::create(format!("{}.err.log", file_path.display()))?;

file_stdout.write_all(&output.stdout)?;
file_stderr.write_all(&output.stderr)?;
if output.stdout.len() > 0 {
let mut file_stdout = File::create(format!("{}.out.log", file_path.display()))?;

file_stdout.write_all(&output.stdout)?;
}
if output.stderr.len() > 0 {
let mut file_stderr = File::create(format!("{}.err.log", file_path.display()))?;

file_stderr.write_all(&output.stderr)?;
}

Ok(())
}
Expand Down
14 changes: 14 additions & 0 deletions rust/package/agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
-------------------------------------------------------------------
Sun Nov 24 17:05:54 UTC 2024 - Michal Filka <[email protected]>

- several improvements for CLI. agama logs store:
- collects logs of agama-web-server service
- collects list of installed packages (e.g. .packages.root or
rpm -qa)
- do not add into the archive empty files when log command
returns nothing
- do not explicitly set archive permissions on client side
- do not hide backend errors (e.g. in case of invalid auth token)
- added service for monitoring Agama's D-Bus bus which stores
errors into a log.

-------------------------------------------------------------------
Thu Nov 14 14:45:47 UTC 2024 - Knut Alejandro Anderssen González <[email protected]>

Expand Down
6 changes: 6 additions & 0 deletions rust/package/agama.spec
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ install -m 0644 %{_builddir}/agama/share/agama.libsonnet %{buildroot}%{_datadir}
install --directory %{buildroot}%{_datadir}/dbus-1/agama-services
install -m 0644 --target-directory=%{buildroot}%{_datadir}/dbus-1/agama-services %{_builddir}/agama/share/org.opensuse.Agama1.service
install -D -m 0644 %{_builddir}/agama/share/agama-web-server.service %{buildroot}%{_unitdir}/agama-web-server.service
install -D -m 0644 %{_builddir}/agama/share/agama-dbus-monitor.service %{buildroot}%{_unitdir}/agama-dbus-monitor.service

# install manpages
mkdir -p %{buildroot}%{_mandir}/man1
Expand All @@ -164,15 +165,19 @@ echo $PATH

%pre
%service_add_pre agama-web-server.service
%service_add_pre agama-dbus-monitor.service

%post
%service_add_post agama-web-server.service
%service_add_post agama-dbus-monitor.service

%preun
%service_del_preun agama-web-server.service
%service_del_preun agama-dbus-monitor.service

%postun
%service_del_postun_with_restart agama-web-server.service
%service_del_postun_with_restart agama-dbus-monitor.service

%files
%doc README.md
Expand All @@ -182,6 +187,7 @@ echo $PATH
%{_datadir}/dbus-1/agama-services
%{_pam_vendordir}/agama
%{_unitdir}/agama-web-server.service
%{_unitdir}/agama-dbus-monitor.service

%files -n agama-cli
%{_bindir}/agama
Expand Down
13 changes: 13 additions & 0 deletions service/share/agama-dbus-monitor.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[Unit]
mchf marked this conversation as resolved.
Show resolved Hide resolved
Description=Agama's D-Bus bus monitor
After=agama.service

[Service]
Type=exec
ExecStart=/usr/bin/busctl --address unix:path=/run/agama/bus --quiet --match type=error monitor
StandardOutput=file:/run/agama/dbus.log
Restart=always
User=root

[Install]
WantedBy=default.target
5 changes: 5 additions & 0 deletions setup-services.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ sudosed() {

# if agama is already running -> stop it
$SUDO systemctl list-unit-files agama.service &>/dev/null && $SUDO systemctl stop agama.service
$SUDO systemctl list-unit-files agama-dbus-monitor.service &>/dev/null && $SUDO systemctl stop agama-dbus-monitor.service
$SUDO systemctl list-unit-files agama-web-server.service &>/dev/null && $SUDO systemctl stop agama-web-server.service

# Ruby services
Expand Down Expand Up @@ -190,6 +191,10 @@ $SUDO cp -v $MYDIR/service/share/dbus.conf /usr/share/dbus-1/agama.conf
$SUDO cp -f agama.pam /usr/lib/pam.d/agama
)

# copy D-Bus monitor service
$SUDO cp -vf $MYDIR/service/share/agama-dbus-monitor.service /usr/lib/systemd/system/agama-dbus-monitor.service
$SUDO chmod 0600 /usr/lib/systemd/system/agama-dbus-monitor.service

# copy the product files
$SUDO mkdir -p /usr/share/agama/products.d
$SUDO cp -f $MYDIR/products.d/*.yaml /usr/share/agama/products.d
Expand Down