oxidecomputer / oxidecomputer/omicron

Reconfigurator: Planner should check disks and datasets before decommissioning a sled

Open
#7,238 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Description

#7234 implements an internal validate_decommissionable() method on SledEditor that checks whether a sled the planner wants to decommission has any non-expunged zones. It should also check for non-expunged disks and datasets, but at the moment doing so causes a variety of test failure. We should add something like this:

    fn validate_decommisionable(&self) -> Result<(), SledEditError> {
        // TODO-john The disks and datasets checks below don't pass what the
        // planner does currently to decommission sleds: if a sled is expunged,
        // we'll omit its disks and datasets from the outgoing blueprint
        // entirely without setting them all to the `Expunged` disposition.
        // Fixing this will conflict with ongoing disk work, so for now these
        // checks are commented out.
        /*
        // Check that all disks are expunged...
        if let Some(disk) =
            self.disks(DiskFilter::All).find(|disk| match disk.disposition {
                BlueprintPhysicalDiskDisposition::InService => true,
                BlueprintPhysicalDiskDisposition::Expunged => false,
            })
        {
            return Err(SledEditError::NonDecommissionableDiskInService {
                disk_id: disk.id,
                zpool_id: disk.pool_id,
            });
        }

        // ... and all datasets are expunged ...
        if let Some(dataset) =
            self.datasets(BlueprintDatasetFilter::All).find(|dataset| {
                match dataset.disposition {
                    BlueprintDatasetDisposition::InService => true,
                    BlueprintDatasetDisposition::Expunged => false,
                }
            })
        {
            return Err(SledEditError::NonDecommissionableDatasetInService {
                dataset_id: dataset.id,
                kind: dataset.kind.clone(),
            });
        }
        */

        // ... rest of method to check zones ...
    }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with SledEditor::validate_decommissionable and the implementation from #7234, then inspect the planner's outgoing blueprint handling for expunged sleds. Restore the disk and dataset checks and run the affected planner tests; done means non-expunged disks and datasets prevent decommissioning without breaking existing sled decommissioning behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.