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

retire options support #190

Merged
merged 1 commit into from
Jun 16, 2022
Merged

retire options support #190

merged 1 commit into from
Jun 16, 2022

Conversation

anarcat
Copy link
Contributor

@anarcat anarcat commented Aug 15, 2021

unattended-upgrades does not need special configuration to dpkg
for proper operation. It handles those itself. All this does is
changes the dpkg configuration for other dpkg runs.

This will, for example, affect major upgrades, typically done by hand,
and not unattended-upgrades, which was not designed for that
purpose. In those cases, unsuspecting users will see their old
configuration file versions silently kept in place, which most
definitely will break things on upgrade, rather noisily in the
end.

People unfamiliar with this mechanism will have a hard time figuring
out what has happened, and while recovery is possible, it is tricky
without special tooling. I wrote such a tool here:

https://gitlab.com/anarcat/koumbit-scripts/-/blob/master/vps/clean_conflicts

... but it not well known and definitely not shipped with Debian by
default.

While it is nice that the unattended-upgrades module allows users to
change dpkg options, this can be more easily (and directly) done with
the apt::conf parameter in the first place.

This reverts commit 3a4abe4 and
possibly later ones when the options type was introduced.

This Pull Request (PR) fixes the following issues

Reverts #52.

@anarcat
Copy link
Contributor Author

anarcat commented Aug 15, 2021

one problem with that PR is that it doesn't cleanup after itself: the options code is removed but the 10options file itself is kept in place, so it won't actually have any effect.

A better approach might be to deprecate the setting, and change the default. Then, eventually, that code could be ripped out.

I mostly opened this as a basis for discussion. To really have the changes take effect, something like:

file { '/etc/apt/apt.conf.d/10options':
    ensure => absent,
}

... would be required.

@anarcat anarcat changed the title Revert "add options support" draft: Revert "add options support" Aug 15, 2021
@anarcat anarcat marked this pull request as draft August 15, 2021 18:59
@anarcat anarcat changed the title draft: Revert "add options support" Revert "add options support" Aug 15, 2021
@anarcat anarcat mentioned this pull request Aug 15, 2021
@anarcat
Copy link
Contributor Author

anarcat commented Aug 15, 2021

another alternative here is, of course, to override the default options:

class unattended_upgrades {
      options                => {
        # default in debian is "true", but is implicit
        # puppet module upstream defaults this to true, excplicitely
        'force_confdef'  => false,
        # default in debian not in puppet module upstream
        'force_confold'  => false,
        # default in debian and puppet module upstream
        'force_confnew'  => false,
        'force_confmiss' => false,
      },
}

... and this could be a more minimalist PR to restore proper behavior here. but it has the disadvantage of forcing that policy change on people. this, arguably, would be better than silently stopping to manage a file.

it could be a stepping stone towards deprecating the code as well.

@kenyon
Copy link
Member

kenyon commented Aug 16, 2021

Agreed that this module should not be messing with these dpkg options.

@anarcat
Copy link
Contributor Author

anarcat commented Aug 16, 2021 via email

@kenyon
Copy link
Member

kenyon commented Aug 16, 2021

On 2021-08-15 17:19:16, Kenyon Ralph wrote: Agreed that this module should not be messing with these dpkg options.
Opinions on how best to roll it back?

I haven't looked into it too much, but the revert you have here seems good to me, plus removal of /etc/apt/apt.conf.d/10options.

anarcat added a commit to anarcat/puppet-unattended_upgrades that referenced this pull request Aug 18, 2021
unattended-upgrades does *not* need special configuration to `dpkg`
for proper operation. It handles those itself. All that parameter does
is changes the `dpkg` configuration for *other* `dpkg` runs.

This will, for example, affect major upgrades, typically done by hand,
and not unattended-upgrades, which was *not* designed for that
purpose. In those cases, unsuspecting users will see their old
configuration file versions silently kept in place, which most
definitely *will* break things on upgrade, rather noisily in the end.

People unfamiliar with this mechanism will have a hard time figuring
out what has happened, and while recovery is possible, it is tricky
without special tooling. I wrote such a tool here:

https://gitlab.com/anarcat/koumbit-scripts/-/blob/master/vps/clean_conflicts

... but it not well known and definitely not shipped with Debian by
default.

While it is nice that the unattended-upgrades module allows users to
change dpkg options, this can be more easily (and directly) done with
the `apt::conf` parameter in the first place.

This also marks the feature as deprecated. Once a proper deprecation
delay has passed, the code can be removed (with, for example, PR voxpupuli#190).
@anarcat
Copy link
Contributor Author

anarcat commented Aug 18, 2021

another alternative here is, of course, to override the default options:

i rolled this out alongside a deprecation in #191. please review. this PR can be kept for the future deprecation...

@anarcat anarcat changed the title Revert "add options support" retire options support Aug 18, 2021
anarcat added a commit to anarcat/puppet-unattended_upgrades that referenced this pull request Aug 19, 2021
unattended-upgrades does *not* need special configuration to `dpkg`
for proper operation. It handles those itself. All that parameter does
is changes the `dpkg` configuration for *other* `dpkg` runs.

This will, for example, affect major upgrades, typically done by hand,
and not unattended-upgrades, which was *not* designed for that
purpose. In those cases, unsuspecting users will see their old
configuration file versions silently kept in place, which most
definitely *will* break things on upgrade, rather noisily in the end.

People unfamiliar with this mechanism will have a hard time figuring
out what has happened, and while recovery is possible, it is tricky
without special tooling. I wrote such a tool here:

https://gitlab.com/anarcat/koumbit-scripts/-/blob/master/vps/clean_conflicts

... but it not well known and definitely not shipped with Debian by
default.

While it is nice that the unattended-upgrades module allows users to
change dpkg options, this can be more easily (and directly) done with
the `apt::conf` parameter in the first place.

This also marks the feature as deprecated. Once a proper deprecation
delay has passed, the code can be removed (with, for example, PR voxpupuli#190).
@anarcat
Copy link
Contributor Author

anarcat commented Dec 9, 2021

should i revive this now that the deprecated options has been around for a while?

@kenyon
Copy link
Member

kenyon commented Dec 10, 2021

should i revive this now that the deprecated options has been around for a while?

Yes I think so. It would mean another major release, so it would be good to combine with other breaking changes if possible. At least rebasing and resolving the conflicts at this point would be good, so this can be ready to merge.

@anarcat anarcat marked this pull request as ready for review January 13, 2022 21:18
@anarcat
Copy link
Contributor Author

anarcat commented Jan 13, 2022

i've rebased this and i think it's basically ready to ship as soon as CI is up.

unattended-upgrades does *not* need special configuration to `dpkg`
for proper operation. It handles those itself. All this does is
changes the `dpkg` configuration for *other* `dpkg` runs.

This will, for example, affect major upgrades, typically done by hand,
and not unattended-upgrades, which was *not* designed for that
purpose. In those cases, unsuspecting users will see their old
configuration file versions silently kept in place, which most
definitely *will* break things on upgrade, rather noisily in the
end.

People unfamiliar with this mechanism will have a hard time figuring
out what has happened, and while recovery is possible, it is tricky
without special tooling. I wrote such a tool here:

https://gitlab.com/anarcat/koumbit-scripts/-/blob/master/vps/clean_conflicts

... but it not well known and definitely not shipped with Debian by
default.

While it is nice that the unattended-upgrades module allows users to
change dpkg options, this can be more easily (and directly) done with
the `apt::conf` parameter in the first place.

This reverts commit 3a4abe4 and
possibly later ones when the options type was introduced.
@bastelfreak bastelfreak merged commit 8c7696e into voxpupuli:master Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants