Skip to content

BFD status shouldn't 404 if only one scrimlet is present#9979

Open
jgallagher wants to merge 3 commits intomainfrom
john/fix-bfd-status-one-scrimlet
Open

BFD status shouldn't 404 if only one scrimlet is present#9979
jgallagher wants to merge 3 commits intomainfrom
john/fix-bfd-status-one-scrimlet

Conversation

@jgallagher
Copy link
Contributor

(I ran into this while shaving yaks as a part of #9832.)

There are three changes here:

af1090c changes Nexus::mg_clients() to look up MGD in DNS to find ports instead of assuming the hardcoded MGD_PORT. This allows mg_clients() to be useful in tests. (#[nexus_test] assigns random ports to services, but does put those ports in DNS.) This is kinda grody; we end up doing three DNS lookups (dendrite, MGS, and MGD) to build a HashMap<SwitchLocation, MgdClient>), but I didn't want to try to tackle that in this same PR. #8999 is related, and discussions some possible options that would let us squash this down.

ae54bbc adds a very simple test that hits the BFD status endpoint, expecting it to be empty. Without the fix from the next commit, this test fails with a 404 because #[nexus_test] only sets up one scrimlet (switch0), causing the BFD status endpoint to 404 because it can't find the other switch:

16:39:39.882Z INFO 913233fe-92a8-4635-9572-183f495429c4 (dropshot_external): request completed
    error_message_external = not found: switch with name "switch1"
    error_message_internal = not found: switch with name "switch1"
    latency_us = 385976
    local_addr = 127.0.0.1:38549
    method = GET
    remote_addr = 127.0.0.1:43174
    req_id = 262916c6-77d8-439f-8252-cede9b46f169
    response_code = 404
    uri = /v1/system/networking/bfd-status

bd79324 changes bfd_status() to skip missing switches instead of 404ing, so we still get status from the one switch. This allows the test in the previous commit to pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant