Skip to content

dropshot-api-manager 0.7.1#1129

Merged
david-crespo merged 6 commits into
masterfrom
bump-dropshot-api
Apr 28, 2026
Merged

dropshot-api-manager 0.7.1#1129
david-crespo merged 6 commits into
masterfrom
bump-dropshot-api

Conversation

@david-crespo

@david-crespo david-crespo commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Intended to supersede #1128, which undoes the Dropshot bump.

Dropshot 0.17 includes a change oxidecomputer/dropshot#1554 that changes the OpenAPI schema for websocket endpoints. Normally to account for a schema change you just have to bump the API version, but dropshot-api-manager is thrown off because when it regenerates the schema for the blessed (old) version, it doesn't match what's on disk because the one on disk doesn't have the websocket change in it. This required oxidecomputer/dropshot-api-manager#87, which tells dropshot-api-manager specifically to ignore that websocket schema change. You still have to add an API version.

Also had to bump progenitor for oxidecomputer/progenitor#1320.

run: cargo test --lib --verbose
- name: Test everything
run: cargo test --locked
# Build and test propolis-the-library on its own; `cargo test --lib` as used

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from #1128

@david-crespo

Copy link
Copy Markdown
Contributor Author

It seems the one remaining check failure is expected.

# This job runs the PHD migrate-from-base tests, which test upgrading from the
# current mainline propolis-server to the propolis-server version under test.
#
# PHD always uses the propolis-client from the commit under test, so these tests
# will fail if there is a breaking change to the Propolis API. They'll also fail
# if there's a breaking change to the migration protocol. These changes may be
# expected, in which case this run will fail. However, the "regular" phd-run
# job should always be green before merging new PRs.
#
# This job will be removed once API breaking changes are no longer allowed.

@jmpesp

jmpesp commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

It seems the one remaining check failure is expected.

# This job runs the PHD migrate-from-base tests, which test upgrading from the
# current mainline propolis-server to the propolis-server version under test.
#
# PHD always uses the propolis-client from the commit under test, so these tests
# will fail if there is a breaking change to the Propolis API. They'll also fail
# if there's a breaking change to the migration protocol. These changes may be
# expected, in which case this run will fail. However, the "regular" phd-run
# job should always be green before merging new PRs.
#
# This job will be removed once API breaking changes are no longer allowed.

In lieu of addressing this here, maybe we should open an issue and limit this PR to only adding cargo text -- test_apis_up_to_date?

@jmpesp

jmpesp commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Nevermind, the Rust workflow isn't activating for PRs, disregard - though that may be its own problem to address

@jmpesp

jmpesp commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

However I think the original change from the other PR to add fetch depth isn't right. From omicron:

    - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
      with:
        ref: ${{ github.event.pull_request.head.sha }} # see omicron#4461
        fetch-depth: 0 # git-stub-vcs needs full history

@david-crespo

Copy link
Copy Markdown
Contributor Author

ah, ew oxidecomputer/omicron#4461. will update

@AlejandroME AlejandroME added this to the 20 milestone Apr 28, 2026
Comment thread .github/workflows/rust.yml Outdated
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
# see https://github.com/oxidecomputer/omicron/issues/4461

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need the with: here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what I get for doing it by hand

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also this needs to be under the build-and-test job :)

@iximeow

iximeow commented Apr 28, 2026

Copy link
Copy Markdown
Member

Instead, it checks out a merge commit of the PR tip with the target branch as of the time the PR was created.

til and what in the world

@iximeow iximeow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thanks.

mote for later re. steps.with.ref being github.event.pull_request.head.sha: consensus is that we'd expect the tests to all pass regardless of the specific ref used for tests here. for the same reason as in Omicron#4461 it's better for GH Actions and buildomat to test changes the same way, so it doesn't hurt to include here too.

@david-crespo david-crespo merged commit d9e5dc0 into master Apr 28, 2026
13 of 14 checks passed
@david-crespo david-crespo deleted the bump-dropshot-api branch April 28, 2026 20:12
jmpesp added a commit to oxidecomputer/omicron that referenced this pull request May 26, 2026
Update Crucible from `7103cd3a` to `bd9a0e2a`, picking up the following
PRs:

- Use an explicit rev for oxidecomputer git deps
(oxidecomputer/crucible#1936)
- Add Clone and Deserialize to VolumeInfo et al
(oxidecomputer/crucible#1935)
- Update omicron/oximeter (oxidecomputer/crucible#1933)
- [meta] update to drift 0.1.4 (oxidecomputer/crucible#1932)
- Don't log if there is nothing to log (oxidecomputer/crucible#1930)
- Add VolumeInfo (oxidecomputer/crucible#1928)
- Remove bonus Volume layer (oxidecomputer/crucible#1927)
- Add session and client id to panic messages
(oxidecomputer/crucible#1926)
- [crucible-agent-types] migrate to RFD 619 pattern
(oxidecomputer/crucible#1899)
- Background read-only region creation (oxidecomputer/crucible#1919)
- [crucible-downstairs-repair] switch to RFD 619 pattern
(oxidecomputer/crucible#1901)
- [crucible-pantry] switch to RFD 619 pattern
(oxidecomputer/crucible#1900)
- Use separate in-memory types (oxidecomputer/crucible#1913)
- Remove old field from dtrace action script
(oxidecomputer/crucible#1917)
- Retry data writes that return an IO error
(oxidecomputer/crucible#1915)
- Bump dropshot to 0.17.0 (oxidecomputer/crucible#1909)
- Reject snapshot requests when read-only (oxidecomputer/crucible#1914)
- update ringbuf method, fix clippy lint (oxidecomputer/crucible#1904)
- bump vergen-v9 version too (oxidecomputer/crucible#1903)
- update dropshot to 0.16.7, dropshot-api-manager to 0.5.2
(oxidecomputer/crucible#1851)
- perf-vol.d updates (oxidecomputer/crucible#1898)
- upgrade progenitor to 0.13, reqwest to 0.13
(oxidecomputer/crucible#1854)
- Remove cargo nextest from github workflow, out of space
(oxidecomputer/crucible#1846)
- Add a test for VCR serialize/deserialize (oxidecomputer/crucible#1843)

Update Propolis from `bc489ddf` to `58ab73bd`, picking up the following
PRs:

- Bump crucible to latest, update Omicron, use explicit revs
(oxidecomputer/propolis#1141)
- Add project and silo ids to VM attestation
(oxidecomputer/propolis#1114)
- Update escargot (oxidecomputer/propolis#1139)
- Prefix shebang and mark D scripts as executable
(oxidecomputer/propolis#1140)
- Fix error in propolis-server README (oxidecomputer/propolis#1138)
- [meta] update to drift 0.1.4 (oxidecomputer/propolis#1137)
- Fix Intel CPUID leaf 4 cache topology for SMT
(oxidecomputer/propolis#1002)
- support NVMe Deallocate (oxidecomputer/propolis#1105)
- viona: do not lose used/avail indices (oxidecomputer/propolis#1135)
- viona: multiqueue device should stay multiqueue across migration
(oxidecomputer/propolis#1121)
- Bump crucible rev to latest (oxidecomputer/propolis#1132)
- expand zerocopy IntoBytes/FromByes use in guest memory accesses
(oxidecomputer/propolis#1130)
- dropshot-api-manager 0.7.1 (oxidecomputer/propolis#1129)
- improve slog component setting (oxidecomputer/propolis#1124)
- wait for viona Poller to run before declaring device running
(oxidecomputer/propolis#1118)
- virtio: tolerate importing queues with adjusted size
(oxidecomputer/propolis#1117)
- Run viona unit tests in CI (oxidecomputer/propolis#1120)
- feature gate Crucible-specific boot digest code
(oxidecomputer/propolis#1119)

Also:

- ran `cargo update -p vergen`

- removed the `reqwest012` dependency

- removed `reqwest012_client` from Nexus

- ran `cargo hakari generate` and `cargo hakari manage-deps`

- replace use of `ProgenitorOperationRetry` with
`retry_operation_while_indefinitely`

- during the region replacement drive saga, consume the new `VolumeInfo`
from Propolis and use that to determine when to consider a replacement
done
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.

4 participants