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.

Sign up for GitHub

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

Jump to bottom

service: SUSE is not based on sysvinit anymore #50396

Merged
merged 1 commit into from
Nov 8, 2018
Merged

service: SUSE is not based on sysvinit anymore #50396

merged 1 commit into from
Nov 8, 2018

Conversation

aplanas
Copy link
Contributor

@aplanas aplanas commented Nov 6, 2018

openSUSE and SLE are systemd based distributions

@cachedout cachedout requested a review from a team November 7, 2018 16:32
brejoc
brejoc suggested changes Nov 8, 2018
View reviewed changes
Copy link
Contributor

@brejoc brejoc left a comment

Choose a reason for hiding this comment

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

I think we can't do that. We've still got SLES11 around and that is afaik not systemd based. So for SLES11 we would fall back to sysvinit and for all the others systemd would be picked -right?

@isbm
Copy link
Contributor

isbm commented Nov 8, 2018

@brejoc but we also not running anything but 2016.11 on SLE11. Why would we stop this in development branch? I think this is good to go.

@brejoc
Copy link
Contributor

brejoc commented Nov 8, 2018

@isbm Ah, good point! Then we are fine!

@cachedout cachedout merged commit 62c3135 into saltstack:develop Nov 8, 2018
@gtmanfred
Copy link
Contributor

This was an unnecessary change.

salt doesn't use the service module if the system is booted with systemd.

https://github.com/aplanas/salt/blob/dd1ec387bb34c72db9a3241c4c345743a75439ef/salt/modules/service.py#L54

so, all this has done is broken systems that are pip installed on suse11, it hasn't fixed anything.

@isbm
Copy link
Contributor

isbm commented Nov 8, 2018 •
edited
Loading

@gtmanfred But pip installed is unnecessary in SUSE world as we have supported packages and strongly recommend to use them. If someone decided to use pip on SLE 11 which is a very bad idea on its own — this stays on their own risk and, of course, this scenario (and all the consequences of it) is unsupported.

That said, this was correct change.

@brejoc
Copy link
Contributor

brejoc commented Nov 9, 2018

While I generally agree, that this change isn't really needed, running anything but our packaged version on SLES11 is really, really hard. SLES11 doesn't even ship with pip and let's not talk about Python versions. So: 🤷

@aplanas
Copy link
Contributor Author

aplanas commented Nov 9, 2018

IMHO the reviews are correct and the merger was also correct. And the change, in some sense, is required. It is used to document a fact, that in 2018 the current SUSE family is systemd based.

But I also agree that there is a bigger problem to solve, that this PR is not doing.

The logic behind the decision of using sysv or systemd is quite error prone. IMHO the list of disable is not a good approach and the comment about the usage is not precise (this is what drives me to do the change). For example, there are names with two spaces there (McAfee OS Server), that I do not know if this is a bug or not. And the delegation of a more modern module to see if this one needs to be loaded or not is a bit unexpected.

@aplanas aplanas deleted the fix_service branch November 9, 2018 10:08
aplanas added a commit to aplanas/salt-1 that referenced this pull request Jan 30, 2019
@aplanas
Backport of:
3ca915b 
saltstack/salt#49668

saltstack/salt#49669

saltstack/salt#49670

saltstack/salt#49803

saltstack/salt#49804

saltstack/salt#50126

saltstack/salt#50175

saltstack/salt#50302

saltstack/salt#50380

saltstack/salt#50396

saltstack/salt#50418

saltstack/salt#50452

saltstack/salt#50473

saltstack/salt#50541

saltstack/salt#50607

saltstack/salt#50635

saltstack/salt#50671

saltstack/salt#50706

saltstack/salt#50725

saltstack/salt#50801

saltstack/salt#50834

saltstack/salt#51074

saltstack/salt#51094

saltstack/salt#51135
@aplanas aplanas mentioned this pull request Jan 30, 2019
Closed
aplanas added a commit to aplanas/salt-1 that referenced this pull request Jan 30, 2019
@aplanas
Backport of:
d4f6435 
* blockdev: fix url from comment
saltstack/salt#49668

* Documentation: fix typo in "equivalent"
saltstack/salt#49669

* states_pt3: fix rST link format
saltstack/salt#49670

* parted: fix _validate_partition_boundary
saltstack/salt#49803

* parted: fix the ordering of list command
saltstack/salt#49804

* Fix lowpkg.diff documentation and parameter name
saltstack/salt#50126

* Add root parameter to useradd, shadow and groupadd
saltstack/salt#50175

* cmd: Add root parameter for wait and run states
saltstack/salt#50302

* systemd: add optional root parameter
saltstack/salt#50380

* service: SUSE is not based on sysvinit anymore
saltstack/salt#50396

* Add new chroot module
saltstack/salt#50418

* Add new module freezer
saltstack/salt#50452

* parted: support variable length output for print
saltstack/salt#50473

* btrfs: add all subvolume commands
saltstack/salt#50541

* file: update attributes for lsattr and chattr
saltstack/salt#50607

* btrfs: add new btrfs state
saltstack/salt#50635

* zypper: demote log from error to warning
saltstack/salt#50671

* blkid: add search by token
saltstack/salt#50706

* mount: add fstab_{present,absent} states
saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
saltstack/salt#50834

* disk: support setting FAT size for format_
saltstack/salt#51074

* cmdmod: add sysfs into the chroot
saltstack/salt#51094

* mount: cache blkid information
saltstack/salt#51135
aplanas added a commit to aplanas/salt-1 that referenced this pull request Feb 4, 2019
@aplanas
Backport of:
6ec4bbf 
* blockdev: fix url from comment
saltstack/salt#49668

* Documentation: fix typo in "equivalent"
saltstack/salt#49669

* states_pt3: fix rST link format
saltstack/salt#49670

* parted: fix _validate_partition_boundary
saltstack/salt#49803

* parted: fix the ordering of list command
saltstack/salt#49804

* Fix lowpkg.diff documentation and parameter name
saltstack/salt#50126

* Add root parameter to useradd, shadow and groupadd
saltstack/salt#50175

* cmd: Add root parameter for wait and run states
saltstack/salt#50302

* systemd: add optional root parameter
saltstack/salt#50380

* service: SUSE is not based on sysvinit anymore
saltstack/salt#50396

* Add new chroot module
saltstack/salt#50418

* Add new module freezer
saltstack/salt#50452

* parted: support variable length output for print
saltstack/salt#50473

* btrfs: add all subvolume commands
saltstack/salt#50541

* file: update attributes for lsattr and chattr
saltstack/salt#50607

* btrfs: add new btrfs state
saltstack/salt#50635

* zypper: demote log from error to warning
saltstack/salt#50671

* blkid: add search by token
saltstack/salt#50706

* mount: add fstab_{present,absent} states
saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
saltstack/salt#50834

* disk: support setting FAT size for format_
saltstack/salt#51074

* cmdmod: add sysfs into the chroot
saltstack/salt#51094

* mount: cache blkid information
saltstack/salt#51135
aplanas added a commit to aplanas/salt-1 that referenced this pull request Feb 4, 2019
@aplanas
Backport of:
1a6a43d 
* blockdev: fix url from comment
saltstack/salt#49668

* Documentation: fix typo in "equivalent"
saltstack/salt#49669

* states_pt3: fix rST link format
saltstack/salt#49670

* parted: fix _validate_partition_boundary
saltstack/salt#49803

* parted: fix the ordering of list command
saltstack/salt#49804

* Fix lowpkg.diff documentation and parameter name
saltstack/salt#50126

* Add root parameter to useradd, shadow and groupadd
saltstack/salt#50175

* cmd: Add root parameter for wait and run states
saltstack/salt#50302

* systemd: add optional root parameter
saltstack/salt#50380

* service: SUSE is not based on sysvinit anymore
saltstack/salt#50396

* Add new chroot module
saltstack/salt#50418

* Add new module freezer
saltstack/salt#50452

* parted: support variable length output for print
saltstack/salt#50473

* btrfs: add all subvolume commands
saltstack/salt#50541

* file: update attributes for lsattr and chattr
saltstack/salt#50607

* btrfs: add new btrfs state
saltstack/salt#50635

* zypper: demote log from error to warning
saltstack/salt#50671

* blkid: add search by token
saltstack/salt#50706

* mount: add fstab_{present,absent} states
saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
saltstack/salt#50834

* disk: support setting FAT size for format_
saltstack/salt#51074

* cmdmod: add sysfs into the chroot
saltstack/salt#51094

* mount: cache blkid information
saltstack/salt#51135
aplanas added a commit to aplanas/salt-1 that referenced this pull request Feb 4, 2019
@aplanas
Backport from develop features required by yomi
215d37b 
* blockdev: fix url from comment
saltstack/salt#49668

* Documentation: fix typo in "equivalent"
saltstack/salt#49669

* states_pt3: fix rST link format
saltstack/salt#49670

* parted: fix _validate_partition_boundary
saltstack/salt#49803

* parted: fix the ordering of list command
saltstack/salt#49804

* Fix lowpkg.diff documentation and parameter name
saltstack/salt#50126

* Add root parameter to useradd, shadow and groupadd
saltstack/salt#50175

* cmd: Add root parameter for wait and run states
saltstack/salt#50302

* systemd: add optional root parameter
saltstack/salt#50380

* service: SUSE is not based on sysvinit anymore
saltstack/salt#50396

* Add new chroot module
saltstack/salt#50418

* Add new module freezer
saltstack/salt#50452

* parted: support variable length output for print
saltstack/salt#50473

* btrfs: add all subvolume commands
saltstack/salt#50541

* file: update attributes for lsattr and chattr
saltstack/salt#50607

* btrfs: add new btrfs state
saltstack/salt#50635

* zypper: demote log from error to warning
saltstack/salt#50671

* blkid: add search by token
saltstack/salt#50706

* mount: add fstab_{present,absent} states
saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
saltstack/salt#50834

* disk: support setting FAT size for format_
saltstack/salt#51074

* cmdmod: add sysfs into the chroot
saltstack/salt#51094

* mount: cache blkid information
saltstack/salt#51135
aplanas added a commit to aplanas/salt-1 that referenced this pull request Feb 26, 2019
@aplanas
Backport from develop features required by yomi
e7ee1f1 
* blockdev: fix url from comment
saltstack/salt#49668

* Documentation: fix typo in "equivalent"
saltstack/salt#49669

* states_pt3: fix rST link format
saltstack/salt#49670

* parted: fix _validate_partition_boundary
saltstack/salt#49803

* parted: fix the ordering of list command
saltstack/salt#49804

* Fix lowpkg.diff documentation and parameter name
saltstack/salt#50126

* Add root parameter to useradd, shadow and groupadd
saltstack/salt#50175

* cmd: Add root parameter for wait and run states
saltstack/salt#50302

* systemd: add optional root parameter
saltstack/salt#50380

* service: SUSE is not based on sysvinit anymore
saltstack/salt#50396

* Add new chroot module
saltstack/salt#50418

* Add new module freezer
saltstack/salt#50452

* parted: support variable length output for print
saltstack/salt#50473

* btrfs: add all subvolume commands
saltstack/salt#50541

* file: update attributes for lsattr and chattr
saltstack/salt#50607

* btrfs: add new btrfs state
saltstack/salt#50635

* zypper: demote log from error to warning
saltstack/salt#50671

* blkid: add search by token
saltstack/salt#50706

* mount: add fstab_{present,absent} states
saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
saltstack/salt#50834

* disk: support setting FAT size for format_
saltstack/salt#51074

* cmdmod: add sysfs into the chroot
saltstack/salt#51094

* mount: cache blkid information
saltstack/salt#51135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brejoc brejoc brejoc approved these changes

@isbm isbm isbm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aplanas @isbm @brejoc @gtmanfred @cachedout