Fix: remote transcoders quietly getting dropped from selection

[stronk-dev]

What does this pull request do?

Fixes a bug in split O/T setups, where a Transcoder can get dropped from selection alltogether, without any indication on the O or T side

See #2706 for more details

Specific updates

Seems to have been introduced in bf5397d so a looong time ago

How did you test each of these updates

Currently running the fix in production on the video miner pool. So far no connected transcoder has been dropped from the remoteTranscoders set as of yet

Does this pull request close any open issues?

Fixes #2706

[codecov]

Codecov Report

Merging #2707 (bf9ec61) into master (a5606ca) will increase coverage by 0.01340%.
The diff coverage is 0.00000%.

❗ Current head bf9ec61 differs from pull request most recent head ab553b4. Consider uploading reports for the commit ab553b4 to get more accurate results

@@                 Coverage Diff                 @@
##              master       #2707         +/-   ##
===================================================
+ Coverage   56.33265%   56.34605%   +0.01340%     
===================================================
  Files             88          88                 
  Lines          19131       19130          -1     
===================================================
+ Hits           10777       10779          +2     
+ Misses          7765        7763          -2     
+ Partials         589         588          -1     

Impacted Files	Coverage Δ
core/orchestrator.go	77.65517% <0.00000%> (ø)
server/mediaserver.go	67.56135% <0.00000%> (+0.20503%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5606ca...ab553b4. Read the comment docs.

Impacted Files	Coverage Δ
core/orchestrator.go	77.65517% <0.00000%> (ø)
server/mediaserver.go	67.56135% <0.00000%> (+0.20503%)	⬆️

[cyberj0g]

That's a nasty one, great finding! Before merging, could you please:

• check why test of removeFromRemoteTranscoders doesn't catch that and update it
• rebase cleanly on top of master

[stronk-dev]

Two issues with the TestRemoveFromRemoteTranscoders:

• At the point where it says it is removing the middle element, it is actually removing the final element, for which exists a separate code path
• The bug only drops Transcoders with an index lower than i-1 and the size of the list when testing removal of a middle Transcoder is 3. Thus, when removing the Transcoder at index 1, the Transcoder at index 0 is still safe. The test would have to be done by removing a Transcoder at index 2 or higher where the index is not the last Transcoder in the list

I'll modify the test to reflect these changes

[cyberj0g]

LGTM!