-
Notifications
You must be signed in to change notification settings - Fork 70
Add disks
field to LKENodePoolRequestBody Schema
#283
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.
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
Add disks
field to LKENodePoolRequestBody Schema
#283
Conversation
leslitagordita
commented
Jul 9, 2020
- Add disks field to LKENodePoolRequestBody schema
- This update affects the following endpoints:
- POST /lke/clusters
- POST /lke/clusters/{clusterId}/pools
- Updated Shell and CLI examples for each endpoint.
This update affects the following endpoints: POST /lke/clusters POST /lke/clusters/{clusterId}/pools Updated Shell and CLI examples for each endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to your question; disks
is not a convenience field in the sense that its effect can be achieved through other public methods. it's an advanced feature that most users would not find useful, but it does open up some very interesting deployment options, like deploying rook on LKE w/ local storage.
openapi.yaml
Outdated
"count": 6, | ||
"disks": [ | ||
{ | ||
"size": "1028", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1028 should be an int, not a string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
openapi.yaml
Outdated
"count": 6, | ||
"disks": [ | ||
{ | ||
"size": "1028", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
openapi.yaml
Outdated
linode-cli lke pool-create 12345 \ | ||
--type g6-standard-4 \ | ||
--count 6 \ | ||
--disks '[{"size": "1028", "type": "ext4"}]' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
openapi.yaml
Outdated
- 2048 | ||
type: | ||
description: > | ||
This custome disk partition's filesystem type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
custom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
openapi.yaml
Outdated
properties: | ||
size: | ||
description: > | ||
The size of this custom disk partition in MB. The size of this disk partition must not exceed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't bother noting the sum of sizes validation, the API validation errors will make it clear. it's a bit complex to describe accurately, since there's a minimum necessary slice we reserve for the boot disk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking about this more; it's worth noting somewhere that whatever disk is leftover once the requested disks are allocated is allocated to the boot disk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added note about boot disk allocation: 347c6a1#diff-fe030a7c1568b3decf599edf399be7f4R16052
Update CLI example size to use integer value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 Strongly recommend including the point in my comment above
Rewrite the way response schema is written to reference individual properties instead of specific schema