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

No insistence on unit_entry ever or service_entry with absent manage_unit #345

Merged
merged 1 commit into from Jun 15, 2023

Conversation

traylenator
Copy link
Contributor

@traylenator traylenator commented May 3, 2023

Pull Request (PR) description

Removing a unit file with systemd::manage_unit with the following:

systemd::manage_unit {'foo.service':
  ensure     => 'absent',
}

This is not possible and a service_entry and unit_entry must be specified as the unit name
is in the service namespace.

Remove insistence for unit_entry and service_name parameter for the absent case.

For the ensure => 'present`` the unit_entryis no longer required since it is perfectly possible to create units without any[Unit]` section.

Also improve failure message when service_entry is not specified for service unit.

@traylenator traylenator added the enhancement New feature or request label May 3, 2023
@Geod24
Copy link

Geod24 commented May 9, 2023

Should we require a unit_entry as well ? All fields in types are optional, and you can have a systemd unit file without a [Unit] entry (e.g. for timers).

@traylenator
Copy link
Contributor Author

Should we require a unit_entry as well ? All fields in types are optional, and you can have a systemd unit file without a [Unit] entry (e.g. for timers).

Agreed:

# cat test.timer  && systemd-analyze verify test.timer && echo 'good'

[Timer]
OnBootSec=10min
OnUnitInactiveSec=1w
Unit=dnf-clean.service

good

I'll make the change.

@traylenator traylenator changed the title No insistence on service_entry with absent manage_unit No insistence on unit_entry ever or service_entry with absent manage_unit May 10, 2023
@traylenator
Copy link
Contributor Author

systemd::manage_unit {'foo.service':
ensure => 'absent',
}

So both:

systemd::manage_unit {'foo.service':
  ensure     => 'absent',
}

and

systemd::manage_unit {'foo.service':
  ensure     => 'present',
  service_entry => {
     ....
  },
}

are now both valid.

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM

When removing a unit file with `systemd::manage_unit` with the
following:

```
systemd::manage_unit {'foo.service':
  ensure     => 'absent',
  unit_entry => {},
}
```

This is not possible and a `service_entry` must be specified as the unit name
is in the service namespace.

Remove insistence the `service_name` for the absent case.
@traylenator traylenator merged commit e3f8e0c into voxpupuli:master Jun 15, 2023
16 of 17 checks passed
@traylenator traylenator deleted the absentfail branch June 15, 2023 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants