Unverified Commit ea3ca97b authored by r-vdp's avatar r-vdp
Browse files

switch-to-configuration-ng: Dispatch on new section in compare_units

When exactly one section was newly introduced, the code looked up
new_unit.get("Unit") first regardless of which section was actually
new. If [Service] was the new section but the unit also had an
unchanged [Unit] section, the [Service] contents were never inspected
and a newly added ExecStart= would not trigger a restart.

Dispatch on section_cmp instead and add unit tests for both the
restart and reload cases.
parent 1086ea49
Loading
Loading
Loading
Loading
+56 −14
Original line number Diff line number Diff line
@@ -618,9 +618,9 @@ fn compare_units(current_unit: &UnitInfo, new_unit: &UnitInfo) -> UnitComparison

    // A section was introduced that was missing in the previous unit
    if !section_cmp.is_empty() {
        if section_cmp.keys().len() == 1
            && (section_cmp.contains_key("Unit") || section_cmp.contains_key("Service"))
        {
        if section_cmp.keys().len() == 1 {
            // Dispatch on which section is actually new.
            if section_cmp.contains_key("Unit") {
                if let Some(new_unit_unit) = new_unit.get("Unit") {
                    for ini_key in new_unit_unit.keys() {
                        if !unit_section_ignores.contains_key(ini_key.as_str()) {
@@ -629,12 +629,15 @@ fn compare_units(current_unit: &UnitInfo, new_unit: &UnitInfo) -> UnitComparison
                            ret = UnitComparison::UnequalNeedsReload;
                        }
                    }
            } else if let Some(new_unit_service) = new_unit.get("Service") {
                }
            } else if section_cmp.contains_key("Service") {
                if let Some(new_unit_service) = new_unit.get("Service") {
                    if new_unit_service.len() == 1 && new_unit_service.contains_key("ExecReload") {
                        ret = UnitComparison::UnequalNeedsReload;
                    } else {
                        return UnitComparison::UnequalNeedsRestart;
                    }
                }
            } else {
                return UnitComparison::UnequalNeedsRestart;
            }
@@ -2668,6 +2671,45 @@ invalid
                ) == super::UnitComparison::UnequalNeedsReload
            );

            // New [Service] section while [Unit] already existed: must inspect
            // the [Service] section, not the (unchanged) [Unit] one.
            assert!(
                super::compare_units(
                    &HashMap::from([(
                        "Unit".to_string(),
                        HashMap::from([("Description".to_string(), vec!["x".to_string()])])
                    )]),
                    &HashMap::from([
                        (
                            "Unit".to_string(),
                            HashMap::from([("Description".to_string(), vec!["x".to_string()])])
                        ),
                        (
                            "Service".to_string(),
                            HashMap::from([("ExecStart".to_string(), vec!["y".to_string()])])
                        ),
                    ]),
                ) == super::UnitComparison::UnequalNeedsRestart
            );
            assert!(
                super::compare_units(
                    &HashMap::from([(
                        "Unit".to_string(),
                        HashMap::from([("Description".to_string(), vec!["x".to_string()])])
                    )]),
                    &HashMap::from([
                        (
                            "Unit".to_string(),
                            HashMap::from([("Description".to_string(), vec!["x".to_string()])])
                        ),
                        (
                            "Service".to_string(),
                            HashMap::from([("ExecReload".to_string(), vec!["y".to_string()])])
                        ),
                    ]),
                ) == super::UnitComparison::UnequalNeedsReload
            );

            // ExecReload added to an existing [Service] section
            assert!(
                super::compare_units(