cobbler · SchoolGuy · Mar 5, 2022 · Feb 22, 2022 · Feb 22, 2022 · Feb 22, 2022
diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml
@@ -20,7 +20,7 @@ jobs:
       - name: Pull Docker Test Container
         run: docker pull registry.opensuse.org/home/schoolguy/branches/opensuse/templates/images/tumbleweed/containers/opensuse/cobbler-github-ci:latest
       - name: Run previously built Docker Container
-        run: docker run -t -d -v $PWD:/code --name cobbler registry.opensuse.org/home/schoolguy/branches/opensuse/templates/images/tumbleweed/containers/opensuse/cobbler-github-ci:latest
+        run: docker run --cap-add=NET_ADMIN -t -d -v $PWD:/code --name cobbler registry.opensuse.org/home/schoolguy/branches/opensuse/templates/images/tumbleweed/containers/opensuse/cobbler-github-ci:latest
       - name: Setup Cobbler in the Container
         shell: 'script -q -e -c "bash {0}"'
         run: |

diff --git a/cobbler/api.py b/cobbler/api.py
@@ -1382,8 +1382,15 @@ def sync_systems(self, systems: List[str], verbose: bool = False):
         :param systems: List of specified systems that needs to be synced
         :param verbose: If the action should be just logged as needed or (if True) as much verbose as possible.
         """
-        self.log("sync_systems")
-        if not (systems and isinstance(systems, list) and all(isinstance(sys_name, str) for sys_name in systems)):
+        self.logger.info("sync_systems")
+        if not (
+                systems
+                and isinstance(systems, list)
+                and all(isinstance(sys_name, str) for sys_name in systems)
+        ):
+            if len(systems) < 1:
+                self.logger.debug("sync_systems needs at least one system to do something. Bailing out early.")
+                return
             raise TypeError('Systems must be a list of one or more strings.')
         sync_obj = self.get_sync(verbose=verbose)
         sync_obj.run_sync_systems(systems)

diff --git a/cobbler/cobbler_collections/collection.py b/cobbler/cobbler_collections/collection.py
@@ -406,11 +406,13 @@ def add(self, ref, save: bool = False, with_copy: bool = False, with_triggers: b
                     if ref.virt_type == "openvz":
                         ref.netboot_enabled = False
                     self.lite_sync.add_single_system(ref.name)
+                    self.api.sync_systems(systems=[ref.name])
                 elif isinstance(ref, profile.Profile):
                     # we don't need openvz containers to be network bootable
                     if ref.virt_type == "openvz":
                         ref.enable_menu = False
                     self.lite_sync.add_single_profile(ref.name)
+                    self.api.sync_systems(systems=ref.get_children())
                 elif isinstance(ref, distro.Distro):
                     self.lite_sync.add_single_distro(ref.name)
                 elif isinstance(ref, image.Image):

diff --git a/cobbler/items/system.py b/cobbler/items/system.py
@@ -1432,7 +1432,7 @@ def enable_ipxe(self) -> bool:
     @enable_ipxe.setter
     def enable_ipxe(self, enable_ipxe: bool):
         """
-        Sets whether or not the system will use iPXE for booting.
+        Sets whether the system will use iPXE for booting.
 
         :param enable_ipxe: If ipxe should be enabled or not.
         :raises TypeError: In case enable_ipxe is not a boolean.

diff --git a/cobbler/manager.py b/cobbler/manager.py
@@ -30,7 +30,7 @@ class ManagerModule:
 
     These are typically but not necessarily used to manage systemd services.
     Enabling can be done via settings ``manage_*`` (e.g. ``manage_dhcp``) and ``restart_*`` (e.g. ``restart_dhcp``).
-    Different modules could manage the same functionality like dhcp can be managed via isc.py or dnsmasq.py
+    Different modules could manage the same functionality as dhcp can be managed via isc.py or dnsmasq.py
     (compare with ``/etc/cobbler/modules.py``).
     """
 
@@ -63,18 +63,18 @@ def write_configs(self):
         E.g. dhcp manager would write ``/etc/dhcpd.conf`` here
         """
 
-    def restart_service(self):
+    def restart_service(self) -> int:
         """
         Write module specific config files.
         E.g. dhcp manager would write ``/etc/dhcpd.conf`` here
         """
 
     def regen_ethers(self):
         """
-        ISC/BIND doesn't use this. It is there for compability reasons with other managers.
+        ISC/BIND doesn't use this. It is there for compatibility reasons with other managers.
         """
 
-    def sync(self):
+    def sync(self) -> int:
         """
         This syncs the manager's server (systemd service) with it's new config files.
         Basically this restarts the service to apply the changes.

diff --git a/cobbler/modules/managers/isc.py b/cobbler/modules/managers/isc.py
@@ -20,7 +20,7 @@
 Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
 02110-1301  USA
 """
-
+import shutil
 import time
 
 from cobbler import utils
@@ -371,7 +371,8 @@ def restart_dhcp(self, service_name: str) -> int:
 
         :param service_name: The name of the DHCP service.
         """
-        return_code_service_restart = utils.subprocess_call("{} -t -q".format(service_name), shell=False)
+        dhcpd_path = shutil.which(service_name)
+        return_code_service_restart = utils.subprocess_call([dhcpd_path, "-t", "-q"], shell=False)
         if return_code_service_restart != 0:
             self.logger.error("Testing config - {} -t failed".format(service_name))
         return_code_service_restart = utils.service_restart(service_name)

diff --git a/cobbler/utils.py b/cobbler/utils.py
@@ -255,12 +255,18 @@ def service_restart(service_name: str):
     if is_supervisord():
         with ServerProxy('http://localhost:9001/RPC2') as server:
             try:
-                if server.supervisor.getProcessInfo(service_name).get("state") in (10, 20):
+                process_state = server.supervisor.getProcessInfo(service_name).get("state")
+                if process_state in (10, 20):
                     server.supervisor.stopProcess(service_name)
                 if server.supervisor.startProcess(service_name):  # returns a boolean
                     return 0
             except xmlrpc.client.Fault as clientFault:
-                logger.error('Restarting service "%s" failed', service_name, exc_info=clientFault)
+                logger.error(
+                    'Restarting service "%s" failed (supervisord process state was "%s")',
+                    service_name,
+                    process_state,
+                    exc_info=clientFault
+                )
                 return 1
     elif is_systemd():
         restart_command = "systemctl restart %s" % service_name

diff --git a/docker/debs/Debian_10/supervisord/conf.d/slapd.conf b/docker/debs/Debian_10/supervisord/conf.d/slapd.conf
@@ -1,3 +1,4 @@
 [program:slapd]
 command=/usr/sbin/slapd -u ldap -h "ldap:/// ldaps:/// ldapi:///" -f /test_dir/tests/test_data/slapd.conf
 redirect_stderr=true
+autostart=false
diff --git a/docker/debs/Debian_10/supervisord/supervisord.conf b/docker/debs/Debian_10/supervisord/supervisord.conf
@@ -1,6 +1,7 @@
 [supervisord]
 loglevel=debug
 logfile=/var/log/supervisor/supervisor.log
+user=root
 
 [rpcinterface:supervisor]
 supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface

diff --git a/docker/debs/Debian_11/supervisord/conf.d/slapd.conf b/docker/debs/Debian_11/supervisord/conf.d/slapd.conf
@@ -1,3 +1,4 @@
 [program:slapd]
 command=/usr/sbin/slapd -u ldap -h "ldap:/// ldaps:/// ldapi:///" -f /test_dir/tests/test_data/slapd.conf
 redirect_stderr=true
+autostart=false
diff --git a/docker/debs/Debian_11/supervisord/supervisord.conf b/docker/debs/Debian_11/supervisord/supervisord.conf
@@ -1,6 +1,7 @@
 [supervisord]
 loglevel=debug
 logfile=/var/log/supervisor/supervisor.log
+user=root
 
 [rpcinterface:supervisor]
 supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface

diff --git a/docker/debs/build-and-install-debs.sh b/docker/debs/build-and-install-debs.sh
@@ -38,7 +38,7 @@ $EXECUTOR run -ti -v "$PWD/deb-build:/usr/src/cobbler/deb-build" -v "$PWD/tmp:/v
 
 # Launch container and install cobbler
 echo "==> Start container ..."
-$EXECUTOR run -t -d --name cobbler -v "$PWD/deb-build:/usr/src/cobbler/deb-build" "$IMAGE" /bin/bash
+$EXECUTOR run --cap-add=NET_ADMIN -t -d --name cobbler -v "$PWD/deb-build:/usr/src/cobbler/deb-build" "$IMAGE" /bin/bash
 
 echo "==> Install fresh packages ..."
 $EXECUTOR exec -it cobbler bash -c 'dpkg -i deb-build/DEBS/all/cobbler*.deb'
@@ -65,6 +65,8 @@ fi
 
 if $RUN_SYSTEM_TESTS
 then
+    echo "==> Wait 15 sec. because supervisord gets two unkown SIGHUPs"
+    $EXECUTOR exec -t cobbler bash -c 'sleep 15'
     echo "==> Preparing the container for system tests..."
     $EXECUTOR exec --privileged -t cobbler make system-test-env
     echo "==> Running system tests ..."

diff --git a/docker/develop/scripts/setup-supervisor.sh b/docker/develop/scripts/setup-supervisor.sh
@@ -18,5 +18,11 @@ sleep 5
 echo "Load openLDAP database"
 ldapadd -Y EXTERNAL -H ldapi:/// -f /code/docker/develop/openldap/test.ldif
 
+echo "Create DHCPD leases file"
+touch /var/lib/dhcp/db/dhcpd.leases
+
 echo "Show Cobbler version"
 cobbler version
+
+echo "Execute system-test-env"
+make system-test-env
diff --git a/docker/develop/supervisord/conf.d/dhcpd.conf b/docker/develop/supervisord/conf.d/dhcpd.conf
@@ -1,3 +1,5 @@
 [program:dhcpd]
-command=/usr/lib/dhcp/dhcpd -4 start
+command=/usr/sbin/dhcpd -4 -f -cf /etc/dhcpd.conf -lf /var/lib/dhcp/db/dhcpd.leases -user dhcpd -group nogroup pxe
+autostart=false
+autorestart=false
 redirect_stderr=true
diff --git a/docker/rpms/Fedora_34/supervisord/conf.d/slapd.conf b/docker/rpms/Fedora_34/supervisord/conf.d/slapd.conf
@@ -1,3 +1,4 @@
 [program:slapd]
 command=/usr/sbin/slapd -u ldap -h "ldap:/// ldaps:/// ldapi:///" -f /test_dir/tests/test_data/slapd.conf
 redirect_stderr=true
+autostart=false
diff --git a/docker/rpms/Fedora_34/supervisord/supervisord.conf b/docker/rpms/Fedora_34/supervisord/supervisord.conf
@@ -1,6 +1,7 @@
 [supervisord]
 loglevel=debug
 logfile=/var/log/supervisor/supervisor.log
+user=root
 
 [rpcinterface:supervisor]
 supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface

diff --git a/docker/rpms/build-and-install-rpms.sh b/docker/rpms/build-and-install-rpms.sh
@@ -39,7 +39,7 @@ $EXECUTOR run -t -v "$PWD/rpm-build:/usr/src/cobbler/rpm-build" "$IMAGE"
 
 # Launch container and install cobbler
 echo "==> Start container ..."
-$EXECUTOR run -t -d --name cobbler \
+$EXECUTOR run --cap-add=NET_ADMIN -t -d --name cobbler \
     -v "$PWD/rpm-build:/usr/src/cobbler/rpm-build" \
     -v "$PWD/system-tests:/usr/src/cobbler/system-tests" \
     "$IMAGE" /bin/bash
@@ -79,6 +79,8 @@ fi
 
 if $RUN_SYSTEM_TESTS
 then
+    echo "==> Wait 15 sec. because supervisord gets two unkown sighups"
+    $EXECUTOR exec -t cobbler bash -c 'sleep 15'
     echo "==> Preparing the container for system tests..."
     $EXECUTOR exec --privileged -t cobbler make system-test-env
     echo "==> Running system tests ..."

diff --git a/docker/rpms/opensuse_leap/supervisord/conf.d/slapd.conf b/docker/rpms/opensuse_leap/supervisord/conf.d/slapd.conf
@@ -1,3 +1,4 @@
 [program:slapd]
 command=/usr/lib/openldap/start
 redirect_stderr=true
+autostart=false
diff --git a/docker/rpms/opensuse_leap/supervisord/supervisord.conf b/docker/rpms/opensuse_leap/supervisord/supervisord.conf
@@ -1,6 +1,7 @@
 [supervisord]
 loglevel=debug
 logfile=/var/log/supervisor/supervisor.log
+user=root
 
 [rpcinterface:supervisor]
 supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface

diff --git a/docker/rpms/opensuse_tumbleweed/supervisord/conf.d/slapd.conf b/docker/rpms/opensuse_tumbleweed/supervisord/conf.d/slapd.conf
@@ -1,3 +1,4 @@
 [program:slapd]
 command=/usr/lib/openldap/start
 redirect_stderr=true
+autostart=false
diff --git a/docker/rpms/opensuse_tumbleweed/supervisord/supervisord.conf b/docker/rpms/opensuse_tumbleweed/supervisord/supervisord.conf
@@ -1,6 +1,7 @@
 [supervisord]
 loglevel=debug
 logfile=/var/log/supervisor/supervisor.log
+user=root
 
 [rpcinterface:supervisor]
 supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface

diff --git a/system-tests/scripts/bootstrap b/system-tests/scripts/bootstrap
@@ -16,8 +16,11 @@ cat >/etc/cobbler/settings.d/system-tests.settings <<-EOF
 	server: ${server}
 	next_server_v4: ${server}
 	manage_dhcp: true
+	manage_dhcp_v4: true
 	manage_dhcp_v6: false
 	power_management_default_type: 'ipmilan'
 EOF
 
+supervisorctl restart cobblerd
+
 cobbler mkloaders
diff --git a/system-tests/scripts/run-tests b/system-tests/scripts/run-tests
@@ -38,6 +38,7 @@ for test in $(list_selected_tests); do
 	test_dir=${SYSTESTS_TMP}/${test}
 	printf '  %s... ' ${test}
 	mkdir -p ${test_dir}
+	env TEST_NAME=${test} ${topdir}/tests/${test} >${test_dir}/_output 2>&1
 	if [ ${?} -eq 0 ]; then
 		echo ok
 	else

diff --git a/system-tests/tests/basic-system-ipxe-dhcpd-conf-update b/system-tests/tests/basic-system-ipxe-dhcpd-conf-update
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+# Check that Cobbler can add and remove systems
+
+source ${SYSTESTS_PRELUDE} && prepare
+
+set -x -e -o pipefail
+
+# Arrange
+cobbler distro add --name fake --arch x86_64 --kernel ${fake_kernel} \
+	--initrd ${fake_initramfs}
+cobbler profile add --name fake --distro fake
+cobbler system add --name testbed --profile fake --mac "aa:bb:cc:dd:ee:ff"
+
+# Act
+cobbler system edit --name testbed --netboot-enabled 1 --enable-ipxe 1
+
+# Assert
+# We assume that the existence of a single http URL is a successful iPXE group creation.
+# See dhcp.template for reference how we generate this.
+dhcpd_conf=$(_dir /etc/dhcpd.conf /etc/dhcp/dhcpd.conf)
+grep 'filename "http://' $dhcpd_conf
diff --git a/tests/actions/buildiso_test.py b/tests/actions/buildiso_test.py
@@ -279,7 +279,7 @@ def test_generate_system(
         # TODO: Make tests more sophisticated
         assert (
             result
-            == " append initrd=testdistro.img install=http://127.0.0.1:80/cblr/links/testdistro autoyast=default.ks"
+            == " append initrd=testdistro.img install=http://192.168.1.1:80/cblr/links/testdistro autoyast=default.ks"
         )
 
     def test_generate_profile(
@@ -309,5 +309,5 @@ def test_generate_profile(
         # TODO: Make tests more sophisticated
         assert (
             result
-            == " append initrd=testdistro.img install=http://127.0.0.1:80/cblr/links/testdistro autoyast=default.ks"
+            == " append initrd=testdistro.img install=http://192.168.1.1:80/cblr/links/testdistro autoyast=default.ks"
         )
diff --git a/tests/api/sync_test.py b/tests/api/sync_test.py
@@ -107,7 +107,9 @@ def test_get_sync(mocker):
     (None, ["t1.systems.de"], does_not_raise()),
     (True, ["t1.systems.de"], does_not_raise()),
     (False, ["t1.systems.de"], does_not_raise()),
+    (False, [], does_not_raise()),
     (False, [42], pytest.raises(TypeError)),
+    (False, None, pytest.raises(TypeError)),
     (False, "t1.systems.de", pytest.raises(TypeError))
 ])
 def test_sync_systems(input_systems, input_verbose, expected_exception, mocker):
@@ -121,8 +123,11 @@ def test_sync_systems(input_systems, input_verbose, expected_exception, mocker):
         test_api.sync_systems(input_systems, input_verbose)
 
         # Assert
-        stub.run_sync_systems.assert_called_once()
-        stub.run_sync_systems.assert_called_with(input_systems)
+        if len(input_systems) > 0:
+            stub.run_sync_systems.assert_called_once()
+            stub.run_sync_systems.assert_called_with(input_systems)
+        else:
+            assert stub.run_sync_systems.call_count == 0
 
 
 def test_image_rename():

diff --git a/tests/modules/managers/isc_test.py b/tests/modules/managers/isc_test.py
@@ -147,7 +147,7 @@ def test_manager_restart_dhcp(mocker, api_isc_mock):
 
     # Assert
     assert mocked_subprocess.call_count == 1
-    mocked_subprocess.assert_called_with("dhcpd -t -q", shell=False)
+    mocked_subprocess.assert_called_with(["/usr/sbin/dhcpd", "-t", "-q"], shell=False)
     assert mocked_service_restart.call_count == 1
     mocked_service_restart.assert_called_with("dhcpd")
     assert result == 0

diff --git a/tests/utils_test.py b/tests/utils_test.py
@@ -265,7 +265,7 @@ def test_grab_tree():
 
     # Assert
     assert isinstance(result, list)
-    assert result[-1].server == "127.0.0.1"
+    assert result[-1].server == "192.168.1.1"
 
 
 def test_blender():
@@ -739,7 +739,7 @@ def test_local_get_cobbler_api_url():
     result = utils.local_get_cobbler_api_url()
 
     # Assert
-    assert result == "http://127.0.0.1:80/cobbler_api"
+    assert result == "http://192.168.1.1:80/cobbler_api"
 
 
 def test_local_get_cobbler_xmlrpc_url():
@@ -956,8 +956,7 @@ def test_service_restart_supervisord(mocker):
     result = utils.service_restart("dhcpd")
 
     # Assert
-    # Currently we check for 1 because dhcpd is not running.
-    assert result == 1
+    assert result == 0
 
 
 def test_service_restart_systemctl(mocker):