Skip to content

Commit d4b6aa9

Browse files
alzimmermsftalzimmermsft
authored
Migrate Compute tools to new tool design (#3096)
* Migrate Compute tools to new tool design * Fix incorrect description and error message * Few small fixes * Revert tool parameter type change * Update recording
1 parent 83a8893 commit d4b6aa9

51 files changed

Lines changed: 790 additions & 1471 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎servers/Azure.Mcp.Server/changelog-entries/1784327078851.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
changes:
2+
- section: "Breaking Changes"
3+
description: "Removed unused parameters from Compute tools."

‎tools/Azure.Mcp.Tools.Compute/src/Azure.Mcp.Tools.Compute.csproj‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@
77
<EmbeddedResource Include="**\Resources\*.json" />
88
</ItemGroup>
99
<ItemGroup>
10-
<ProjectReference Include="..\..\..\core\Azure.Mcp.Core\src\Azure.Mcp.Core.csproj" />
10+
<ProjectReference Include="$(RepoRoot)core\Azure.Mcp.Core\src\Azure.Mcp.Core.csproj" />
1111
</ItemGroup>
1212
<ItemGroup>
1313
<PackageReference Include="Azure.ResourceManager" />
1414
<PackageReference Include="Azure.ResourceManager.Compute" />
1515
<PackageReference Include="Azure.ResourceManager.Network" />
1616
<PackageReference Include="Microsoft.Extensions.DependencyInjection" />
1717
<PackageReference Include="ModelContextProtocol" />
18-
<PackageReference Include="System.CommandLine" />
1918
</ItemGroup>
2019
</Project>

‎tools/Azure.Mcp.Tools.Compute/src/Commands/BaseComputeCommand.cs‎

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎tools/Azure.Mcp.Tools.Compute/src/Commands/ComputeJsonContext.cs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,14 @@ namespace Azure.Mcp.Tools.Compute.Commands;
1616
[JsonSerializable(typeof(DiskDeleteCommand.DiskDeleteCommandResult))]
1717
[JsonSerializable(typeof(DiskGetCommand.DiskGetCommandResult))]
1818
[JsonSerializable(typeof(DiskUpdateCommand.DiskUpdateCommandResult))]
19-
[JsonSerializable(typeof(Models.DiskInfo))]
20-
[JsonSerializable(typeof(List<Models.DiskInfo>))]
19+
[JsonSerializable(typeof(DiskInfo))]
20+
[JsonSerializable(typeof(List<DiskInfo>))]
2121
[JsonSerializable(typeof(VmCreateCommand.VmCreateCommandResult))]
2222
[JsonSerializable(typeof(VmCreateResult))]
2323
[JsonSerializable(typeof(VmUpdateCommand.VmUpdateCommandResult))]
2424
[JsonSerializable(typeof(VmUpdateResult))]
25-
[JsonSerializable(typeof(VmGetCommand.VmGetSingleResult))]
26-
[JsonSerializable(typeof(VmGetCommand.VmGetListResult))]
27-
[JsonSerializable(typeof(VmssGetCommand.VmssGetSingleResult))]
28-
[JsonSerializable(typeof(VmssGetCommand.VmssGetListResult))]
29-
[JsonSerializable(typeof(VmssGetCommand.VmssGetVmInstanceResult))]
25+
[JsonSerializable(typeof(VmGetCommand.VmGetResult))]
26+
[JsonSerializable(typeof(VmssGetCommand.VmssGetResult))]
3027
[JsonSerializable(typeof(VmssCreateCommand.VmssCreateCommandResult))]
3128
[JsonSerializable(typeof(VmssCreateResult))]
3229
[JsonSerializable(typeof(VmDeleteCommand.VmDeleteCommandResult))]

‎tools/Azure.Mcp.Tools.Compute/src/Commands/Disk/DiskCreateCommand.cs‎

Lines changed: 30 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33

44
using System.Net;
55
using System.Security;
6+
using Azure.Mcp.Core.Commands.Subscription;
7+
using Azure.Mcp.Core.Services.Azure.Subscription;
68
using Azure.Mcp.Tools.Compute.Models;
7-
using Azure.Mcp.Tools.Compute.Options;
89
using Azure.Mcp.Tools.Compute.Options.Disk;
910
using Azure.Mcp.Tools.Compute.Services;
11+
using Microsoft.Extensions.Logging;
1012
using Microsoft.Mcp.Core.Commands;
11-
using Microsoft.Mcp.Core.Extensions;
1213
using Microsoft.Mcp.Core.Models.Command;
13-
using Microsoft.Mcp.Core.Models.Option;
1414

1515
namespace Azure.Mcp.Tools.Compute.Commands.Disk;
1616

@@ -28,131 +28,48 @@ namespace Azure.Mcp.Tools.Compute.Commands.Disk;
2828
ReadOnly = false,
2929
Secret = false,
3030
LocalRequired = false)]
31-
public sealed class DiskCreateCommand(
32-
ILogger<DiskCreateCommand> logger,
33-
IComputeService computeService)
34-
: BaseComputeCommand<DiskCreateOptions>(true)
31+
public sealed class DiskCreateCommand(ILogger<DiskCreateCommand> logger, IComputeService computeService, ISubscriptionResolver subscriptionResolver)
32+
: SubscriptionCommand<DiskCreateOptions, DiskCreateCommand.DiskCreateCommandResult>(subscriptionResolver)
3533
{
36-
3734
private readonly ILogger<DiskCreateCommand> _logger = logger ?? throw new ArgumentNullException(nameof(logger));
3835
private readonly IComputeService _computeService = computeService ?? throw new ArgumentNullException(nameof(computeService));
3936

40-
protected override void RegisterOptions(Command command)
37+
public override void ValidateOptions(DiskCreateOptions options, ValidationResult validationResult)
4138
{
42-
base.RegisterOptions(command);
43-
command.Options.Add(ComputeOptionDefinitions.Disk.AsRequired());
44-
command.Options.Add(ComputeOptionDefinitions.Source);
45-
command.Options.Add(ComputeOptionDefinitions.Location);
46-
command.Options.Add(ComputeOptionDefinitions.SizeGb);
47-
command.Options.Add(ComputeOptionDefinitions.Sku);
48-
command.Options.Add(ComputeOptionDefinitions.OsType);
49-
command.Options.Add(ComputeOptionDefinitions.Zone);
50-
command.Options.Add(ComputeOptionDefinitions.HyperVGeneration);
51-
command.Options.Add(ComputeOptionDefinitions.MaxShares);
52-
command.Options.Add(ComputeOptionDefinitions.NetworkAccessPolicy);
53-
command.Options.Add(ComputeOptionDefinitions.EnableBursting);
54-
command.Options.Add(ComputeOptionDefinitions.Tags);
55-
command.Options.Add(ComputeOptionDefinitions.DiskEncryptionSet);
56-
command.Options.Add(ComputeOptionDefinitions.EncryptionType);
57-
command.Options.Add(ComputeOptionDefinitions.DiskAccessId);
58-
command.Options.Add(ComputeOptionDefinitions.Tier);
59-
command.Options.Add(ComputeOptionDefinitions.GalleryImageReference);
60-
command.Options.Add(ComputeOptionDefinitions.GalleryImageReferenceLun);
61-
command.Options.Add(ComputeOptionDefinitions.DiskIopsReadWrite);
62-
command.Options.Add(ComputeOptionDefinitions.DiskMbpsReadWrite);
63-
command.Options.Add(ComputeOptionDefinitions.UploadType);
64-
command.Options.Add(ComputeOptionDefinitions.UploadSizeBytes);
65-
command.Options.Add(ComputeOptionDefinitions.SecurityType);
66-
command.Validators.Add(result =>
67-
{
68-
var source = result.GetValueOrDefault<string>(ComputeOptionDefinitions.Source.Name);
69-
var sizeGb = result.GetValueOrDefault<int>(ComputeOptionDefinitions.SizeGb.Name);
70-
var galleryImageReference = result.GetValueOrDefault<string>(ComputeOptionDefinitions.GalleryImageReference.Name);
71-
var uploadType = result.GetValueOrDefault<string>(ComputeOptionDefinitions.UploadType.Name);
72-
var uploadSizeBytes = result.GetValueOrDefault<long>(ComputeOptionDefinitions.UploadSizeBytes.Name);
73-
var securityType = result.GetValueOrDefault<string>(ComputeOptionDefinitions.SecurityType.Name);
74-
75-
if (string.IsNullOrEmpty(source) && sizeGb <= 0 && string.IsNullOrEmpty(galleryImageReference) && string.IsNullOrEmpty(uploadType))
76-
{
77-
result.AddError("Either --source, --size-gb, --gallery-image-reference, or --upload-type must be specified.");
78-
}
79-
80-
if (!string.IsNullOrEmpty(uploadType) && uploadSizeBytes <= 0)
81-
{
82-
result.AddError("--upload-size-bytes is required when --upload-type is specified.");
83-
}
84-
85-
if (string.Equals(uploadType, "UploadWithSecurityData", StringComparison.OrdinalIgnoreCase)
86-
&& string.IsNullOrEmpty(securityType))
87-
{
88-
result.AddError("--security-type is required when --upload-type is 'UploadWithSecurityData'.");
89-
}
90-
});
91-
}
92-
93-
protected override DiskCreateOptions BindOptions(ParseResult parseResult)
94-
{
95-
var options = base.BindOptions(parseResult);
96-
options.Disk = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Disk.Name);
97-
options.Source = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Source.Name);
98-
options.Location = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Location.Name);
99-
options.ResourceGroup ??= parseResult.GetValueOrDefault<string>(OptionDefinitions.Common.ResourceGroup.Name);
100-
101-
var sizeGb = parseResult.GetValueOrDefault<int>(ComputeOptionDefinitions.SizeGb.Name);
102-
options.SizeGb = sizeGb > 0 ? sizeGb : null;
103-
104-
options.Sku = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Sku.Name);
105-
options.OsType = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.OsType.Name);
106-
options.Zone = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Zone.Name);
107-
options.HyperVGeneration = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.HyperVGeneration.Name);
108-
109-
var maxShares = parseResult.GetValueOrDefault<int>(ComputeOptionDefinitions.MaxShares.Name);
110-
options.MaxShares = maxShares > 0 ? maxShares : null;
111-
112-
options.NetworkAccessPolicy = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.NetworkAccessPolicy.Name);
113-
options.EnableBursting = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.EnableBursting.Name);
114-
options.Tags = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Tags.Name);
115-
options.DiskEncryptionSet = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.DiskEncryptionSet.Name);
116-
options.EncryptionType = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.EncryptionType.Name);
117-
options.DiskAccessId = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.DiskAccessId.Name);
118-
options.Tier = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Tier.Name);
119-
options.GalleryImageReference = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.GalleryImageReference.Name);
120-
121-
options.GalleryImageReferenceLun = parseResult.GetValueOrDefault<int?>(ComputeOptionDefinitions.GalleryImageReferenceLun.Name);
39+
base.ValidateOptions(options, validationResult);
12240

123-
var iops = parseResult.GetValueOrDefault<long>(ComputeOptionDefinitions.DiskIopsReadWrite.Name);
124-
options.DiskIopsReadWrite = iops > 0 ? iops : null;
125-
126-
var mbps = parseResult.GetValueOrDefault<long>(ComputeOptionDefinitions.DiskMbpsReadWrite.Name);
127-
options.DiskMbpsReadWrite = mbps > 0 ? mbps : null;
128-
129-
options.UploadType = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.UploadType.Name);
130-
131-
var uploadSizeBytes = parseResult.GetValueOrDefault<long>(ComputeOptionDefinitions.UploadSizeBytes.Name);
132-
options.UploadSizeBytes = uploadSizeBytes > 0 ? uploadSizeBytes : null;
133-
134-
options.SecurityType = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.SecurityType.Name);
41+
if (string.IsNullOrEmpty(options.Source) &&
42+
(options.SizeGb == null || options.SizeGb <= 0) &&
43+
string.IsNullOrEmpty(options.GalleryImageReference) &&
44+
string.IsNullOrEmpty(options.UploadType))
45+
{
46+
validationResult.Errors.Add("Either --source, --size-gb, --gallery-image-reference, or --upload-type must be specified.");
47+
}
13548

136-
return options;
137-
}
49+
if (!string.IsNullOrEmpty(options.UploadType) &&
50+
(options.UploadSizeBytes == null || options.UploadSizeBytes <= 0))
51+
{
52+
validationResult.Errors.Add("--upload-size-bytes is required when --upload-type is specified.");
53+
}
13854

139-
public override async Task<CommandResponse> ExecuteAsync(CommandContext context, ParseResult parseResult, CancellationToken cancellationToken)
140-
{
141-
if (!Validate(parseResult.CommandResult, context.Response).IsValid)
55+
if (string.Equals(options.UploadType, "UploadWithSecurityData", StringComparison.OrdinalIgnoreCase)
56+
&& string.IsNullOrEmpty(options.SecurityType))
14257
{
143-
return context.Response;
58+
validationResult.Errors.Add("--security-type is required when --upload-type is 'UploadWithSecurityData'.");
14459
}
60+
}
14561

146-
var options = BindOptions(parseResult);
62+
public override async Task<CommandResponse> ExecuteAsync(CommandContext context, DiskCreateOptions options, CancellationToken cancellationToken)
63+
{
14764
try
14865
{
14966
_logger.LogInformation(
15067
"Creating disk {DiskName} in resource group {ResourceGroup}, location {Location}, source {Source}",
151-
options.Disk, options.ResourceGroup, options.Location ?? "(default)", options.Source ?? "(none)");
68+
options.DiskName, options.ResourceGroup, options.Location ?? "(default)", options.Source ?? "(none)");
15269

15370
var disk = await _computeService.CreateDiskAsync(
154-
options.Disk!,
155-
options.ResourceGroup!,
71+
options.DiskName,
72+
options.ResourceGroup,
15673
options.Subscription!,
15774
options.Source,
15875
options.Location,
@@ -167,7 +84,7 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
16784
options.Tags,
16885
options.DiskEncryptionSet,
16986
options.EncryptionType,
170-
options.DiskAccessId,
87+
options.DiskAccess,
17188
options.Tier,
17289
options.GalleryImageReference,
17390
options.GalleryImageReferenceLun,
@@ -184,7 +101,7 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
184101
}
185102
catch (Exception ex)
186103
{
187-
_logger.LogError(ex, "Error creating disk. Disk: {Disk}, ResourceGroup: {ResourceGroup}.", options.Disk, options.ResourceGroup);
104+
_logger.LogError(ex, "Error creating disk. Disk: {Disk}, ResourceGroup: {ResourceGroup}.", options.DiskName, options.ResourceGroup);
188105
HandleException(context, ex);
189106
}
190107

‎tools/Azure.Mcp.Tools.Compute/src/Commands/Disk/DiskDeleteCommand.cs‎

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
using System.Net;
5-
using Azure.Mcp.Tools.Compute.Options;
4+
using Azure.Mcp.Core.Commands.Subscription;
5+
using Azure.Mcp.Core.Services.Azure.Subscription;
66
using Azure.Mcp.Tools.Compute.Options.Disk;
77
using Azure.Mcp.Tools.Compute.Services;
8+
using Microsoft.Extensions.Logging;
89
using Microsoft.Mcp.Core.Commands;
9-
using Microsoft.Mcp.Core.Extensions;
1010
using Microsoft.Mcp.Core.Models.Command;
11-
using Microsoft.Mcp.Core.Models.Option;
1211

1312
namespace Azure.Mcp.Tools.Compute.Commands.Disk;
1413

@@ -26,53 +25,30 @@ namespace Azure.Mcp.Tools.Compute.Commands.Disk;
2625
ReadOnly = false,
2726
Secret = true,
2827
LocalRequired = false)]
29-
public sealed class DiskDeleteCommand(
30-
ILogger<DiskDeleteCommand> logger,
31-
IComputeService computeService)
32-
: BaseComputeCommand<DiskDeleteOptions>(true)
28+
public sealed class DiskDeleteCommand(ILogger<DiskDeleteCommand> logger, IComputeService computeService, ISubscriptionResolver subscriptionResolver)
29+
: SubscriptionCommand<DiskDeleteOptions, DiskDeleteCommand.DiskDeleteCommandResult>(subscriptionResolver)
3330
{
34-
3531
private readonly ILogger<DiskDeleteCommand> _logger = logger ?? throw new ArgumentNullException(nameof(logger));
3632
private readonly IComputeService _computeService = computeService;
3733

38-
protected override void RegisterOptions(Command command)
39-
{
40-
base.RegisterOptions(command);
41-
command.Options.Add(ComputeOptionDefinitions.Disk.AsRequired());
42-
}
43-
44-
protected override DiskDeleteOptions BindOptions(ParseResult parseResult)
45-
{
46-
var options = base.BindOptions(parseResult);
47-
options.DiskName = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Disk.Name);
48-
return options;
49-
}
50-
51-
public override async Task<CommandResponse> ExecuteAsync(CommandContext context, ParseResult parseResult, CancellationToken cancellationToken)
34+
public override async Task<CommandResponse> ExecuteAsync(CommandContext context, DiskDeleteOptions options, CancellationToken cancellationToken)
5235
{
53-
if (!Validate(parseResult.CommandResult, context.Response).IsValid)
54-
{
55-
return context.Response;
56-
}
57-
58-
var options = BindOptions(parseResult);
59-
6036
try
6137
{
6238
_logger.LogInformation(
6339
"Deleting disk {DiskName} in resource group {ResourceGroup}",
6440
options.DiskName, options.ResourceGroup);
6541

6642
var deleted = await _computeService.DeleteDiskAsync(
67-
options.DiskName!,
68-
options.ResourceGroup!,
43+
options.DiskName,
44+
options.ResourceGroup,
6945
options.Subscription!,
7046
options.Tenant,
7147
options.RetryPolicy,
7248
cancellationToken);
7349

7450
context.Response.Results = ResponseResult.Create(
75-
new DiskDeleteCommandResult(deleted, options.DiskName!),
51+
new(deleted, options.DiskName),
7652
ComputeJsonContext.Default.DiskDeleteCommandResult);
7753
}
7854
catch (Exception ex)
@@ -86,14 +62,6 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
8662
return context.Response;
8763
}
8864

89-
protected override HttpStatusCode GetStatusCode(Exception ex) => ex switch
90-
{
91-
RequestFailedException reqEx => (HttpStatusCode)reqEx.Status,
92-
Identity.AuthenticationFailedException => HttpStatusCode.Unauthorized,
93-
ArgumentException => HttpStatusCode.BadRequest,
94-
_ => base.GetStatusCode(ex)
95-
};
96-
9765
protected override string GetErrorMessage(Exception ex) => ex switch
9866
{
9967
RequestFailedException reqEx when reqEx.Status == 409 =>
@@ -110,5 +78,5 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
11078
/// <summary>
11179
/// Result record for the disk delete command.
11280
/// </summary>
113-
public record DiskDeleteCommandResult(bool Deleted, string DiskName);
81+
public sealed record DiskDeleteCommandResult(bool Deleted, string DiskName);
11482
}

0 commit comments

Comments
 (0)