Skip to content

Conversation

@jgallagher
Copy link
Contributor

This is followup from #7308 and #7500 (comment):

  • Remove From<BlueprintZonesConfig> for OmicronZonesConfig. This included expunged zones, but thankfully had no callers.
  • Change BlueprintZonesConfig::to_omicron_zones_config(filter) to BlueprintZonesConfig::into_running_omicron_zones_config() (no filter argument). All the callers of this method were passing BlueprintZoneFilter::ShouldBeRunning, and I don't think there's a reason to use any other filter?
  • Remove From<BlueprintPhysicalDisksConfig> for OmicronPhysicalDisksConfig (which included expunged disks), and replace it with BlueprintPhysicalDisksConfig::into_in_service_disks(). This one did have callers, including the blueprint executor, but I think we've avoided problems because the planner current drops disks if the incoming planning input says they're not in service. I'm not sure that planner behavior is right, and might change with Expunge and Decommission disks in planner #7286, so it seemed safer to go ahead and fix this now.

@jgallagher jgallagher merged commit 21d47c5 into main Feb 10, 2025
16 checks passed
@jgallagher jgallagher deleted the john/kill-bad-from-impls branch February 10, 2025 15:34
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