Don't unwrap when we can't create a dataset

[leftwo]

If we can't create a dataset for a region, don't panic, just fail.

This removes all the unwrap() calls from the worker function.

Here is the log from the agent when a region creation fails:

21:58:17.488Z INFO crucible-agent (worker): Region size:53687091200 reservation:67108864000 quota:161061273600                                                      
21:58:17.505Z INFO crucible-agent (worker): zfs set reservation of 67108864000 for oxp_31bd71cd-4736-4a12-a387-9b74b050396f/crucible/regions/b3eec9cd-1152-4a50-aa4-8a87cd8c916a                                                                      
21:58:17.505Z INFO crucible-agent (worker): zfs set quota of 161061273600 for oxp_31bd71cd-4736-4a12-a387-9b74b050396f/crucible/regions/b3eec9cd-1152-4a50-aa41-8a87cd8c916a                                                                          
21:58:17.557Z ERRO crucible-agent (worker): Dataset b3eec9cd-1152-4a50-aa41-8a87cd8c916a creation failed: zfs create failed! out: err:cannot create 'oxp_31bd71cd-4736-4a12-a387-9b74b050396f/crucible/regions/b3eec9cd-1152-4a50-aa41-8a87cd8c916a': out of space                                                                      
21:58:17.557Z INFO crucible-agent (datafile): region b3eec9cd-1152-4a50-aa41-8a87cd8c916a state: Requested -> Destroyed                                             
21:58:17.792Z INFO crucible-agent (dropshot): request completed 

[leftwo]

What would you consider if we also make:
&region_dataset.path().unwrap(),

A local variable to this match arm, and do the same break requested if we can't get the name.
I see there are a few places with that same unwrap, but I don't know if we should forge ahead if
that unwrap fails, or also df.fail(&r.id) and break..

[leftwo]

More unwraps removed, or, less unwraps, or .. fewer.

[leftwo]

Attached is a nexus log from the disk create that fails when there is not enough space:

saga-log.txt

This change, while not 100% of what we want, will at least prevent us from going off the edge
and panic'ing the agent.

[jmpesp]

I agree with these changes, however we may want to think about retrying until success here. Nexus will issue a request and then poll the agent waiting for the state transition to take place. If there's an error here, Nexus will poll indefinitely.

[jmpesp]

Actually, scratch what I said before! If the agent attempts to retry and we're out of space, that's not desirable.

[jmpesp]

Why not df.fail here?

[leftwo]

I had df.fail() there originally, but then the disk create saga unwinds and trys to delete the
disk, and we can't delete a failed disk. I also had tried making it Tombstoned, and that made
the saga unhappy as well.

The current stuff here does not require any changes on the Omicron side. I think eventually we
may want to pass a specific message back to Omicron that we lack the space for this request.
Also in the work to do column is having Omicron do better accounting and not even try to allocate
a disk if we don't have space for it.

[leftwo]

Actually, scratch what I said before! If the agent attempts to retry and we're out of space, that's not desirable.

The way it works now, we just fail the request and the saga unwinds and reports a 500 back to the user:

error; status code: 500 Internal Server Error
{
  "error_code": "Internal",
  "message": "Internal Server Error",
  "request_id": "998d13ff-29d8-482e-841d-2d305928bbfd"
}

Not the best, but better than the agent panicing :)
I think a longer term solution can come in with Omicron changes to better calculate disk resources to begin with,
as well as take more return codes from the agent and pass them back to the user.

[leftwo]

Now Fail is a vaild state to then destroy.
No Omicron side changes needed.

[jmpesp]

I don't think Nexus is doing the right thing if it's asking for snapshots from a failed region - where did the call come from?

[leftwo]

As part of the region delete, it first checks for snapshots, agent/src/server.rs:

#[endpoint {                                                                      
    method = DELETE,                                                              
    path = "/crucible/0/regions/{id}",                                            
}]                                                                                
async fn region_delete(                                                           
    rc: RequestContext<Arc<DataFile>>,                                            
    path: TypedPath<RegionPath>,                                                  
) -> SResult<HttpResponseDeleted, HttpError> {                                    
    let p = path.into_inner();                                                    
                                                                                  
    // Cannot delete a region that's backed by a ZFS dataset if there are         
    // snapshots.                                                                 
                                                                                      let snapshots = match rc.context().get_snapshots_for_region(&p.id) {          
        Ok(results) => results,                                                   
        Err(e) => {                                                               
            return Err(HttpError::for_internal_error(e.to_string()));             
        }                                                                         
    };

Without that, the delete is refused because we get an error back from the get_snapshots call.