Skip to contentSkip to conversation

containerd: remove redundant systemd detection

PATCH In review Pascal Bach 2 participants 5 messages Mar 14, 2018 – Mar 16, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 containerd: remove redundant systemd detection

The variables only take effect if systemd is enabled so the check can be removed.

---
 recipes-containers/containerd/containerd.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
recipes-containers/containerd/containerd.inc +2 -2
index f91e018..801ace1 100644
@@ -53,8 +53,8 @@ do_compile() {
 
 # Note: disabled for now, since docker is launching containerd
 # inherit systemd
-# SYSTEMD_PACKAGES = "${@bb.utils.contains('DISTRO_FEATURES','systemd','${PN}','',d)}"
-# SYSTEMD_SERVICE_${PN} = "${@bb.utils.contains('DISTRO_FEATURES','systemd','containerd.service','',d)}"
+# SYSTEMD_PACKAGES = "${PN}"
+# SYSTEMD_SERVICE_${PN} = "containerd.service"
 
 do_install() {
     mkdir -p ${D}/${bindir}
Signature · 1 line
2.11.0
#1 lore raw
Quoted 2 lines · Pascal Bach

The variables only take effect if systemd is enabled so the check can be removed.

Same comment.

This layer, and many others, have always had this check since that's how the systemd
classes worked before.

Can you log the oe-core commit that changes the behaviour in the long log of the two
cleanup patches ?

Bruce

Quoted 28 lines
---
 recipes-containers/containerd/containerd.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
recipes-containers/containerd/containerd.inc +2 -3
index f91e018..801ace1 100644
@@ -53,8 +53,8 @@ do_compile() {
 
 # Note: disabled for now, since docker is launching containerd
 # inherit systemd
-# SYSTEMD_PACKAGES = "${@bb.utils.contains('DISTRO_FEATURES','systemd','${PN}','',d)}"
-# SYSTEMD_SERVICE_${PN} = "${@bb.utils.contains('DISTRO_FEATURES','systemd','containerd.service','',d)}"
+# SYSTEMD_PACKAGES = "${PN}"
+# SYSTEMD_SERVICE_${PN} = "containerd.service"
 
 do_install() {
     mkdir -p ${D}/${bindir}
--

2.11.0

--
_______________________________________________
meta-virtualization mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/meta-virtualization

Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"
#2 lore raw
Quoted 6 lines
Quoted 1 line · Pascal Bach

The variables only take effect if systemd is enabled so the check can be

removed.

Quoted 1 line
Quoted 7 lines

Same comment.

This layer, and many others, have always had this check since that's how the systemd classes worked before.

Can you log the oe-core commit that changes the behaviour in the long log of the two cleanup patches ?

I cannot point you to a specific commit in oe-core. But I also don't see anything like this anywhere else.
All recipes in oe-core I had a look at just set these variables, no checks.

So you make any observations that makes the check necessary in this layer but not in oe-core?

Pascal

#3 lore raw
Quoted 11 lines · Pascal
Quoted 11 lines
Quoted 1 line · Pascal Bach

The variables only take effect if systemd is enabled so the check can be

removed.

Quoted 1 line

Same comment.

This layer, and many others, have always had this check since that's how the systemd classes worked before.

Quoted 7 lines
Quoted 2 lines

Can you log the oe-core commit that changes the behaviour in the long log of the two cleanup patches ?

I cannot point you to a specific commit in oe-core. But I also don't see anything like this anywhere else.
All recipes in oe-core I had a look at just set these variables, no checks.

So you make any observations that makes the check necessary in this layer but not in oe-core?

Throughout the various systemd integrations, we've had services that didn't start/install when they should, we've had services that would start when the shouldn't, and all sorts of other combinations.

Plus historically we supported both systemd and sysvinit, so both the packaging and install rules needed to be conditional on the init system.

In those same recipes, the .service files aren't being installed if systemd isn't enabled.

Although I haven't tested it lately, IIRC, that lack of .service file + the packaging being enabled triggers a warning or error.

I'm actually fine in walking away from sysvinit support, but I know there are still some users of it, so I should first document it as depreciated, throw a warning and then remove it completely in the next release (i.e. not just these cleanups, but remove all the systemd init system checks).

Bruce

Quoted 4 lines

Pascal

Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"
#4 lore raw

Hi Bruce

Quoted 23 lines
Quoted 7 lines
Quoted 5 lines

This layer, and many others, have always had this check since that's how the systemd classes worked before.

Can you log the oe-core commit that changes the behaviour in the long log of the two cleanup patches ?

I cannot point you to a specific commit in oe-core. But I also don't see

anything like this anywhere else.

Quoted 3 lines

All recipes in oe-core I had a look at just set these variables, no checks.

So you make any observations that makes the check necessary in this layer

but not in oe-core?

Throughout the various systemd integrations, we've had services that didn't start/install when they should, we've had services that would start when the shouldn't, and all sorts of other combinations.

Plus historically we supported both systemd and sysvinit, so both the packaging and install rules needed to be conditional on the init system.

In those same recipes, the .service files aren't being installed if systemd isn't enabled.

Quoted 7 lines

Although I haven't tested it lately, IIRC, that lack of .service file + the packaging being enabled triggers a warning or error.

I'm actually fine in walking away from sysvinit support, but I know there are still some users of it, so I should first document it as depreciated, throw a warning and then remove it completely in the next release (i.e. not just these cleanups, but remove all the systemd init system checks).

I don't think there is a need to drop sysvinit. They can coexist without problem.
We used many recipes that have both sysvinit and systemd service and never had any issues at least since pyro.

Pascal