Project

General

Profile

Actions

Bug #16997

closed

arvados-server config-check doesn't sort Containers/InstanceTypes

Added by Ward Vandewege over 3 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Story points:
-
Release relationship:
Auto

Description

This means that running `arvados-server` isn't deterministic:

workbench.su92l:/var/www/arvados-workbench/current/config# arvados-server --version
arvados-server 2.1.0~rc5 (go1.13.4)
workbench.su92l:~# arvados-server config-check
workbench.su92l:~# arvados-server config-check
Your configuration is relying on deprecated entries. Suggest making the following changes.
--- without-deprecated-configs
+++ relying-on-deprecated-configs
@@ -528,26 +528,6 @@
         RAM: 34359738368
         Scratch: 100000000000
         VCPUs: 4
-      Standard_E48_v3:
-        AddedScratch: 0
-        IncludedScratch: 1200000000000
-        Name: Standard_E48_v3
-        Preemptible: false
-        Price: 3.501001
-        ProviderType: Standard_E48_v3
-        RAM: 412316860416
-        Scratch: 1200000000000
-        VCPUs: 48
-      Standard_E48s_v3:
-        AddedScratch: 0
-        IncludedScratch: 768000000000
-        Name: Standard_E48s_v3
-        Preemptible: false
-        Price: 3.501
-        ProviderType: Standard_E48s_v3
-        RAM: 412316860416
-        Scratch: 768000000000
-        VCPUs: 48
       Standard_E4s_v3:
         AddedScratch: 0
         IncludedScratch: 64000000000
@@ -638,6 +618,26 @@
         RAM: 274877906944
         Scratch: 512000000000
         VCPUs: 32
+      Standard_E48_v3:
+        AddedScratch: 0
+        IncludedScratch: 1200000000000
+        Name: Standard_E48_v3
+        Preemptible: false
+        Price: 3.501001
+        ProviderType: Standard_E48_v3
+        RAM: 412316860416
+        Scratch: 1200000000000
+        VCPUs: 48
+      Standard_E48s_v3:
+        AddedScratch: 0
+        IncludedScratch: 768000000000
+        Name: Standard_E48s_v3
+        Preemptible: false
+        Price: 3.501
+        ProviderType: Standard_E48s_v3
+        RAM: 412316860416
+        Scratch: 768000000000
+        VCPUs: 48
       Standard_E64_v3:
         AddedScratch: 0
         IncludedScratch: 1600000000000

Subtasks 1 (1 open0 closed)

Task #17556: Review 16997-sort-config-for-diffIn ProgressTom Clegg04/27/2021Actions
Actions #1

Updated by Ward Vandewege over 3 years ago

  • Description updated (diff)
Actions #2

Updated by Ward Vandewege over 3 years ago

  • Description updated (diff)
Actions #3

Updated by Tom Clegg almost 3 years ago

  • Target version changed from To Be Groomed to 2021-04-28 bughunt sprint
  • Assigned To set to Tom Clegg
Actions #4

Updated by Tom Clegg almost 3 years ago

  • Status changed from New to In Progress
Actions #5

Updated by Tom Clegg almost 3 years ago

I haven't been able to reproduce this yet (even with arvados 2.1.0 + go1.13) or figure out why keys would be printed in non-lexical order, but I added a test, and while investigating I found two different duplicate-warning bugs to fix.
  • config-check calls Load() twice, so "missing/invalid token" warnings were appearing twice
  • config-check had its own way of warning about missing tokens, which wasn't deleted when Load() started warning about them too

16997-sort-config-for-diff @ 727c8c37baa64fe63bec04aacea870ea47a7daf0 -- developer-run-tests: #2440

Actions #6

Updated by Tom Clegg almost 3 years ago

Mystery solved: go-yaml uses a fancy Less() function for sorting that is non-deterministic for keys like {"a32a", "a48a", "a4a"}. Sometimes we get the sort order {a4a < a32a < a48a}, sometimes we get {a32a < a48a < a4a}.

The real solution is to fix go-yaml's Less function again (other sorting bugs have been fixed before and other people have hit this same bug but it hasn't been updated since the version we're using, v2.2.4) ... but in the meantime here's a kludge to work around it.

16997-sort-config-for-diff @ 9a7efddc9cf8118a346c797365027f168a1e49fe -- developer-run-tests: #2441

Actions #7

Updated by Ward Vandewege almost 3 years ago

Tom Clegg wrote:

Mystery solved: go-yaml uses a fancy Less() function for sorting that is non-deterministic for keys like {"a32a", "a48a", "a4a"}. Sometimes we get the sort order {a4a < a32a < a48a}, sometimes we get {a32a < a48a < a4a}.

The real solution is to fix go-yaml's Less function again (other sorting bugs have been fixed before and other people have hit this same bug but it hasn't been updated since the version we're using, v2.2.4) ... but in the meantime here's a kludge to work around it.

16997-sort-config-for-diff @ 9a7efddc9cf8118a346c797365027f168a1e49fe -- developer-run-tests: #2441

The kludge works, LGTM. If you're planning to submit a PR upstream to fix the bug, can you add a comment to our code referencing that PR, with a note to remove the kludge once a fixed version is released upstream?

Actions #8

Updated by Tom Clegg almost 3 years ago

Here's the PR. https://github.com/go-yaml/yaml/pull/733

Here's a branch that adds the same test, but switches to our patched go-yaml instead of the awful kludge.

16997-sort-config-for-diff @ 7eccf2d2d8d45b5b6f10ba6cf123cba1cc90e4e3 -- developer-run-tests: #2442

Actions #9

Updated by Ward Vandewege almost 3 years ago

Tom Clegg wrote:

Here's the PR. https://github.com/go-yaml/yaml/pull/733

Here's a branch that adds the same test, but switches to our patched go-yaml instead of the awful kludge.

16997-sort-config-for-diff @ 7eccf2d2d8d45b5b6f10ba6cf123cba1cc90e4e3 -- developer-run-tests: #2442

LGTM, thanks!

Actions #10

Updated by Peter Amstutz almost 3 years ago

  • Target version changed from 2021-04-28 bughunt sprint to 2021-05-12 sprint
Actions #11

Updated by Tom Clegg almost 3 years ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved
Actions #12

Updated by Tom Clegg almost 3 years ago

  • Target version changed from 2021-05-12 sprint to 2021-04-28 bughunt sprint
Actions #13

Updated by Peter Amstutz over 2 years ago

  • Release set to 41
Actions

Also available in: Atom PDF