Skip to content

merge_tops : throw better error if top is invalid #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

merge_tops : throw better error if top is invalid #15

wants to merge 1 commit into from

Conversation

@zaz
Copy link
Contributor

@zaz zaz commented May 8, 2025

This is a step towards fixing #9950. It doesn't show the file name that caused the error, so I'll look into adding a check higher level, too.

@marmarek
Copy link
Member

marmarek commented May 9, 2025

Full json dump of the top file won't be very readable... maybe limit it somehow (first X chars?). Ideally it should point at the file and line, but that's rather hard to do at this abstraction level...

This was referenced May 9, 2025
Merged
Merged
Merged
Draft
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Open
Merged
@qubesos-bot
Copy link

qubesos-bot commented May 9, 2025 •
edited
Loading

OpenQA test summary

Complete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025061003-4.3&flavor=pull-requests

Test run included the following:

New failures, excluding unstable

Compared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025061004-4.3&flavor=update

  • system_tests_guivm_gui_interactive

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'menu-qubes-tools' mat...

  • system_tests_dispvm

  • system_tests_guivm_vnc_gui_interactive

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'menu-qubes-tools' mat...

  • system_tests_audio

  • system_tests_qwt_win10_seamless@hw13

    • windows_install: wait_serial (wait serial expected)
      # wait_serial expected: qr/0ZKG~-\d+-/...

    • windows_install: Failed (test died + timed out)
      # Test died: command 'script -e -c 'bash -x /usr/bin/qvm-create-win...

  • system_tests_basic_vm_qrexec_gui_btrfs

    • TC_20_NonAudio_whonix-gateway-17-pool: test_000_start_shutdown (error)
      subprocess.CalledProcessError: Command 'qubes.WaitForSession' retur...

  • system_tests_guivm_gpu_gui_interactive@hw13

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'menu-qubes-tools' mat...

  • system_tests_basic_vm_qrexec_gui_ext4

  • system_tests_qwt_win11@hw13

    • windows_install: Failed (test died)
      # Test died: command 'script -e -c 'bash -x /usr/bin/qvm-create-win...

  • system_tests_qwt_win10@hw13

    • windows_install: wait_serial (wait serial expected)
      # wait_serial expected: qr/LDWI1-\d+-/...

    • windows_install: Failed (test died + timed out)
      # Test died: command 'script -e -c 'bash -x /usr/bin/qvm-create-win...

Failed tests

37 failures

  • system_tests_extra

    • TC_00_QVCTest_whonix-workstation-17: test_010_screenshare (failure)
      AssertionError: 1 != 0 : Timeout waiting for /dev/video0 in test-in...

  • system_tests_guivm_gui_interactive

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'menu-qubes-tools' mat...

  • system_tests_dispvm

  • system_tests_kde_gui_interactive

    • gui_keyboard_layout: wait_serial (wait serial expected)
      # wait_serial expected: "echo -e '[Layout]\nLayoutList=us,de' | sud...

    • gui_keyboard_layout: wait_serial (wait serial expected)
      # wait_serial expected: qr/JmOTS-\d+-/...

    • gui_keyboard_layout: Failed (test died + timed out)
      # Test died: command 'echo -e '[Layout]...

    • gui_keyboard_layout: wait_serial (wait serial expected)
      # wait_serial expected: "# "...

    • gui_keyboard_layout: wait_serial (wait serial expected)
      # wait_serial expected: qr/2E8vz-\d+-/...

  • system_tests_guivm_vnc_gui_interactive

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'menu-qubes-tools' mat...

  • system_tests_audio

  • system_tests_qwt_win10_seamless@hw13

    • windows_install: wait_serial (wait serial expected)
      # wait_serial expected: qr/0ZKG~-\d+-/...

    • windows_install: Failed (test died + timed out)
      # Test died: command 'script -e -c 'bash -x /usr/bin/qvm-create-win...

  • system_tests_basic_vm_qrexec_gui_btrfs

    • TC_20_NonAudio_whonix-gateway-17-pool: test_000_start_shutdown (error)
      subprocess.CalledProcessError: Command 'qubes.WaitForSession' retur...

  • system_tests_guivm_gpu_gui_interactive@hw13

    • guivm_manager: unnamed test (unknown)
    • guivm_manager: Failed (test died)
      # Test died: no candidate needle with tag(s) 'menu-qubes-tools' mat...

  • system_tests_basic_vm_qrexec_gui_ext4

  • system_tests_qwt_win11@hw13

    • windows_install: Failed (test died)
      # Test died: command 'script -e -c 'bash -x /usr/bin/qvm-create-win...

  • system_tests_qwt_win10@hw13

    • windows_install: wait_serial (wait serial expected)
      # wait_serial expected: qr/LDWI1-\d+-/...

    • windows_install: Failed (test died + timed out)
      # Test died: command 'script -e -c 'bash -x /usr/bin/qvm-create-win...

Fixed failures

Compared to: https://openqa.qubes-os.org/tests/142375#dependencies

10 fixed

Unstable tests

Performance Tests

Performance degradation:

10 performance degradations
  • debian-12-xfce_exec-data-duplex-root: 81.25 🔺 ( previous job: 70.01, degradation: 116.05%)
  • whonix-workstation-17_exec-data-duplex-root: 104.24 🔺 ( previous job: 86.00, degradation: 121.21%)
  • dom0_root_rnd4k_q1t1_write 3:write_bandwidth_kb: 80.00 :small_red_triangle: ( previous job: 1840.00, degradation: 4.35%)
  • dom0_varlibqubes_seq1m_q8t1_read 3:read_bandwidth_kb: 143625.00 :small_red_triangle: ( previous job: 289182.00, degradation: 49.67%)
  • dom0_varlibqubes_seq1m_q8t1_write 3:write_bandwidth_kb: 106520.00 :small_red_triangle: ( previous job: 122848.00, degradation: 86.71%)
  • dom0_varlibqubes_rnd4k_q32t1_write 3:write_bandwidth_kb: 7336.00 :small_red_triangle: ( previous job: 8874.00, degradation: 82.67%)
  • dom0_varlibqubes_rnd4k_q1t1_write 3:write_bandwidth_kb: 3671.00 :small_red_triangle: ( previous job: 4420.00, degradation: 83.05%)
  • fedora-41-xfce_root_seq1m_q1t1_write 3:write_bandwidth_kb: 52905.00 :small_red_triangle: ( previous job: 75708.00, degradation: 69.88%)
  • fedora-41-xfce_root_rnd4k_q32t1_write 3:write_bandwidth_kb: 2307.00 :small_red_triangle: ( previous job: 3547.00, degradation: 65.04%)
  • fedora-41-xfce_volatile_seq1m_q1t1_write 3:write_bandwidth_kb: 20035.00 :small_red_triangle: ( previous job: 62556.00, degradation: 32.03%)

Remaining performance tests:

62 tests
  • debian-12-xfce_exec: 7.72 🟢 ( previous job: 8.63, improvement: 89.45%)
  • debian-12-xfce_exec-root: 29.08 🟢 ( previous job: 29.44, improvement: 98.78%)
  • debian-12-xfce_socket: 8.75 🔺 ( previous job: 8.50, degradation: 102.91%)
  • debian-12-xfce_socket-root: 7.94 🟢 ( previous job: 8.31, improvement: 95.47%)
  • debian-12-xfce_exec-data-simplex: 63.92 🟢 ( previous job: 65.51, improvement: 97.56%)
  • debian-12-xfce_exec-data-duplex: 74.25 🔺 ( previous job: 73.55, degradation: 100.95%)
  • debian-12-xfce_socket-data-duplex: 162.10 🔺 ( previous job: 161.35, degradation: 100.47%)
  • fedora-41-xfce_exec: 9.13 🟢 ( previous job: 9.30, improvement: 98.11%)
  • fedora-41-xfce_exec-root: 61.28 🔺 ( previous job: 60.59, degradation: 101.13%)
  • fedora-41-xfce_socket: 8.56 🔺 ( previous job: 8.48, degradation: 100.96%)
  • fedora-41-xfce_socket-root: 8.28 🟢 ( previous job: 8.81, improvement: 93.92%)
  • fedora-41-xfce_exec-data-simplex: 65.54 🟢 ( previous job: 76.90, improvement: 85.23%)
  • fedora-41-xfce_exec-data-duplex: 67.67 🟢 ( previous job: 68.34, improvement: 99.02%)
  • fedora-41-xfce_exec-data-duplex-root: 106.52 🟢 ( previous job: 109.83, improvement: 96.98%)
  • fedora-41-xfce_socket-data-duplex: 134.89 🟢 ( previous job: 156.23, improvement: 86.34%)
  • whonix-gateway-17_exec: 7.25 🟢 ( previous job: 7.34, improvement: 98.84%)
  • whonix-gateway-17_exec-root: 38.67 🟢 ( previous job: 39.57, improvement: 97.71%)
  • whonix-gateway-17_socket: 6.75 🟢 ( previous job: 7.85, improvement: 85.91%)
  • whonix-gateway-17_socket-root: 8.20 🔺 ( previous job: 7.89, degradation: 103.85%)
  • whonix-gateway-17_exec-data-simplex: 64.92 🟢 ( previous job: 77.76, improvement: 83.48%)
  • whonix-gateway-17_exec-data-duplex: 80.06 🔺 ( previous job: 78.39, degradation: 102.14%)
  • whonix-gateway-17_exec-data-duplex-root: 97.60 🔺 ( previous job: 90.74, degradation: 107.55%)
  • whonix-gateway-17_socket-data-duplex: 154.80 🟢 ( previous job: 161.95, improvement: 95.58%)
  • whonix-workstation-17_exec: 8.55 🔺 ( previous job: 8.27, degradation: 103.37%)
  • whonix-workstation-17_exec-root: 54.41 🟢 ( previous job: 57.61, improvement: 94.45%)
  • whonix-workstation-17_socket: 7.92 🟢 ( previous job: 8.97, improvement: 88.34%)
  • whonix-workstation-17_socket-root: 8.66 🟢 ( previous job: 9.46, improvement: 91.53%)
  • whonix-workstation-17_exec-data-simplex: 74.66 🔺 ( previous job: 74.54, degradation: 100.16%)
  • whonix-workstation-17_exec-data-duplex: 70.60 🟢 ( previous job: 74.84, improvement: 94.34%)
  • whonix-workstation-17_socket-data-duplex: 166.32 🔺 ( previous job: 160.20, degradation: 103.82%)
  • dom0_root_seq1m_q8t1_read 3:read_bandwidth_kb: 303319.00 :green_circle: ( previous job: 289982.00, improvement: 104.60%)
  • dom0_root_seq1m_q8t1_write 3:write_bandwidth_kb: 202850.00 :green_circle: ( previous job: 101988.00, improvement: 198.90%)
  • dom0_root_seq1m_q1t1_read 3:read_bandwidth_kb: 292571.00 :green_circle: ( previous job: 14284.00, improvement: 2048.24%)
  • dom0_root_seq1m_q1t1_write 3:write_bandwidth_kb: 45759.00 :green_circle: ( previous job: 32696.00, improvement: 139.95%)
  • dom0_root_rnd4k_q32t1_read 3:read_bandwidth_kb: 19300.00 :green_circle: ( previous job: 17102.00, improvement: 112.85%)
  • dom0_root_rnd4k_q32t1_write 3:write_bandwidth_kb: 2381.00 :green_circle: ( previous job: 1091.00, improvement: 218.24%)
  • dom0_root_rnd4k_q1t1_read 3:read_bandwidth_kb: 11992.00 :green_circle: ( previous job: 11086.00, improvement: 108.17%)
  • dom0_varlibqubes_seq1m_q1t1_read 3:read_bandwidth_kb: 414456.00 :small_red_triangle: ( previous job: 433654.00, degradation: 95.57%)
  • dom0_varlibqubes_seq1m_q1t1_write 3:write_bandwidth_kb: 161673.00 :small_red_triangle: ( previous job: 167872.00, degradation: 96.31%)
  • dom0_varlibqubes_rnd4k_q32t1_read 3:read_bandwidth_kb: 101486.00 :small_red_triangle: ( previous job: 108760.00, degradation: 93.31%)
  • dom0_varlibqubes_rnd4k_q1t1_read 3:read_bandwidth_kb: 7772.00 :green_circle: ( previous job: 6356.00, improvement: 122.28%)
  • fedora-41-xfce_root_seq1m_q8t1_read 3:read_bandwidth_kb: 370652.00 :small_red_triangle: ( previous job: 401292.00, degradation: 92.36%)
  • fedora-41-xfce_root_seq1m_q8t1_write 3:write_bandwidth_kb: 144578.00 :green_circle: ( previous job: 138008.00, improvement: 104.76%)
  • fedora-41-xfce_root_seq1m_q1t1_read 3:read_bandwidth_kb: 324536.00 :green_circle: ( previous job: 306332.00, improvement: 105.94%)
  • fedora-41-xfce_root_rnd4k_q32t1_read 3:read_bandwidth_kb: 88506.00 :green_circle: ( previous job: 88110.00, improvement: 100.45%)
  • fedora-41-xfce_root_rnd4k_q1t1_read 3:read_bandwidth_kb: 8423.00 :green_circle: ( previous job: 7675.00, improvement: 109.75%)
  • fedora-41-xfce_root_rnd4k_q1t1_write 3:write_bandwidth_kb: 1571.00 :green_circle: ( previous job: 950.00, improvement: 165.37%)
  • fedora-41-xfce_private_seq1m_q8t1_read 3:read_bandwidth_kb: 370652.00 :small_red_triangle: ( previous job: 404699.00, degradation: 91.59%)
  • fedora-41-xfce_private_seq1m_q8t1_write 3:write_bandwidth_kb: 112424.00 :green_circle: ( previous job: 99783.00, improvement: 112.67%)
  • fedora-41-xfce_private_seq1m_q1t1_read 3:read_bandwidth_kb: 342783.00 :green_circle: ( previous job: 330572.00, improvement: 103.69%)
  • fedora-41-xfce_private_seq1m_q1t1_write 3:write_bandwidth_kb: 99189.00 :green_circle: ( previous job: 43760.00, improvement: 226.67%)
  • fedora-41-xfce_private_rnd4k_q32t1_read 3:read_bandwidth_kb: 94525.00 :green_circle: ( previous job: 86107.00, improvement: 109.78%)
  • fedora-41-xfce_private_rnd4k_q32t1_write 3:write_bandwidth_kb: 4465.00 :green_circle: ( previous job: 1209.00, improvement: 369.31%)
  • fedora-41-xfce_private_rnd4k_q1t1_read 3:read_bandwidth_kb: 8573.00 :small_red_triangle: ( previous job: 8908.00, degradation: 96.24%)
  • fedora-41-xfce_private_rnd4k_q1t1_write 3:write_bandwidth_kb: 1585.00 :green_circle: ( previous job: 653.00, improvement: 242.73%)
  • fedora-41-xfce_volatile_seq1m_q8t1_read 3:read_bandwidth_kb: 321945.00 :small_red_triangle: ( previous job: 335115.00, degradation: 96.07%)
  • fedora-41-xfce_volatile_seq1m_q8t1_write 3:write_bandwidth_kb: 155287.00 :green_circle: ( previous job: 88088.00, improvement: 176.29%)
  • fedora-41-xfce_volatile_seq1m_q1t1_read 3:read_bandwidth_kb: 335974.00 :green_circle: ( previous job: 323135.00, improvement: 103.97%)
  • fedora-41-xfce_volatile_rnd4k_q32t1_read 3:read_bandwidth_kb: 85456.00 :small_red_triangle: ( previous job: 86131.00, degradation: 99.22%)
  • fedora-41-xfce_volatile_rnd4k_q32t1_write 3:write_bandwidth_kb: 3007.00 :green_circle: ( previous job: 2636.00, improvement: 114.07%)
  • fedora-41-xfce_volatile_rnd4k_q1t1_read 3:read_bandwidth_kb: 7936.00 :small_red_triangle: ( previous job: 8052.00, degradation: 98.56%)
  • fedora-41-xfce_volatile_rnd4k_q1t1_write 3:write_bandwidth_kb: 2676.00 :green_circle: ( previous job: 1713.00, improvement: 156.22%)

This was referenced May 9, 2025
Merged
Merged
@zaz
Copy link
Contributor Author

zaz commented May 9, 2025

@marmarek Thanks a lot! Yes, I agree. I am working on adding some nested loops in get_tops as that has the filepath. By the time it reaches merge_tops it should be correct. It might still be a good idea to keep something like the above so if all else fails the user can infer from the JSON data where the error occurred (e.g. by using grep for some of the output). But perhaps you have a better way of wording the error or would rather specify in the docstring that the input must be checked before being passed to merge_tops.

@zaz
Copy link
Contributor Author

zaz commented May 9, 2025

@marmarek Thanks a lot! Yes, I agree. I am working on adding some nested loops in get_tops as that has the filepath. By the time it reaches merge_tops it should be correct. It might still be a good idea to keep something like the above so if all else fails the user can infer from the JSON data where the error occurred (e.g. by using grep for some of the output). But perhaps you have a better way of wording the error or would rather specify in the docstring that the input must be checked before being passed to merge_tops.

Here's the WIP gist of what I was thinking. I switched from throwing an error to logging a warning because I thought that made the output a lot cleaner, but unfortunately that caused an error to come from the minion:

debian:
      ----------
      _error:
          The command resulted in a non-zero exit code
      parsed:
          None
      retcode:
          126
      stderr:
          Request refused
      stdout:
  Cannot open display "default display"
  /usr/lib/python3.13/site-packages/salt/client/ssh/__init__.py:1606: DeprecationWarning: 'maxsplit' is passed as positional argument
    shim_command = re.split(r"\r?\n", stdout, 1)[0].strip()

I'm guessing the merge_tops from topd.py runs in dom0 and then the merge_tops macro from top.jinja runs on the minion? Did I get that right?

Sorry I haven't had time to fully understand how the whole system works. I don't believe it is documented anywhere?

@zaz
Copy link
Contributor Author

zaz commented May 10, 2025

Actually, I'm thinking instead of looping over all the tops to check them before passing to merge_tops it might be cleaner to pass the filenames to merge_tops to allow for better error logging there.

This was referenced May 17, 2025
Merged
Merged
Draft
Merged
This was referenced May 27, 2025
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Jun 5, 2025
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zaz @marmarek @qubesos-bot