Skip to content

Conversation

@bnaecker
Copy link
Collaborator

Subnets

  • Extends SubnetError variant for communicating overlapping address
    ranges to contain the actual subnet that caused the problem. This can
    be used to determine if it was a V4 or V6, client-provided or
    auto-generated subnet.
  • Reworks the SQL for checking this case. It now tries to insert NULL
    for a subnet if that's been found to overlap with an existing range,
    which violates a not-NULL constraint on the vpc_subnet table. By
    inspecting which constraint failed, we can learn which input caused
    the issue.
  • Updates tests to verify, ensuring we get the actual bad subnet back

@bnaecker bnaecker requested a review from smklein March 14, 2022 19:56
@bnaecker
Copy link
Collaborator Author

This will address #722 when merged, see that issue for more context. Briefly, this checks for an detects overlapping IP address ranges for IPv4 and IPv6 blocks separately when creating a VPC Subnet. That specific failure is returned to the client, so they know which exactly IP CIDR block they need to fix.

@bnaecker bnaecker force-pushed the subnet-alloc-error-handling branch from 7105339 to 731f5e7 Compare March 15, 2022 20:40
@bnaecker
Copy link
Collaborator Author

Thanks for the quick review @smklein!

@bnaecker bnaecker force-pushed the subnet-alloc-error-handling branch from 731f5e7 to 38ea525 Compare March 15, 2022 20:50
@bnaecker bnaecker enabled auto-merge (squash) March 15, 2022 20:50
Subnets

- Extends `SubnetError` variant for communicating overlapping address
  ranges to contain the actual subnet that caused the problem. This can
  be used to determine if it was a V4 or V6, client-provided or
  auto-generated subnet.
- Reworks the SQL for checking this case. It now tries to insert NULL
  for a subnet if that's been found to overlap with an existing range,
  which violates a not-NULL constraint on the `vpc_subnet` table. By
  inspecting which constraint failed, we can learn which input caused
  the issue.
- Updates tests to verify, ensuring we get the actual bad subnet back
@bnaecker bnaecker merged commit a0b2e89 into main Mar 15, 2022
@bnaecker bnaecker deleted the subnet-alloc-error-handling branch March 15, 2022 21:30
leftwo pushed a commit that referenced this pull request Sep 18, 2024
Crucible changes:
    Make crutest use BlockIO trait instead of a Guest (#1452)
    Use new syncfs syscall (#1427)
    Increment write backpressure before deferred encryption (#1444)
    Use RAII handles for backpressure (#1443)
    Fine-tuning backpressure clamping, and API cleanups (#1442)
    Fix outdated comment (#1447)
    Update Rust crate rusqlite to 0.32 (#1418)
    Fix write reordering bug (#1448)
    Remove `history_file` (#1446)
    Remove optionality for `BlockRes` in `DeferredWrite` (#1441)

Propolis changes
    instance spec rework: tighten up component naming (#761)
    instance spec rework: remove most dependencies on `InstanceSpecV0` from propolis-server (#757)
    fix new 1.81.0 warning and clippy error (#760)
    standalone: be more helpful with bad block device configs (#758)
leftwo added a commit that referenced this pull request Sep 18, 2024
Crucible changes:
    Make crutest use BlockIO trait instead of a Guest (#1452)
    Use new syncfs syscall (#1427)
    Increment write backpressure before deferred encryption (#1444)
    Use RAII handles for backpressure (#1443)
    Fine-tuning backpressure clamping, and API cleanups (#1442)
    Fix outdated comment (#1447)
    Update Rust crate rusqlite to 0.32 (#1418)
    Fix write reordering bug (#1448)
    Remove `history_file` (#1446)
    Remove optionality for `BlockRes` in `DeferredWrite` (#1441)

Propolis changes
    instance spec rework: tighten up component naming (#761)
instance spec rework: remove most dependencies on `InstanceSpecV0` from
propolis-server (#757)
    fix new 1.81.0 warning and clippy error (#760)
    standalone: be more helpful with bad block device configs (#758)

Co-authored-by: Alan Hanson <alan@oxide.computer>
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.

3 participants