Re: [pbs-devel] [PATCH v2 backup stable-2] pbs2to3: add upgrade checker binary

[Fiona Ebner <f.ebner@proxmox.com>]

Am 28.06.23 um 11:42 schrieb Christian Ebner:
> +
> +    fn check_repo_file(
> +        &mut self,
> +        repo_file: APTRepositoryFile,
> +    ) -> Result<(bool, Vec<(String, String)>), Error> {
> +        let mut strange_suite = false;
> +        let mut found_suite: Option<(String, String)> = None;
> +        let mut mismatches = Vec::new();

As already talked off-list, currently only detects mismatches within a file.

> +    fn get_systemd_unit_state(
> +        &mut self,
> +        unit: &str,
> +    ) -> Result<(SystemdUnitState, SystemdUnitState), Error> {
> +        let mut command = std::process::Command::new("systemctl");
> +        command.arg("is-enabled");
> +        command.arg(unit);
> +        let output = command
> +            .output()
> +            .map_err(|err| format_err!("failed to execute {:?} - {}", command, err))?;
> +
> +        let is_enabled_state = match output.stdout.as_slice() {

Nit: I'd prefer enabled_state because it's not a boolean

> +            b"enabled\n" => SystemdUnitState::Enabled,
> +            b"disabled\n" => SystemdUnitState::Disabled,
> +            _ => SystemdUnitState::Unknown,
> +        };
> +
> +        let mut command = std::process::Command::new("systemctl");
> +        command.arg("is-active");
> +        command.arg(unit);
> +        let output = command
> +            .output()
> +            .map_err(|err| format_err!("failed to execute {:?} - {}", command, err))?;
> +
> +        let is_active_state = match output.stdout.as_slice() {

Same

> +            b"active\n" => SystemdUnitState::Active,
> +            b"inactive\n" => SystemdUnitState::Inactive,
> +            b"failed\n" => SystemdUnitState::Failed,
> +            _ => SystemdUnitState::Unknown,
> +        };
> +        Ok((is_enabled_state, is_active_state))
> +    }
> +
> +    fn check_pbs_services(&mut self) -> Result<(), Error> {
> +        self.output.log_info("Checking pbs daemon services..")?;

s/pbs/PBS/

> +
> +        for service in ["proxmox-backup.service", "proxmox-backup-proxy.service"] {
> +            match self.get_systemd_unit_state(service)? {
> +                (_, SystemdUnitState::Active) => {
> +                    self.output.log_pass(
> +                        format!("systemd unit '{}' is in state 'active'", service).as_str(),
> +                    )?;
> +                }
> +                (_, SystemdUnitState::Inactive) => {
> +                    self.output.log_fail(
> +                        format!(
> +                            "systemd unit '{}' is in state 'inactive'\
> +                            \n    Please check the service for errors and start it.",
> +                            service,
> +                        )
> +                        .as_str(),
> +                    )?;
> +                }
> +                (_, SystemdUnitState::Failed) => {
> +                    self.output.log_fail(
> +                        format!(
> +                            "systemd unit '{}' is in state 'failed'\
> +                            \n    Please check the service for errors and start it.",
> +                            service,
> +                        )
> +                        .as_str(),
> +                    )?;
> +                }
> +                (_, _) => {
> +                    self.output.log_fail(
> +                        format!(
> +                            "systemd unit '{}' is not in state 'active'\
> +                            \n    Please check the service for errors and start it.",
> +                            service,
> +                        )
> +                        .as_str(),
> +                    )?;
> +                }
> +            }

The enabled state is ignored? You just look at the active state.

> +        }
> +        Ok(())
> +    }
> +
Other than that, didn't see anything obviously wrong :)