Hi Steve,

> Ok, now I see - but that means deb-systemd-helper's auxiliary state will be
> permanently out of sync with the systemd state, I think?

We are purposefully manipulating d-s-h's state here, but it never
becomes out of sync with the systemd state.

> Also, this
> dh_systemd_enable snippet calls deb-systemd-helper enable if 'was-enabled'
> was true, but I don't see that you want to call enable again, as this is
> either a no-op, or it will enable the service which we explicitly are aiming
> to disable.

Exactly! :) We need `was-enabled` to fail or else the timer will get
enabled. This is why we are calling "enable" then "disable" up above. I
will explain via example below.

> In what case do you see incorrect behavior by calling (the more obviously
> named) 'deb-systemd-helper disable'?

If we only call "disable", the total relevant script will look like this

# our call
deb-systemd-helper disable ua-timer.timer >/dev/null || true
# d-s-h snippet
deb-systemd-helper unmask ua-timer.timer >/dev/null || true
if deb-systemd-helper --quiet was-enabled ua-timer.timer; then
        deb-systemd-helper enable ua-timer.timer >/dev/null || true
else
        deb-systemd-helper update-state ua-timer.timer >/dev/null || true
fi

In this case, "was-enabled" will _succeed_, and "deb-systemd-helper enable" in 
the d-s-h snippet will run. This is due to "was-enabled" having an explicit 
special case for the first install: If "deb-systemd-helper enable" has never 
been run before, then "was-enabled" will succeed. They note this in a comment 
in their autosnippet.
So as a result, if we _only_ run "deb-systemd-helper disable", then the timer 
will end up getting enabled.


Now, suppose we adopt the simplified version of what we're doing here 
(https://github.com/canonical/ubuntu-advantage-client/pull/1842/commits/f8b1a7222eeeea21f7b416a2bdd3d08e688cbef7).

Then the total relevant script will be this:

# our calls
deb-systemd-helper enable ua-timer.timer >/dev/null || true
deb-systemd-helper disable ua-timer.timer >/dev/null || true
# d-s-h snippet
deb-systemd-helper unmask ua-timer.timer >/dev/null || true
if deb-systemd-helper --quiet was-enabled ua-timer.timer; then
        deb-systemd-helper enable ua-timer.timer >/dev/null || true
else
        deb-systemd-helper update-state ua-timer.timer >/dev/null || true
fi

In this case, "was-enabled" will _fail_. So the d-s-h snippet will not run the 
"deb-systemd-helper enable" line. "was-enabled" fails here because d-s-h was 
used to enable the timer at least once in the past, but the timer is currently 
disabled.
So as a result, if we run "enable" and then "disable", then the timer will end 
up disabled.

We have tested all of the above. The timer is correctly disabled if we
call "enable" then "disable". The timer is not correctly disabled if we
only call "disable".


If you agree, then we will apply this commit
https://github.com/canonical/ubuntu-advantage-
client/pull/1842/commits/f8b1a7222eeeea21f7b416a2bdd3d08e688cbef7 to
this release to at least make the code here simpler.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1942929

Title:
  [SRU] ubuntu-advantage-tools (27.2.2 -> 27.3) Xenial, Bionic, Focal,
  Hirsute

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/ubuntu-advantage-tools/+bug/1942929/+subscriptions


-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to