Hi James

If "ConditionPathExistsGlob = /var/lib/mdcheck/MD_UUID_*" is not met, the timer
should not run. It's what ConditionPathExistsGlob wants, right?

So it's an expected result that the timer fails when /var/lib/mdcheck/MD_UUID_* doesn't exist.

By the way, the MD_UUID_* is left by the last mdcheck. When the raid is too big, it
can't finish the check at a time. So the file MD_UUID_* is left. The mdcheck_continue.service
will do the check from the last interruption place.

From my side, this is not a bug. What's your opinion?

Regards
Xiao

My view is a service or timer should only fail if it had an issue and needs administrative action (a skip should be clean exit). Adding the same condition to the timer allows the timer to be skipped (like the service) and not go into a failed state.

So only items needing administrative action should be reported in the output of:
systemctl list-units --failed

(In reply to James Hartsock from comment #4)
> My view is a service or timer should only fail if it had an issue and needs
> administrative action (a skip should be clean exit). Adding the same
> condition to the timer allows the timer to be skipped (like the service) and
> not go into a failed state.

Hi James

You mean add the ConditionPathExistsGlob = /var/lib/mdcheck/MD_UUID_*
in mdcheck_continuer.timer, right? If the condition doesn't met, so the
mdcheck_continuer.timer doesn't run, so mdcheck_continuer.service doesn't
run too. Right?

I'm not familiar with systemd. Do you know how to skip a service/timer and
the service/timer doesn't go into a failed state? Could you give a patch
for this? 

Regards
Xiao

From my side, this looks like a feature rather a bug. Move this to next release

Yes, adding the same Conditions to .service & .timer.  This way both are skipped and do not end up with the .timer in failed state since it triggers only to fail start the service.

And I suspect customers getting alerts on failed services across their enterprise for this issue would not consider this a feature.

BTW ... I am now using same work-around on RHEL 9

$ rpm -q mdadm
mdadm-4.2-2.el9.x86_64

$ sudo systemctl cat mdcheck_continue.service | grep Cond
ConditionPathExistsGlob = /var/lib/mdcheck/MD_UUID_*

$ sudo systemctl cat mdcheck_continue.timer | tail -n 3
# /etc/systemd/system/mdcheck_continue.timer.d/james.conf
[Unit]
ConditionPathExistsGlob = /var/lib/mdcheck/MD_UUID_*

(In reply to James Hartsock from comment #9)
> BTW ... I am now using same work-around on RHEL 9
> 
> $ rpm -q mdadm
> mdadm-4.2-2.el9.x86_64
> 
> $ sudo systemctl cat mdcheck_continue.service | grep Cond
> ConditionPathExistsGlob = /var/lib/mdcheck/MD_UUID_*
> 
> $ sudo systemctl cat mdcheck_continue.timer | tail -n 3
> # /etc/systemd/system/mdcheck_continue.timer.d/james.conf
> [Unit]
> ConditionPathExistsGlob = /var/lib/mdcheck/MD_UUID_*

There is a problem with this work-around. The mdcheck_continue.timer
is started by mdcheck_start.timer. If we add the check in mdcheck_continue.timer,
the continue timer can't be started. Can it be started again?

(In reply to XiaoNi from comment #10)
> 
> There is a problem with this work-around. The mdcheck_continue.timer
> is started by mdcheck_start.timer. If we add the check in
> mdcheck_continue.timer,
> the continue timer can't be started. Can it be started again?

If the mdcheck_continue.timer can't be started again. The mdcheck action
can't go on checking from the interrupted position.

Thank you for letting me know flaw in the work-around.  On system it is currently in use on has no mdarray so not a condition I faced.

We need to start the mdcheck_continue.timer when start mdcheck_start.timer.
So the only place to add the check is in mdcheck_continue.service.

Do you agree closing this bug?

No, but agree my work-around is flawed.  A service or timer should not go to failed when it should be skipped. It is standard for admin to run and believe action is needed on anything listed in systemctl list-units --failed

That makes sense. But as mentioned, we can't add the condition in mdcheck_continue.timer.
The only place adds the condition is at mdcheck_continue.service. I'll try to find a
way if we can skip mdcheck_continue.service when the condition is not met.

(In reply to James Hartsock from comment #12)
> Thank you for letting me know flaw in the work-around.  On system it is
> currently in use on has no mdarray so not a condition I faced.

Hi James

The mdcheck_start.timer is not enable by default. If your system hasn't
raid, why do you start the mdcheck_start.timer.

Sorry, I do have RAID ... I was on wrong host when I checked before saying that yesterday.

$ cat /proc/mdstat 
Personalities : [raid1] 
md126 : active raid1 sda[1] sdb[0]
      927881216 blocks super external:/md127/0 [2/2] [UU]
      
md127 : inactive sdb[1](S) sda[0](S)
      10402 blocks super external:imsm
       
unused devices: <none>

with mdadm-4.2-8.el8  
When triggered the  mdcheck_continue.timer didn't fail anymore.

[root@storageqe-26 ~]# systemctl status mdcheck_continue.timer
● mdcheck_continue.timer - MD array scrubbing - continuation
   Loaded: loaded (/usr/lib/systemd/system/mdcheck_continue.timer; disabled; vendor preset: disabled)
   Active: inactive (dead) since Fri 2023-06-02 05:39:00 EDT; 24s ago
  Trigger: n/a

Jun 02 05:38:41 storageqe-26.sqe.lab.eng.bos.redhat.com systemd[1]: Started MD array scrubbing - continuation.
Jun 02 05:39:00 storageqe-26.sqe.lab.eng.bos.redhat.com systemd[1]: mdcheck_continue.timer: Succeeded.
Jun 02 05:39:00 storageqe-26.sqe.lab.eng.bos.redhat.com systemd[1]: Stopped MD array scrubbing - continuation.
[root@storageqe-26 ~]# 

[root@storageqe-26 ~]# systemctl list-units --failed | grep mdcheck
[root@storageqe-26 ~]#

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (mdadm bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:7128