Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding Container Exec Spec #2584

Merged
merged 6 commits into from
Mar 13, 2018
Merged

Adding Container Exec Spec #2584

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7b12627
Adding exec api to swagger operation
Feb 26, 2018
a60b503
fixing small syntax error
Mar 7, 2018
1ec1e3f
Updating correct examples for async operation
Mar 7, 2018
24d6011
fixing ARM header field
Mar 7, 2018
9fecbb9
remvoing extra async operations
Mar 12, 2018
3388123
Update Python version
lmazuel Mar 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 203 additions & 0 deletions ...rce-manager/Microsoft.ContainerInstance/preview/2018-02-01-preview/containerInstance.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@
},
"201": {
"description": "Created - the container group is created.",
"headers": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason for this header, long running operation are handled by the x-ms-long-running-operation flag

Copy link
Contributor

@zhedahht zhedahht Mar 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we implemented this feature, we followed the following document about Azure Async Operations:
https://docs.microsoft.com/en-us/azure/azure-resource-manager/resource-manager-async-operations

It mentioned:

Azure-AsyncOperation - URL for checking the ongoing status of the operation. If your operation returns this value, always use it (instead of Location) to track the status of the operation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure you implemented the server the right way though :)
I'm saying that inside the Swagger, we have a special flag to say this is a ARM async operation and then generators do all this cumbersome work of headers and polling for free.
So you just use that flag, you define your return type with the final resource, and let Autorest deals with the rest.
Example:

azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/stable/2017-12-01/compute.json

Lines 403 to 462 in 8b34c73

"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/virtualMachines/{vmName}/extensions/{vmExtensionName}": {
"put": {
"tags": [
"VirtualMachineExtensions"
],
"operationId": "VirtualMachineExtensions_CreateOrUpdate",
"description": "The operation to create or update the extension.",
"parameters": [
{
"name": "resourceGroupName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the resource group."
},
{
"name": "vmName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the virtual machine where the extension should be create or updated."
},
{
"name": "vmExtensionName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the virtual machine extension."
},
{
"name": "extensionParameters",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/VirtualMachineExtension"
},
"description": "Parameters supplied to the Create Virtual Machine Extension operation."
},
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/VirtualMachineExtension"
}
},
"201": {
"description": "Created",
"schema": {
"$ref": "#/definitions/VirtualMachineExtension"
}
}
},
"x-ms-long-running-operation": true

"Azure-AsyncOperation": {
"description": "Operation Status Location URI",
"type": "string"
}
},
"schema": {
"$ref": "#/definitions/ContainerGroup"
}
Expand Down Expand Up @@ -206,6 +212,7 @@
{
"name": "Resource",
"description": "The container group resource with just the tags to be updated.",
"required": true,
"in": "body",
"schema": {
"$ref": "#/definitions/Resource"
Expand Down Expand Up @@ -314,6 +321,39 @@
}
}
},
"/subscriptions/{subscriptionId}/providers/Microsoft.ContainerInstance/locations/{location}/operations/{operationId}": {
"get": {
"operationId": "AsyncContainerGroupOperation_Get",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name seems unrelated to semantic of the operation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there is something I don't get in your service, if this is the operation to follow your previous async operation, you don't need this at all. If you follow ARM guidelines, this operation will be discovered by the "x-ms-long-running" flag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not need to add this to the swagger spec.

"x-ms-examples": {
"ContainerUsage": {
"$ref": "./examples/ContainerGroupUsage.json"
}
},
"description": "Get the usage for a subscription",
"parameters": [
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"$ref": "#/parameters/LocationParameter"
},
{
"$ref": "#/parameters/OperationIdParameter"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't understand what you'are trying to do here :(
Even the example "ContainerGroupUsage" doesn't have this information

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example was incorrect. The latest commit fixed this issue.

},
{
"$ref": "#/parameters/ApiVersionParameter"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AsyncOperation"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name seems unrelated to semantic of the operation.

}
}
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ContainerInstance/containerGroups/{containerGroupName}/containers/{containerName}/logs": {
"get": {
"operationId": "ContainerLogs_List",
Expand Down Expand Up @@ -360,6 +400,56 @@
}
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ContainerInstance/containerGroups/{containerGroupName}/containers/{containerName}/exec": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

action name exec can be more descriptive. launchExecCommand etc.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exec is the Kubernetes convention.

"post": {
"operationId": "StartContainer_Exec",
"x-ms-examples": {
"ContainerExecStart": {
"$ref": "./examples/ContainerExecStart.json"
}
},
"summary": "Starts the exec command for a specific container instance.",
"description": "Starts the exec command for a specified container instance in a specified resource group and container group.",
"parameters": [
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/ResourceGroupNameParameter"
},
{
"$ref": "#/parameters/ContainerGroupNameParameter"
},
{
"name": "containerName",
"in": "path",
"description": "The name of the container instance.",
"required": true,
"type": "string"
},
{
"name":"containerExecRequest",
"in":"body",
"description":"The request for the exec command.",
"required":true,
"schema":{
"$ref":"#/definitions/ContainerExecRequest"
}
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/ContainerExecResponse"
}
}
}
}
}
},
"definitions": {
Expand Down Expand Up @@ -1030,6 +1120,73 @@
}
}
},
"AsyncOperation": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the way we model long running operation at all. Please see for example:

azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/stable/2017-12-01/compute.json

Line 403 in 8b34c73

"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/virtualMachines/{vmName}/extensions/{vmExtensionName}": {

"type": "object",
"description": "Azure async operation status.",
"properties": {
"id": {
"type": "string",
"description": "Async operation id."
},
"status": {
"type": "string",
"description": "Async operation status."
},
"startTime": {
"type": "string",
"description": "The date time that the async operation started.",
"format": "date-time",
"readOnly": true
},
"properties": {
"$ref": "#/definitions/AsyncOperationProperties",
"description": "this structure contains the detailed properties of the operation."
},
"error": {
"$ref": "#/definitions/AzureResourceExtendedErrorInfo",
"description": "If the async operation fails, this structure contains the error details."
}
}
},
"AsyncOperationProperties": {
"type": "object",
"description": "The properties of the async operation",
"properties": {
"events": {
"readOnly": true,
"description": "The events of the async operation.",
"type": "array",
"items": {
"$ref": "#/definitions/Event"
}
}
}
},
"AzureResourceExtendedErrorInfo": {
"type": "object",
"description": "The error detail information for async operation",
"properties": {
"code": {
"type": "string",
"description": "The error code."
},
"target": {
"type": "string",
"description": "The error target."
},
"message": {
"type": "string",
"description": "The error message."
},
"details": {
"type": "array",
"description": "An array containing error information.",
"items": {
"$ref": "#/definitions/AzureResourceExtendedErrorInfo"
}
}
}
},
"ContainerGroupListResult": {
"description": "The container group list response that contains the container group properties.",
"type": "object",
Expand Down Expand Up @@ -1057,6 +1214,44 @@
}
}
},
"ContainerExecRequest": {
"description": "The start container exec request.",
"type": "object",
"properties": {
"command": {
"type": "string",
"description": "The command to be executed."
},
"terminalSize": {
"type": "object",
"description": "The size of the terminal.",
"properties": {
"row":{
"type": "integer",
"description": "The row size of the terminal"
},
"column": {
"type": "integer",
"description": "The column size of the terminal"
}
}
}
}
},
"ContainerExecResponse": {
"description": "The information for the container exec command.",
"type": "object",
"properties": {
"webSocketUri": {
"type": "string",
"description": "The uri for the exec websocket."
},
"password": {
"type": "string",
"description": "The password to start the exec command."
}
}
},
"Resource": {
"type": "object",
"description": "The Resource model definition.",
Expand Down Expand Up @@ -1131,6 +1326,14 @@
"type": "string",
"description": "The name of the container group.",
"x-ms-parameter-location": "method"
},
"OperationIdParameter": {
"name": "operationId",
"in": "path",
"required": true,
"type": "string",
"description": "The operation Id.",
"x-ms-parameter-location": "method"
}
}
}
24 changes: 24 additions & 0 deletions ...r/Microsoft.ContainerInstance/preview/2018-02-01-preview/examples/ContainerExecStart.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"parameters": {
"subscriptionId": "subid",
"api-version": "2018-02-01-preview",
"resourceGroupName": "demo",
"containerGroupName": "demo1",
"containerName": "container1",
"containerExecRequest": {
"command": "/bin/bash",
"terminalSize": {
"row": 12,
"column": 12
}
}
},
"responses": {
"200": {
"body": {
"webSocketUri": "wss://web-socket-uri",
"password": "password"
}
}
}
}