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

Implementing agentpool API #5352

Merged
merged 8 commits into from
Mar 15, 2019
Merged

Implementing agentpool API #5352

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6ac7354
copy 2018-08-01-preview folder to 2019-02-01 without changes
Feb 4, 2019
bed90f3
implementing agentpool api in 2019-02-01
Feb 4, 2019
01448f8
fixing minor api-version errors
Feb 5, 2019
5d5bd63
fixing the agentpool list operation paths
Feb 5, 2019
cda2833
support agentpools api
Mar 9, 2019
9b8b68c
fixing the agentpool example
Mar 13, 2019
ed7db73
add default response
Mar 15, 2019
db67eaa
including name validation
Mar 15, 2019
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
47 changes: 47 additions & 0 deletions ...anager/Microsoft.ContainerService/stable/2019-02-01/examples/AgentPoolsCreate_Update.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"parameters": {
"api-version": "2019-02-01",
"subscriptionId": "subid1",
"resourceGroupName": "rg1",
"managedClusterName": "clustername1",
"agentPoolName":"agentpool1",
"parameters": {
"properties": {
"orchestratorVersion": "",
"count": 3,
"vmSize": "Standard_DS1_v2",
"osType": "Linux"
}
}
},
"responses": {
"200": {
"body": {
"id": "/subscriptions/subid1/resourcegroups/rg1/providers/Microsoft.ContainerService/managedClusters/clustername1/agentPools/agentpool1",
xizha162 marked this conversation as resolved.
Show resolved Hide resolved
"name": "nodepool1",
"properties": {
"provisioningState": "Succeeded",
"orchestratorVersion": "1.9.6",
"count": 3,
"vmSize": "Standard_DS1_v2",
"maxPods": 110,
"osType": "Linux"
}
}
},
"201": {
"body": {
"id": "/subscriptions/subid1/resourcegroups/rg1/providers/Microsoft.ContainerService/managedClusters/clustername1/agentPools/agentpool1",
"name": "nodepool1",
"properties": {
"provisioningState": "Creating",
"orchestratorVersion": "1.9.6",
"count": 3,
"vmSize": "Standard_DS1_v2",
"maxPods": 110,
"osType": "Linux"
}
}
}
}
}
13 changes: 13 additions & 0 deletions ...ource-manager/Microsoft.ContainerService/stable/2019-02-01/examples/AgentPoolsDelete.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"parameters": {
"api-version": "2019-02-01",
"subscriptionId": "subid1",
"resourceGroupName": "rg1",
"managedClusterName": "clustername1",
"agentPoolName":"agentpool1"
},
"responses": {
"202": {},
"204": {}
}
}
26 changes: 26 additions & 0 deletions ...resource-manager/Microsoft.ContainerService/stable/2019-02-01/examples/AgentPoolsGet.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"parameters": {
"api-version": "2019-02-01",
"subscriptionId": "subid1",
"resourceGroupName": "rg1",
"managedClusterName": "clustername1",
"agentPoolName":"agentpool1"
},
"responses": {
"200": {
"body": {
"id": "/subscriptions/subid1/resourcegroups/rg1/providers/Microsoft.ContainerService/managedClusters/clustername1/agentPools/agentpool1",
"name": "agentpool1",
"properties": {
"provisioningState": "Succeeded",
"count": 3,
"vmSize": "Standard_DS1_v2",
"maxPods": 110,
"osType": "Linux",
"orchestratorVersion": "1.9.6"
}
}
}
}
}

29 changes: 29 additions & 0 deletions ...esource-manager/Microsoft.ContainerService/stable/2019-02-01/examples/AgentPoolsList.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"parameters": {
"api-version": "2019-02-01",
"subscriptionId": "subid1",
"resourceGroupName": "rg1",
"managedClusterName": "clustername1"
},
"responses": {
"200": {
"body": {
"value": [
{
"id": "/subscriptions/subid1/resourcegroups/rg1/providers/Microsoft.ContainerService/managedClusters/clustername1/agentPools/agentpool1",
"name": "agentpool1",
"properties": {
"provisioningState": "Succeeded",
"count": 3,
"vmSize": "Standard_DS1_v2",
"maxPods": 110,
"osType": "Linux",
"orchestratorVersion": "1.9.6"
}
}
]
}
}
}
}

221 changes: 221 additions & 0 deletions ...ervice/resource-manager/Microsoft.ContainerService/stable/2019-02-01/managedClusters.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,227 @@
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ContainerService/managedClusters/{managedClusterName}/agentPools": {
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 one major problem in these new APIs that are being added now. And yes we reviewed them in the past but never reached a conclusion on the optimal design. The problem is as follows -
Currently, before these new APIs, agentPools is modeled as a property on the managedClusters resource. Its modeled as an array. Now, you are also modeling agentPools as a child resource under managedClusters.
First, child resource payload MUST not be included in the parent resource. It makes PUT very dangerous and few big customers have recently been bitten by this inconsistent PUT behavior in Azure today. Modeling child resources in the parent can potentially make it that much more destructive. If you need a reference to the child resource in the parent, please do so using just ARM resource Ids.
Second, It's unclear how will the PUT on managedCluster work in this case. As part of PUT, can one or more agentPool resources be created?
Third, if you allow multiple to be created, how would subsequent PUTs behave if they dont include an agent Pool which was included in the array and created in the first PUT.
Fourth, such modeling can also potentially break RBAC. If someone was explicitly denied read at the child scope (for whatever reason), they can still read all child resource properties by having read access at the parent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to clarify - the agentPools are proxy only resource, it's not tracked by ARM. Consider it's an API to patch the array in managedClusters. This is similar to Microsoft.Network/virtualNetworks/subnets. I hope this address the first concern.
Regarding the second, as part of PUT managedCluster, client can put multiple agentPools in the array.
Regarding the third, the current PUT API behaves as patch, so if a subsequent PUT doesn't include the agentpool in array, it's considered a patch to the array. In implementation it's a merge of the array, and the reason is that the agentPools is relatively heavy and we want the deletion to be more explicit.
Regarding the fourth the scenario doesn't really apply. Cluster is the minimum unit to manage AKS, if one can read cluster, they should see agentPools. This is similar to vnet/subnet case.

hope this help address the concerns?

Copy link
Contributor

Choose a reason for hiding this comment

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

we call the behavior for 'third' as 'PUTCH' (unofficial term :)) as PUT is behaving like PATCH and it is not recommended.

"get": {
"tags": [
"AgentPools"
],
"operationId": "AgentPools_List",
"summary": "Gets a list of agent pools in the specified managed cluster.",
"description": "Gets a list of agent pools in the specified managed cluster. The operation returns properties of each agent pool.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"name": "resourceGroupName",
"in": "path",
"required": true,
"type": "string",
"minLength": 1,
"description": "The name of the resource group."
},
{
"name": "managedClusterName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the managed cluster resource."
}
],
"responses": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add "default" response to all the API responses. This would capture the error state and the schema must match the error schema as described in ARM RPC. See Batch RP swagger for example

Copy link
Contributor Author

@xizha162 xizha162 Mar 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Good point. Fixed. Added default response.

"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AgentPoolListResult"
}
}
},
"x-ms-pageable": {
"nextLinkName": "nextLink"
},
"x-ms-examples": {
"List Agent Pools by Managed Cluster": {
"$ref": "./examples/AgentPoolsList.json"
}
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ContainerService/managedClusters/{managedClusterName}/agentPools/{agentPoolName}": {
"get": {
"tags": [
"AgentPools"
],
"operationId": "AgentPools_Get",
"summary": "Gets the agent pool.",
"description": "Gets the details of the agent pool by managed cluster and resource group.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"name": "resourceGroupName",
"in": "path",
"required": true,
"type": "string",
"minLength": 1,
"description": "The name of the resource group."
},
{
"name": "managedClusterName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the managed cluster resource."
},
{
"name": "agentPoolName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the agent pool."
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AgentPool"
}
}
},
"x-ms-examples": {
"Get Agent Pool": {
"$ref": "./examples/AgentPoolsGet.json"
}
}
},
"put": {
"tags": [
"AgentPools"
],
"operationId": "AgentPools_CreateOrUpdate",
"summary": "Creates or updates an agent pool.",
"description": "Creates or updates an agent pool in the specified managed cluster.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"name": "resourceGroupName",
"in": "path",
"required": true,
"type": "string",
"minLength": 1,
"description": "The name of the resource group."
},
{
"name": "managedClusterName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the managed cluster resource."
},
{
"name": "agentPoolName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the agent pool."
},
{
"name": "parameters",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/AgentPool"
},
"description": "Parameters supplied to the Create or Update an agent pool operation."
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AgentPool"
}
},
"201": {
"description": "Created",
"schema": {
"$ref": "#/definitions/AgentPool"
}
}
},
"x-ms-long-running-operation": true,
"x-ms-examples": {
"Create/Update Agent Pool": {
"$ref": "./examples/AgentPoolsCreate_Update.json"
}
}
},
"delete": {
"tags": [
"AgentPools"
],
"operationId": "AgentPools_Delete",
"summary": "Deletes an agent pool.",
"description": "Deletes the agent pool in the specified managed cluster.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"name": "resourceGroupName",
"in": "path",
"required": true,
"type": "string",
"minLength": 1,
"description": "The name of the resource group."
},
{
"name": "managedClusterName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the managed cluster resource."
},
{
"name": "agentPoolName",
"in": "path",
"required": true,
"type": "string",
"description": "The name of the agent pool."
}
],
"responses": {
"202": {
"description": "Accepted"
},
"204": {
"description": "NoContent"
}
},
"x-ms-long-running-operation": true,
"x-ms-examples": {
"Delete Agent Pool": {
"$ref": "./examples/AgentPoolsDelete.json"
}
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ContainerService/managedClusters/{resourceName}/resetServicePrincipalProfile": {
"post": {
"tags": [
Expand Down
4 changes: 2 additions & 2 deletions specification/containerservice/resource-manager/readme.python.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ Please also specify `--python-sdks-folder=<path to the root directory of your az

``` yaml $(tag) == 'package-2019-02-only' && $(python)
python:
namespace: azure.mgmt.containerservice.v2019_02_01_preview
output-folder: $(python-sdks-folder)/azure-mgmt-containerservice/azure/mgmt/containerservice/v2019_02_01_preview
namespace: azure.mgmt.containerservice.v2019_02_01
output-folder: $(python-sdks-folder)/azure-mgmt-containerservice/azure/mgmt/containerservice/v2019_02_01
```

### Tag: package-2018-09-preview-only and python
Expand Down