Skip to content

many tests do not clean up after themselves#991

Merged
davepacheco merged 1 commit into
mainfrom
test-cleanup
Apr 28, 2022
Merged

many tests do not clean up after themselves#991
davepacheco merged 1 commit into
mainfrom
test-cleanup

Conversation

@davepacheco

Copy link
Copy Markdown
Collaborator

In the tradition of #499 and #543, this change fixes most of the tests that currently leave detritus in $TMPDIR. I looked into this while looking at #988. Like I said in #543, we could really use a macro like @teisenbe did in #502 but for things that only use a LogContext. I'm not taking that on right now.

This does not fix the internal_dns tests because that's a bunch more work. I filed #990 for that.

I may separately look at having CI fail if a successful run leaves anything in $TMPDIR.

@davepacheco

Copy link
Copy Markdown
Collaborator Author

As before, I found these by setting TMPDIR to a new directory, then listing it after a successful run:

$ ls -l $TMPDIR
total 108 
drwxr-xr-x   3 dap      staff          3 Apr 28 12:18 internal-dns-test.CRYbrg6MCjrx/
drwxr-xr-x   3 dap      staff          3 Apr 28 12:18 internal-dns-test.jaVVEPleAbdQ/
-rw-r--r--   1 dap      staff       1224 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_explain_async.15568.16.log
-rw-r--r--   1 dap      staff       1275 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_explain_full_table_scan.15568.17.log
-rw-r--r--   1 dap      staff       1194 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_explain.15568.15.log
-rw-r--r--   1 dap      staff      87648 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_lookup.15568.18.log
-rw-r--r--   1 dap      staff       1350 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_queries_do_not_require_full_table_scan.15568.5.log
-rw-r--r--   1 dap      staff      28265 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_region_allocation_is_idempotent.15568.13.log
-rw-r--r--   1 dap      staff      28456 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_region_allocation_not_enough_datasets.15568.8.log
-rw-r--r--   1 dap      staff      28807 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_region_allocation_out_of_space_does_not_fail_yet.15568.10.log
-rw-r--r--   1 dap      staff      27813 Apr 28 12:18 omicron_nexus-b671045baf67c4da-test_region_allocation.15568.6.log
-rw-r--r--   1 dap      staff      41970 Apr 28 12:18 test_all-2b010d8188459a74-serial_console_communication.15566.0.log
-rw-r--r--   1 dap      staff      28377 Apr 28 12:18 test_all-2b010d8188459a74-serial_console_communication.15566.1.log
-rw-r--r--   1 dap      staff       5363 Apr 28 12:19 test_all-6cb92dc331378648-test_update_end_to_end.16071.56.log

The file names reflect the names of the tests that created them.

@davepacheco davepacheco enabled auto-merge (squash) April 28, 2022 20:18
@davepacheco davepacheco merged commit 316f9ec into main Apr 28, 2022
@davepacheco davepacheco deleted the test-cleanup branch April 28, 2022 21:00
leftwo pushed a commit that referenced this pull request Nov 15, 2023
Crucible changes:
test-crudd can collect more info, test_up can be gentle (#997)
Decrypt without holding the downstairs lock (#1021)
Add raw file backend (#991)
Don't hold the Downstairs lock while doing encryption (#1019)
Antagonize the Crucible Agent (#1011)
The Pantry should reject non-block sized writes (#1013)
Propolis changes:
Just the commit that updates Crucible
leftwo added a commit that referenced this pull request Nov 16, 2023
Crucible changes:
test-crudd can collect more info, test_up can be gentle (#997) 
Decrypt without holding the downstairs lock (#1021) 
Add raw file backend (#991)
Don't hold the Downstairs lock while doing encryption (#1019) 
Antagonize the Crucible Agent (#1011)
The Pantry should reject non-block sized writes (#1013)

Propolis changes:
make headroom for linux virtio/9p client impl (#565)
Guarantee Tokio access for Entity methods

---------

Co-authored-by: Alan Hanson <alan@oxide.computer>
iximeow added a commit that referenced this pull request Jan 8, 2026
Propolis changes are pretty important, it's unfortunate I'd overlooked
driving the changes through to Omicron. The full list of changes:

* Do not lose wakeups for block device workers (#973)
* fix arg for dtrace script (#978)
  - no changes to Propolis
* Add support for programmable SMBIOS Type 1 table (#977)
  - unused in Nexus
* NVMe reset can discard request Permits early (#983)
* Wire up viona notify fast path (#754)
* distinguish probes across NVMe devices (#993)
* update dropshot to 0.16.6, dropshot-api-manager to 0.3.0 (#992)
* distinguish file backend thread names across backend instances (#997)
* bump softnpu (#1000)
* block/file: do pread/pwrite from Propolis heap instead of VM memory (#985)
* bins: intial CPU binding support (#991)

At least some of these are fixes for bugs we've seen in the last month
or two (#983 made for a very coreful dogfood update as Propolises were
stopped), many are either unused by Nexus (#977, #987, #993, #997) or
just not reachable from a Nexus-configured VM *yet* (#973, #985).

Both #991 and #754 are effectful today.

The initial CPU binding in Propolis is such that if a VM would use more
than half of a sled's CPUs, we explicitly bind vCPU threads to uppermost
CPUs 1:1. This helps keep VM exit/reentry quick, mostly interesting
while under high I/O load. In the limit this binding should be chosen by
Nexus, and probably should be applied for all VMs.

Additionally, #754 affects all VM NICs. It's just to avoid going out to
Propolis and back for what really can be handled in-kernel; there should
be no behavioral change here.

Finally, while there *is* a new `propolis-server` API version, and I
have sled-agent using it, I'm intentionally not plumbing that through to
Nexus. There isn't anything valuable for Nexus to do with it, so it
would be a new sled-agent API rev to include the new Propolis API type
.. for no reason. Instead, I've opted to have sled-agent do the lossless
conversion from older Propolis spec as provided by Nexus, to newer
Propolis spec.

We'll definitely have more changes to the Propolis API that will go
through to Nexus, such as aforementioned CPU binding assignments, so I
figure this will get more normalized sooner than later.
askfongjojo pushed a commit that referenced this pull request Jan 9, 2026
Propolis changes are pretty important, it's unfortunate I'd overlooked
driving the changes through to Omicron. The full list of changes:

* Do not lose wakeups for block device workers (#973)
* fix arg for dtrace script (#978)
  - no changes to Propolis
* Add support for programmable SMBIOS Type 1 table (#977)
  - unused in Nexus
* NVMe reset can discard request Permits early (#983)
* Wire up viona notify fast path (#754)
* distinguish probes across NVMe devices (#993)
* update dropshot to 0.16.6, dropshot-api-manager to 0.3.0 (#992)
* distinguish file backend thread names across backend instances (#997)
* bump softnpu (#1000)
* block/file: do pread/pwrite from Propolis heap instead of VM memory (#985)
* bins: intial CPU binding support (#991)

At least some of these are fixes for bugs we've seen in the last month
or two (#983 made for a very coreful dogfood update as Propolises were
stopped), many are either unused by Nexus (#977, #987, #993, #997) or
just not reachable from a Nexus-configured VM *yet* (#973, #985).

Both #991 and #754 are effectful today.

The initial CPU binding in Propolis is such that if a VM would use more
than half of a sled's CPUs, we explicitly bind vCPU threads to uppermost
CPUs 1:1. This helps keep VM exit/reentry quick, mostly interesting
while under high I/O load. In the limit this binding should be chosen by
Nexus, and probably should be applied for all VMs.

Additionally, #754 affects all VM NICs. It's just to avoid going out to
Propolis and back for what really can be handled in-kernel; there should
be no behavioral change here.

Finally, while there *is* a new `propolis-server` API version, and I
have sled-agent using it, I'm intentionally not plumbing that through to
Nexus. There isn't anything valuable for Nexus to do with it, so it
would be a new sled-agent API rev to include the new Propolis API type
.. for no reason. Instead, I've opted to have sled-agent do the lossless
conversion from older Propolis spec as provided by Nexus, to newer
Propolis spec.

We'll definitely have more changes to the Propolis API that will go
through to Nexus, such as aforementioned CPU binding assignments, so I
figure this will get more normalized sooner than later.
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.

2 participants