feat(dedicated-cloud): Add datacenter management commands and enhanced display #89
Conversation
This file contains hidden or 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
added 18 commits
November 18, 2025 10:35
…d display - Add datacenter list and get commands - Display datacenter details with clusters, hosts, and filers information - Add cluster information with DRS/HA status indicators - Enhance host display with connection state, maintenance status, cores, RAM, VMs, billing - Enhance filer display with size, space free, cluster, connection state, visibility - Add totals (CPU cores, RAM, VMs, Disk Space) in both datacenter list and get views - Support for both local and global filers - Format version and location mapping for better readability - Use color indicators for connection and status states Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
…mmands - Add TestDedicatedCloudListCmd to test list command with regionLocation and version formatting - Add TestDedicatedCloudGetCmd to test get command with datacenters table - Add TestDedicatedCloudDatacenterListCmd to test datacenter list with totals calculation - Add TestDedicatedCloudDatacenterGetCmd to test datacenter get with clusters, hosts, and filers All tests use httpmock to mock API responses and verify the correct display of enriched data. Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
Move {{if index .Result "datacenters"}} condition above the '## Datacenters' title
so that the entire section (including title) is conditionally displayed.
If the datacenters list is empty, the section won't be shown at all.
Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
Replace the type switch for datacenterId conversion with fmt.Sprint(). Since the API schemas guarantee datacenterId is an int, we can trust the API and use fmt.Sprint() which handles all numeric types automatically. This simplifies the code and makes it more maintainable. Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
…helpers The helper functions toFloat64 and toInt were not handling json.Number type, which caused all totals (CPU cores, RAM, VMs, disk space) to be displayed as 0 in the datacenter list and get commands. This commit adds support for json.Number type conversion in both helper functions, ensuring that numeric values are correctly extracted and summed regardless of their JSON deserialization type. Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
…le wrapping - Remove IAM information fetch from datacenter get command as it belongs to the dedicated cloud service, not the datacenter. Users can get IAM info by running dedicated-cloud get separately if needed. - Add glamour.WithWordWrap(0) to disable word wrapping for markdown tables, ensuring hosts and filers tables display on a single line - Restore full column names (Connection, State, Space Free, etc.) and remove name truncation to display all information without cutting Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
Simplify version field extraction by using direct type assertion with blank identifier instead of checking ok and assigning separately. Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
Use direct type assertion for versionStr from major field and use local variables in if statements for minor and build fields. Also add empty string checks to avoid adding dots when values are empty. Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
…ter details Replace --hosts, --filers, --clusters flags with dedicated subcommands: - dedicated-cloud datacenter get: shows basic info + totals only - dedicated-cloud datacenter hosts: shows hosts information only - dedicated-cloud datacenter filers: shows filers information only - dedicated-cloud datacenter clusters: shows clusters information only This reduces the number of API calls per command and makes commands less error-prone, as suggested in code review. Users can now fetch only the information they need instead of everything at once. Totals (CPU cores, RAM, VMs, disk space) are only displayed in the main 'get' command, not in the subcommands. Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Replace 'for i := range hosts' with 'for _, host := range hosts' - Use 'host' directly instead of 'hosts[i]' for better readability - Add json.Number case to switch statements for RAM and VM totals - Update test expectations for datacenter get command Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Replace 'for i := range hosts' with 'for _, host := range hosts' - Replace 'for i := range allFilers' with 'for i, filer := range allFilers' - Use switch statement for clusterId type assertion - Use direct variable references instead of array indexing for better readability Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Replace if statements with switch for host and filer vmTotal - Handle float64, int, and json.Number types consistently - Improve code consistency across dedicatedcloud service Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Replace int(toFloat64(cpuNumRaw)) with toInt(cpuNumRaw) - Simplify code by using dedicated toInt function instead of double conversion - Apply to both ListDatacenter and getDatacenterWithOptions functions Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Replace multiple if statements with switch statement - Improve code consistency with rest of dedicatedcloud service - Handle int, float64, and json.Number types consistently Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Replace multiple if statements with switch statement - Improve code consistency with toInt function - Handle int, float64, and json.Number types consistently Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Replace switch statement with direct toFloat64 call - Simplify code by leveraging toFloat64 function that handles all types - Apply to both ListDatacenter and getDatacenterWithOptions functions Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- All TestDedicatedCloudDatacenterGetCmd tests pass - All TestDedicatedCloudDatacenterListCmd tests pass - All TestDedicatedCloudGetCmd tests pass - All TestDedicatedCloudListCmd tests pass Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
- Update expected table format to match WithWordWrap(0) behavior - Table columns are now more compact due to word wrap disabled - All tests now pass Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com> Signed-off-by: François Loiseau <francois.loiseau@ovhcloud.com>
Signed-off-by: fluatovh <76485062+fluatovh@users.noreply.github.com>
removed unused variable Signed-off-by: fluatovh <76485062+fluatovh@users.noreply.github.com>
fixing conflict Signed-off-by: fluatovh <76485062+fluatovh@users.noreply.github.com>
|
@ovh/su-developer-platform-api-exposition any news at that PR? |
Collaborator
|
@Hugo1380 sorry for the delay, the PR is quite big… I'll check it asap |
amstuta
requested changes
Dec 30, 2025
| exitError("failed to execute template: %s", err) | ||
| } | ||
|
|
||
| // Define word wrap for the renderer. |
Collaborator
There was a problem hiding this comment.
why did you remove this ?
| dedicatedcloudDatacenterCmd.AddCommand(datacenterGetCmd) | ||
|
|
||
| dedicatedcloudDatacenterCmd.AddCommand(&cobra.Command{ | ||
| Use: "hosts <service_name> <datacenter_id>", |
Collaborator
There was a problem hiding this comment.
should be datacenter host list <service_name> <datacenter_id> to be consistent with the other CLI commands (same for filers and clusters)
| out, err := cmd.Execute("dedicated-cloud", "list") | ||
|
|
||
| require.CmpNoError(err) | ||
| assert.Cmp(cleanWhitespacesHelper(out), td.Contains("pcc-12345")) |
Collaborator
There was a problem hiding this comment.
why not directly comparing the whole output ?
| datacenterTemplate string | ||
| ) | ||
|
|
||
| // toFloat64 converts any numeric type to float64 |
Collaborator
There was a problem hiding this comment.
could you move these two funcs in internal/utils/utils.go ?
|
|
||
| // Build location map by fetching each location detail | ||
| locationMap := make(map[string]string) | ||
| for _, locationID := range locationIDs { |
Collaborator
There was a problem hiding this comment.
you should use httpLib.FetchObjectsParallel instead (https://github.com/ovh/ovhcloud-cli/blob/main/internal/http/client.go#L50)
|
|
||
| // Sum VMs | ||
| if vmTotal, ok := host["vmTotal"]; ok && vmTotal != nil { | ||
| switch v := vmTotal.(type) { |
Collaborator
There was a problem hiding this comment.
why not use your toInt func ?
| host["coreNumber"] = coreNumber | ||
|
|
||
| // Add VM count | ||
| switch v := host["vmTotal"].(type) { |
Collaborator
There was a problem hiding this comment.
you should use toInt func
| if includeHosts { | ||
| for _, host := range hosts { | ||
| if clusterName, ok := host["clusterName"].(string); ok && clusterName != "" { | ||
| switch v := host["clusterId"].(type) { |
| filer["clusterName"] = clusterName | ||
|
|
||
| // Add VM count | ||
| switch v := filer["vmTotal"].(type) { |
Collaborator
There was a problem hiding this comment.
same, you should use toInt func
| getDatacenterWithOptions(args, false, false, true, false) | ||
| } | ||
|
|
||
| func getDatacenterWithOptions(args []string, includeHosts, includeFilers, includeClusters, includeTotals bool) { |
Collaborator
There was a problem hiding this comment.
I think this func and the related CLI commands should be re-designed a bit:
- the other commands of the CLI are designed to fetch one kind of object per command. Here you have 4 commands that fetch the same base and then other sub-objects. It increases the number of HTTP calls necessary to run any command, so it will be longer to execute and more error-prone in case of API errors. I would prefer that each command only fetches one kind of information (only the hosts, only the filers, ...), OR having only the command to fetch a datacenter with flags like
--with-hosts,--with-filers, … - this single func to fetch everything is hard to read so it will be hard to maintain, it should at least be splitted in separate funcs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
This PR adds comprehensive datacenter management features to the dedicated-cloud commands.
Changes
Files Changed
Testing
Tested with personal datacenter & PCC
IA Generated PR & code.