fix(gcu): stop injecting phantom admin_status moves in patch sorter

[rookie-who]

Problem Statement

RequiredValueMoveExtender in the GCU patch sorter injects admin_status=down moves even when the original patch does not include any admin_status changes. This causes the sorter to produce operations the caller never requested.

How we got here

While investigating flaky configlet.test_add_rack.test_add_rack failures on Cisco-8102-C64 and Mellanox SN4600C/SN4700 platforms (build 20251110.26), we traced the root cause to the patch sorter producing a phantom admin_status round-trip:

1. The test generates a removal patch that removes BGP, buffer, queue, and PFC config for a T0 neighbor, but does not include admin_status=down for the decommissioned port
2. The sorter's RequiredValueMoveExtender injects admin_status=down because YANG requires the port to be down before removing buffer/queue config
3. After all buffer/queue removals complete, LowLevelMoveGenerator sees that the current state (admin_status=down) differs from the target (admin_status=up, since the patch didn't touch it) and generates a restore admin_status=up
4. CONFIG_DB ends with admin_status=up for a port with no operational config

A manual config interface shutdown workaround ran after the patch, but on Cisco/Mellanox platforms with dynamic buffer management, buffermgrd raced with the manual shutdown and reset port state, causing the DB comparison to fail.

Design discussion

We considered several options:

Option A — Filter out-of-scope moves after sorting: Remove any sorted move that touches paths not in the original patch. Problem: the intermediate admin_status=down is required for YANG validation of buffer removals — can't simply filter it out.

Option B — Caller responsibility: Don't change the sorter. Require callers to always produce YANG-complete patches. If you're removing buffers, include admin_status=down.

Chosen approach — Hybrid: Stop the sorter from injecting phantom operations when the patch doesn't touch the required path, while preserving the ordering protection when the patch does include admin_status changes. If a patch is YANG-incomplete (removes buffers without setting admin_status=down), the sorter will report "no valid ordering" instead of silently inventing operations.

The principle: GCU should be a faithful executor — it sorts and applies what the patch specifies, nothing more. If YANG requires admin_status=down before removing buffer config, the patch itself must include that operation.

Historical context

RequiredValueMoveExtender was added in PR #1998 (Feb 2022, fixing issue #1930) for the AddRack scenario. In AddRack, the patch explicitly includes admin_status=up (adding a new port). The extender ensures:

• Port-critical config (buffers, queues) is applied while the port is admin down
• admin_status=up is applied last, after all critical config is in place

This injection was correct for AddRack (patch includes admin_status), but also fired in RemoveRack (patch has no admin_status op), creating the phantom down/up round-trip.

Fix

Modified RequiredValueMoveExtender.extend() to only inject admin_status=down moves when the original patch actually modifies admin_status (current != target for the required path). If current == target for admin_status, the patch didn't touch it and no injection occurs.

Preserved: The flip protection behavior — when the patch DOES include admin_status changes, the extender still prevents premature admin_status=up while port-critical changes remain. This ensures the AddRack ordering guarantee is maintained.

Companion PR

• fix(configlet): include admin_status=down in removal patch for decommissioned ports sonic-mgmt#24657 / fix(configlet): include admin_status=down in removal patch for decommissioned ports sonic-mgmt#24658 — fixes the test_add_rack test to include admin_status=down in the removal patch, making the patch YANG-complete

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).