Allow Nexus to specify Zone filesystem location #5931

smklein · 2024-06-21T23:30:07Z

This is a newer version of #5050 , focused on a slightly smaller scope.

This PR adds an optional filesystem_pool to Sled Agent's API, and to Nexus' blueprint for zone construction.
This allows Nexus to have control over which zpool is being used for zone provisioning, rather than letting the Sled Agent control this decision as it used to.

By giving more control to Nexus, Nexus can better understand the impact of faults (e.g., will pulling a physical disk cause a zone to fail?) and can have more control over zone-reallocation.

This field is optional largely for backwards compatibility, but in the future, will eventually become mandatory.

Fixes #5048
Part of #5929

…ms-2

smklein · 2024-06-24T22:39:59Z

dev-tools/reconfigurator-cli/tests/test_basic.rs

+    // Add a zpool to both sleds, just to ensure that all new zones can find
+    // a transient filesystem wherever they end up being placed.
+    let _sled_agent_zpools = DiskTestBuilder::new(&cptestctx)
+        .on_sled(SledUuid::from_untyped_uuid(
+            cptestctx.sled_agent.sled_agent.id,
+        ))
+        .with_zpool_count(1)
+        .build()
+        .await;
+    let _sled_agent2_zpools = DiskTestBuilder::new(&cptestctx)
+        .on_sled(SledUuid::from_untyped_uuid(
+            cptestctx.sled_agent2.sled_agent.id,
+        ))
+        .with_zpool_count(1)
+        .build()
+        .await;


This "works" -- it forces zpools to be allocated on all sleds before we try to create a blueprint -- but it's a little gross.

I've punted cleanup of the DiskTest stuff into #5939, just to keep this PR more reasonably-sized.

smklein · 2024-06-24T22:44:20Z

nexus/test-utils/src/resource_helpers.rs

@@ -703,6 +704,53 @@ pub struct TestZpool {
    pub datasets: Vec<TestDataset>,
 }

+pub struct DiskTestBuilder<'a, N: NexusServer> {


This builder specifically is cleaned up / used by default in #5939 . However, doing so in a "not-just-additive" way changes a lot of code unrelated to this PR, so, it got split out.

smklein · 2024-06-24T22:45:26Z

nexus/types/src/deployment/zone_type.rs

-    // TODO-cleanup This currently returns `None` for zones that only have
-    // transient datasets. This should change to a non-optional value once Nexus
-    // is aware of them.
-    pub fn dataset(&self) -> Option<&OmicronZoneDataset> {


I opted to change the name of this function as a way to address this "TODO".

schema/all-zones-requests.json

smklein · 2024-06-25T22:50:21Z

As a follow-up: #5952 actually expunges these zones if the physical disk backing one of these transient zone filesystems goes away.

andrewjstone

Looks great. It's awesome to see this control progressing up to Nexus!

schema/all-zones-requests.json

andrewjstone · 2024-07-01T19:03:00Z

nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

            }
        }

-        for &zpool_id in resources.all_zpools(ZpoolFilter::InService) {
+        for &zpool_id in all_in_service_zpools {


I know this isn't explicitly part of this PR, but it appears that we always choose the first zpool that doesn't have a zone of the same type on it. This means that we aren't really balancing the load across zpools. I'm not sure if this matters or not, but it seems like it might be somewhat accidental.

If I'm reading blame correctly, this disposition was originally made in #5797 , when the sled_alloc_zpool function was originally added.

@jgallagher - was this to make pool allocation deterministic? I agree that we will probably want the ability to evenly distribute load across different zpools.

@jgallagher - was this to make pool allocation deterministic?

Yes, and also it was only choosing CRDB zones, so I didn't put much thought into it.

We probably want something like OmicronZonePlacement but for disks? (Or to expand OmicronZonePlacement to also place disks.) It is deterministic but also distributes load as evenly as it can within the set of restrictions it has. I'll file an issue.

Makes total sense, thanks for filing. I can look into the issue.

Builds on #5931 The `DiskTest` utility provides mechanisms to create zpools and datasets within a particular Sled Agent. Historically, this was used on a *single* sled agent within the control plane test context, but more recently, we've added a fair amount of multi-sled tests. This PR updates the `DiskTest` to be more usable with multiple sleds: - It provides a `DiskTestBuilder` to help callers customize behavior - It lets callers access virtual disks spread across multiple distinct sleds

This is a direct follow-up to #5931 Before this PR, physical disk expungement would not change zones based on transient filesystem pool selection. Now, this selection is a factor, and expunging disks can cause zones to be expunged if they relied on those disks. Part of #5929

Provides an internal API to remove disks, and wires it into omdb. Additionally, expands omdb commands for visibility. - `omdb db physical-disks` can be used to view all "control plane physical disks". This is similar to, but distinct from, the `omdb db inventory physical-disks` command, as it reports control plane disks that have been adopted in the control plane. This command is necessary for identifying the UUID of the associated control plane object, which is not observable via inventory. - `omdb nexus sleds expunge-disk` can be used to expunge a physical disk from a sled. This relies on many prior patches to operate correctly, but with the combination of: #5987, #5965, #5931, #5952, #5601, #5599, we can observe the following behavior: expunging a disk leads to all "users" of that disk (zone filesystems, datasets, zone bundles, etc) being removed. I tested this PR on a4x2 using the following steps: ```bash # Boot a4x2, confirm the Nexus zone is running # From g0, zlogin oxz_switch $ omdb db sleds SERIAL IP ROLE POLICY STATE ID g2 [fd00:1122:3344:103::1]:12345 - in service active 29fede5f-37e4-4528-bcf2-f3ee94924894 g0 [fd00:1122:3344:101::1]:12345 scrimlet in service active 6a2c7019-d055-4256-8bad-042b97aa0e5e g1 [fd00:1122:3344:102::1]:12345 - in service active a611b43e-3995-4cd4-9603-89ca6aca3dc5 g3 [fd00:1122:3344:104::1]:12345 scrimlet in service active f62f2cfe-d17b-4bd6-ae64-57e8224d3672 # We'll plan on expunging a disk on g1, and observing the effects. $ export SLED_ID=a611b43e-3995-4cd4-9603-89ca6aca3dc5 $ export OMDB_SLED_AGENT_URL=http://[fd00:1122:3344:102::1]:12345 $ omdb sled-agent zones list "oxz_cockroachdb_b3fecda8-2eb8-4ff3-9cf6-90c94fba7c50" "oxz_crucible_19831c98-3137-4af4-a93d-fc1a17c138f2" "oxz_crucible_6adcb8ec-6c9e-4e8a-a8d4-bbf9ad44e2c4" "oxz_crucible_74b2f587-10ce-4131-97fd-9832c52c8a41" "oxz_crucible_9e422508-f4d5-4c24-8dde-0080c0916419" "oxz_crucible_a47e9625-d189-4001-877a-cc3aa5b1f3eb" "oxz_crucible_pantry_c3b4e3cb-3e23-4f5e-921b-04e4801924fd" "oxz_external_dns_7e669b6f-a3fe-47a9-addd-20e42c58b8bb" "oxz_internal_dns_1a45a6e8-5b03-4ab4-a3db-e83fb7767767" "oxz_ntp_209ad0d0-a5e7-4ab8-ac8f-e99902697b32" "oxz_oximeter_864efebb-790f-4b7a-8377-b2c82c87f5b8" $ omdb db physical-disks | grep $SLED_ID ID SERIAL VENDOR MODEL SLED_ID POLICY STATE 23524716-a331-4d57-aa71-8bd4dbc916f8 synthetic-serial-g1_0 synthetic-vendor synthetic-model-U2 a611b43e-3995-4cd4-9603-89ca6aca3dc5 in service active 3ca1812b-55e3-47ed-861f-f667f626c8a0 synthetic-serial-g1_3 synthetic-vendor synthetic-model-U2 a611b43e-3995-4cd4-9603-89ca6aca3dc5 in service active 40139afb-7076-45d9-84cf-b96eefe7acf8 synthetic-serial-g1_1 synthetic-vendor synthetic-model-U2 a611b43e-3995-4cd4-9603-89ca6aca3dc5 in service active 5c8e33dd-1230-4214-af78-9be892d9f421 synthetic-serial-g1_4 synthetic-vendor synthetic-model-U2 a611b43e-3995-4cd4-9603-89ca6aca3dc5 in service active 85780bbf-8e2d-481e-9013-34611572f191 synthetic-serial-g1_2 synthetic-vendor synthetic-model-U2 a611b43e-3995-4cd4-9603-89ca6aca3dc5 in service active # Let's expunge the "0th" disk here. $ omdb nexus sleds expunge-disk 23524716-a331-4d57-aa71-8bd4dbc916f8 -w $ omdb nexus blueprints regenerate -w $ omdb nexus blueprints show $NEW_BLUEPRINT_ID # Observe that the new blueprint for the sled expunges some zones -- minimally, # the Crucible zone -- and no longer lists the "g1_0" disk. This should also be # summarized in the blueprint metadata comment. $ omdb nexus blueprints target set $NEW_BLUEPRINT_ID enabled -w $ omdb sled-agent zones list zones: "oxz_crucible_19831c98-3137-4af4-a93d-fc1a17c138f2" "oxz_crucible_74b2f587-10ce-4131-97fd-9832c52c8a41" "oxz_crucible_9e422508-f4d5-4c24-8dde-0080c0916419" "oxz_crucible_a47e9625-d189-4001-877a-cc3aa5b1f3eb" "oxz_crucible_pantry_c3b4e3cb-3e23-4f5e-921b-04e4801924fd" "oxz_ntp_209ad0d0-a5e7-4ab8-ac8f-e99902697b32" "oxz_oximeter_864efebb-790f-4b7a-8377-b2c82c87f5b8" # As we can see, the expunged zones have been removed. # We can also access the sled agent logs from g1 to observe that the expected requests have been sent # to adjust the set of control plane disks and expunge the expected zones. ``` This is a major part of #4719 Fixes #5370

smklein added 21 commits June 20, 2024 12:36

Start requiring zone filesystem argument

cee78bc

Deprecate the old service format

3be4b6e

Merge branch 'main' into nexus-zone-filesystems-2

8756076

Merge branch 'deprecate-services-migration' into nexus-zone-filesyste…

0aac450

…ms-2

Plumbing through filesystem_pool, still need to make it optional

3833549

Merge branch 'main' into deprecate-services-migration

aea4bdb

review feedback

b58352f

no bail just warn

a04e9c7

Merge branch 'deprecate-services-migration' into nexus-zone-filesyste…

4615f1b

…ms-2

Merge branch 'main' into deprecate-services-migration

9f09c32

Merge branch 'deprecate-services-migration' into nexus-zone-filesyste…

9db3042

…ms-2

optional value

a96fc81

are we optional yet

9858dbf

lie about filesystem_pools for simulated sled agent

f1e6f7a

Patch test_builder_zones

8a9ade7

Fix test_silos_external_dns_end_to_end

d7c462c

patch v3 schema

3c59610

Patch blueprint edit

1270098

Add schema change

f48fba3

fmt

684932d

Merge branch 'main' into nexus-zone-filesystems-2

87b8df9

smklein mentioned this pull request Jun 24, 2024

Sled Agent: Ensure that all U.2 users "let go" on disk expungement #5929

Closed

11 tasks

helios tests

52406a6

smklein mentioned this pull request Jun 24, 2024

Make DiskTest easier to use across multiple sleds #5939

Merged

smklein added 2 commits June 24, 2024 15:41

Merge branch 'main' into nexus-zone-filesystems-2

acaf91f

Cleanup

b1339d4

smklein commented Jun 24, 2024

View reviewed changes

smklein marked this pull request as ready for review June 24, 2024 22:49

smklein requested review from jgallagher and davepacheco June 24, 2024 22:51

smklein added 4 commits June 25, 2024 11:03

Merge branch 'main' into nexus-zone-filesystems-2

fcea2f1

only pick in-service zpools from reconfigurator - regression test wanted

53027a3

Merge zpool selection fns

ae41399

Add colocation test

5b38070

smklein mentioned this pull request Jun 25, 2024

Expunge zones when their transient filesystem pools are expunged #5952

Merged

smklein requested a review from andrewjstone June 26, 2024 22:10

Merge branch 'main' into nexus-zone-filesystems-2

f0ab1c2

andrewjstone approved these changes Jul 1, 2024

View reviewed changes

Merge branch 'main' into nexus-zone-filesystems-2

d83a553

smklein merged commit 98bf635 into main Jul 1, 2024
20 checks passed

smklein deleted the nexus-zone-filesystems-2 branch July 1, 2024 23:26

jgallagher mentioned this pull request Jul 2, 2024

Reconfigurator: Choosing a zpool should distribute load #5991

Open

smklein mentioned this pull request Jul 5, 2024

[nexus] Expunge disk internal API, omdb commands #5994

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Nexus to specify Zone filesystem location #5931

Allow Nexus to specify Zone filesystem location #5931

smklein commented Jun 21, 2024 •

edited

Loading

smklein Jun 24, 2024

smklein Jun 24, 2024

smklein Jun 24, 2024

smklein commented Jun 25, 2024

andrewjstone left a comment

andrewjstone Jul 1, 2024

smklein Jul 1, 2024

jgallagher Jul 2, 2024

smklein Jul 2, 2024

Allow Nexus to specify Zone filesystem location #5931

Allow Nexus to specify Zone filesystem location #5931

Conversation

smklein commented Jun 21, 2024 • edited Loading

smklein Jun 24, 2024

Choose a reason for hiding this comment

smklein Jun 24, 2024

Choose a reason for hiding this comment

smklein Jun 24, 2024

Choose a reason for hiding this comment

smklein commented Jun 25, 2024

andrewjstone left a comment

Choose a reason for hiding this comment

andrewjstone Jul 1, 2024

Choose a reason for hiding this comment

smklein Jul 1, 2024

Choose a reason for hiding this comment

jgallagher Jul 2, 2024

Choose a reason for hiding this comment

smklein Jul 2, 2024

Choose a reason for hiding this comment

smklein commented Jun 21, 2024 •

edited

Loading