More omdb improvements #7805
Open
More omdb improvements #7805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Three improvements to omdb: - for the `replacements-to-do` command, show if there's an existing replacement request: display the id, request time, and state - add a sub-command to show if any volumes are cooked, aka unable to activate due to a region set containing all expunged targets - add a sub-command to show what volumes reference an ip, netmask, read-only region, or region snapshot
|
Some sample output: |
|
cooked output: |
leftwo
reviewed
Mar 21, 2025
dev-tools/omdb/src/bin/omdb/db.rs
Outdated
| @@ -847,6 +852,10 @@ enum VolumeCommands { | |||
| List, | |||
| /// What is holding the lock? | |||
| LockHolder(VolumeLockHolderArgs), | |||
| /// What volumes are cooked (read: cannot activate)? | |||
| Cooked, | |||
There was a problem hiding this comment.
While Iove the cooked name, I think we might want a better one that describes what this command will returned. Invalid? illegal? And, also, the comment should not be a question :)
There was a problem hiding this comment.
The question part is for the help commands listing:
Commands:
info Get info for a specific volume
list Summarize current volumes
lock-holder What is holding the lock?
cooked What volumes are cooked (read: cannot activate)?
reference What volumes reference a thing?
help Print this message or the help of the given subcommand(s)
I've changed this to CannotActivate in 21d9658
| #[clap(group( | ||
| ArgGroup::new("volume-reference-group") | ||
| .required(true) | ||
| .args(&["ip", "net", "read_only_region", "region_snapshot"]) |
There was a problem hiding this comment.
I think a question users are going to have is "what is the difference between a region_snapshot and just a snapshot". I don't know how to tell them. I also don't know what if anything we do different here.
dev-tools/omdb/src/bin/omdb/db.rs
Outdated
| VolumeCookedResult::TargetNotFound { target } => { | ||
| println!( | ||
| "target {target} not found (was probably \ | ||
| deleted concurrently when `volume_cooked` was called)" |
There was a problem hiding this comment.
I think we should not print the probably ... message here, as transient things like this can happen all over omdb and we make no guarantee that things remain static.
There was a problem hiding this comment.
agreed, done in 4d16c73
| fetch_opts: &DbFetchOptions, | ||
| args: &VolumeReferenceArgs, | ||
| ) -> Result<(), anyhow::Error> { | ||
| let volume_ids: Vec<Uuid> = if let Some(ip) = args.ip { |
There was a problem hiding this comment.
I think you are working on the "clap should not .." path through here already.
There was a problem hiding this comment.
yeah, 996da6e validates that three UUIDs are supplied
dev-tools/omdb/src/bin/omdb/db.rs
Outdated
| #[clap(long, conflicts_with_all = ["ip", "net", "region_snapshot"])] | ||
| read_only_region: Option<Uuid>, | ||
|
|
||
| /// Provide dataset, region, and snapshot ID in a string, delimited by |
There was a problem hiding this comment.
This help comment here is what I need to see.
From the help output:
--region-snapshot <REGION_SNAPSHOT>
That makes it seem like I only need a single item. Is this part done by clap?
There was a problem hiding this comment.
yeah. with 996da6e this now looks like --region_snapshot <REGION_SNAPSHOT> <REGION_SNAPSHOT> <REGION_SNAPSHOT> and I don't know how to fix it 👎
leftwo
approved these changes
Mar 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Three improvements to omdb:
for the
replacements-to-docommand, show if there's an existing replacement request: display the id, request time, and stateadd a sub-command to show if any volumes are cooked, aka unable to activate due to a region set containing all expunged targets
add a sub-command to show what volumes reference an ip, netmask, read-only region, or region snapshot