[jira] [Updated] (YUNIKORN-3521) TestFailReplacePlaceholder leaks its failing predicate plugin into later scheduler tests

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/YUNIKORN-3521?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

ASF GitHub Bot updated YUNIKORN-3521:
-------------------------------------
    Labels: pull-request-available  (was: )

> TestFailReplacePlaceholder leaks its failing predicate plugin into later scheduler tests
> ----------------------------------------------------------------------------------------
>
>                 Key: YUNIKORN-3521
>                 URL: https://issues.apache.org/jira/browse/YUNIKORN-3521
>             Project: Apache YuniKorn
>          Issue Type: Bug
>          Components: core - scheduler, test - unit
>            Reporter: Dian-Xuan Yang
>            Assignee: Dian-Xuan Yang
>            Priority: Minor
>              Labels: pull-request-available
>
> h3. Problem
> {{TestFailReplacePlaceholder}} in {{pkg/scheduler/partition_test.go}} registers a predicate plugin that marks {{node-1}} as not feasible for a reservation, so the placeholder has to be replaced on {{node-2}}. The deferred "restore" at the end of the test registers a second plugin built with exactly the same arguments:
> {code}
> plugin := mock.NewPredicatePlugin(false, false, map[string]int{nodeID1: 0, nodeID2: 1})
> plugins.RegisterSchedulerPlugin(plugin)
> defer func() {
>     passPlugin := mock.NewPredicatePlugin(false, false, map[string]int{nodeID1: 0, nodeID2: 1})
>     plugins.RegisterSchedulerPlugin(passPlugin)
> }()
> {code}
> The failing plugin therefore stays registered for the rest of the {{pkg/scheduler}} test binary. Nine later tests run against it ({{TestLimitMaxApplications}}, {{TestLimitMaxApplicationsForReservedAllocation}}, {{TestAppSchedulingOrderFIFO}}, {{TestPlaceholderAllocationTracking}}, {{TestUserHeadroom}}, {{TestTryAllocateMaxRunning}}, {{TestPlaceholderAllocationAndReplacementAfterRecovery}}, {{TestReservationTracking}}, {{TestApplicationBackoff}}); two reservation attempts in that run are rejected as "node not feasible". They all still pass, so the effect is hidden today, but any new test placed after {{TestFailReplacePlaceholder}} that reserves or replaces a placeholder on {{node-1}} fails for no visible reason. The test added in apache/yunikorn-core#1192 fails that way as soon as the deferred re-registration is put back.
> h3. Cause
> YUNIKORN-3300 changed the mock constructor. Before that change the deferred plugin was {{mock.NewPredicatePlugin(false, nil)}}, a nil node map that makes every node feasible, so it really was a pass-through. The update copied the failing arguments into the defer as well.
> h3. Proposed fix
> * Replace the deferred re-registration with {{defer plugins.UnregisterSchedulerPlugins()}}, as every test in {{pkg/scheduler/objects}} already does.
> * Register {{mock.NewContainerStateUpdater()}} in {{TestInspectOutstandingRequests}} and assert that the {{FAILED}} scheduling-state update is sent. Today the leaked plugin is the only thing covering the {{UpdateContainerSchedulingState}} branch in {{inspectOutstandingRequests}}; removing the leak alone drops {{pkg/scheduler}} coverage from 80.4% to 80.3%, and the test never checked that the callback is invoked.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org