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.

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

increase default instance root volume size #2847

Merged

Conversation

chrislovecnm
Copy link
Contributor

@chrislovecnm chrislovecnm commented Jun 30, 2017

Fixing #1929

masters will default 50 GB
nodes will default to 100 GB

50 and 100 just seemed weird ... doing 64 and 128 next rebase ...


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 30, 2017
Copy link
Member

@geojaz geojaz left a comment

Choose a reason for hiding this comment

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

sorely needed. good stuff lgtm, just fix the tests for volume size :)

@justinsb
Copy link
Member

justinsb commented Jul 1, 2017

I was thinking "we can't change the defaults", but I guess we can actually in a major release if we think it is a better default, and we have a release note that calls it out clearly.

i.e. not specifying means "omakase" and we can change things as long as the change is clearly signposted and unlikely to actively cause problems.

In this case, the only damage is finanical. It is $10 per month instead of $2 per month, per instance, so we should explain it in the release notes, but if you & geojaz both think it a better default, it works for me.

@justinsb
Copy link
Member

justinsb commented Jul 1, 2017

Oh, and the three benefits from a bigger disk:

  • more disk space (duh!)
  • more inodes (because of the filesystem)
  • more iops (because of the gp2 instance type)

A potential downside is that a bigger disk might mean more scanning, and we've seen reports of that getting slow, but I think we need to address that anyway (in k8s/k8s) if it turns out to be an issue.

@justinsb
Copy link
Member

justinsb commented Jul 1, 2017

lgtm once tests passing. If you want to start a "release-notes-1.7" doc alongside the 1.6 one as well that would be great :-)

@chrislovecnm
Copy link
Contributor Author

The one time I don't run ci - heh

@chrislovecnm chrislovecnm force-pushed the increase-default-volume-size branch from 933a86d to 1f3212c Compare July 5, 2017 02:21
@chrislovecnm
Copy link
Contributor Author

chrislovecnm commented Jul 5, 2017

@justinsb / @geojaz - I increase the size to 128 nodes, 64 master, 32 bastion. Having odd HD sizes like 50 and 100 was hurting my head. CI is passing.

Copy link
Member

@justinsb justinsb left a comment

Choose a reason for hiding this comment

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

One doc change needed then lgtm (i.e. self merge after doc change)

## Changing the root volume size or type

The default volume size for Masters is 50 GB, while the default volume size for a node is 100 GB.
Copy link
Member

Choose a reason for hiding this comment

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

64/128

@justinsb
Copy link
Member

justinsb commented Jul 6, 2017

Merging, will send doc fix.

@justinsb justinsb merged commit 2222111 into kubernetes:master Jul 6, 2017
@justinsb
Copy link
Member

justinsb commented Jul 6, 2017

Doc fix in #2873

@chrislovecnm chrislovecnm deleted the increase-default-volume-size branch December 30, 2017 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants