Skip to content

[4.16] systemvm: configurable root disk size based on new systemvm template definition - #3900

Closed
ustcweizhou wants to merge 2 commits into
apache:masterfrom
ustcweizhou:4.14-systemvm-root-disk-size
Closed

[4.16] systemvm: configurable root disk size based on new systemvm template definition#3900
ustcweizhou wants to merge 2 commits into
apache:masterfrom
ustcweizhou:4.14-systemvm-root-disk-size

Conversation

@ustcweizhou

@ustcweizhouustcweizhou commented Feb 20, 2020

Copy link
Copy Markdown
Contributor

Description

The official systemvm template has very few free spaces, it would be better to make the disk size configuration.

Main changes in systemvm template
(1) remove /var and /tmp partitions
(2) move / partition to the last.
(3) install cloud-guest-utils to support "growpart"
(4) install haproxy 1.8 from stretch-backports instead of haproxy 1.7

other changes
(3) add global setting systemvm.root.disk.size
(4) when systemvm starts, allocate all free spaces on disk to the extend partition, root partition (/dev/vda6) and file system.

This applies on all systemvms (cpvm,ssvm,virtual routers, etc)

This fixes#3455

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

  1. build new systemvm template
  2. register new systemvm template
  3. change type from "USER" to "SYSTEM"
  4. change global setting systemvm.root.disk.size to 3GB
  5. destroy ssvm or cpvm
  6. new ssvm/cpvm will have 3GB disk (default 1.5GB) and 2.5 GB free space (default 1GB) on filesystem.

old VR

root@r-269-VM:~# df
Filesystem 1K-blocks Used Available Use% Mounted on
udev 103748 0 103748 0% /dev
tmpfs 47352 23480 23872 50% /run
/dev/vda5 2086316 1209892 860040 59% /
tmpfs 118372 0 118372 0% /dev/shm
tmpfs 5120 0 5120 0% /run/lock
tmpfs 118372 0 118372 0% /sys/fs/cgroup
/dev/vda1 93207 46230 46977 50% /boot
tmpfs 23672 0 23672 0% /run/user/0

new VR

root@r-270-VM:~# df
Filesystem 1K-blocks Used Available Use% Mounted on
udev 103688 0 103688 0% /dev
tmpfs 47352 5556 41796 12% /run
/dev/vda6 2687084 970912 1699788 37% /
tmpfs 118372 0 118372 0% /dev/shm
tmpfs 5120 0 5120 0% /run/lock
tmpfs 118372 0 118372 0% /sys/fs/cgroup
/dev/vda1 93207 46361 46846 50% /boot
tmpfs 23672 0 23672 0% /run/user/0

@ustcweizhou

Copy link
Copy Markdown
ContributorAuthor

@rhtyd this has some small conflicts with your PR #3797

@weizhouapache

Copy link
Copy Markdown
Member

@rhtyd
if the specified value is small than template size, then it will not be used. "-1" and "0" makes no difference actually.

 Long size = _tmpltMgr.getTemplateSize(template.getId(), vm.getDataCenterId());
if (rootDisksize != null) {
rootDisksize = rootDisksize * 1024 * 1024 * 1024;
if (rootDisksize > size) {
s_logger.debug("Using root disk size of " + rootDisksize + " Bytes for volume " + name);
size = rootDisksize;
} else {
s_logger.debug("Using root disk size of " + size + " Bytes for volume " + name + "since specified root disk size of " + rootDisksize + " Bytes is smaller than template");
}
}

https://github.com/apache/cloudstack/blob/master/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java#L713-L722

@DaanHoogland

Copy link
Copy Markdown
Contributor

hm, I think you want milestone 4.14 for this one as well, right @weizhouapache ?

@weizhouapacheweizhouapache changed the title 4.14 systemvm root disk sizesystemvm: configurable root disk size based on new systemvm template definitionFeb 20, 2020
@weizhouapache

Copy link
Copy Markdown
Member

hm, I think you want milestone 4.14 for this one as well, right @weizhouapache ?

@DaanHoogland I expect to add to 4.14. However, it require to build new systemvm template (and test it).
4.15 is also good for me.

@yadvr

Copy link
Copy Markdown
Member

Thanks per your comment assigned against 4.15 @weizhouapache

@yadvr

Copy link
Copy Markdown
Member

@ustcweizhou do we want this for master/4.15+? Can you fix conflicts?

@PaulAngus

Copy link
Copy Markdown
Member

bump @ustcweizhou - can you fix conflicts so we can progress into testing please.

DaanHoogland
DaanHoogland previously approved these changes Jul 29, 2020

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

code looks good, we need this integration tested on the new release (which will happen automatically 🤞 )

@yadvryadvr removed this from the 4.15.0.0 milestone Jul 30, 2020
@weizhouapache

Copy link
Copy Markdown
Member

@rhtyd @DaanHoogland any chance to get this into 4.15 ?


Long rootDiskSize = rootDiskOfferingInfo.getSize();
if (vm.getType().isUsedBySystem() && SystemVmRootDiskSize.value() != null) {
rootDiskSize = SystemVmRootDiskSize.value();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ustcweizhou should there be a check for > 0 ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rhtyd added check.

if which growpart > /dev/null; then
growpart /dev/vda 2
growpart /dev/vda 6
resize2fs /dev/vda6

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ustcweizhou have you tested this for KVM, and non-KVM hypervisors? It's possible the partitions are not called (hard-coded) vda; instead of this can be find/use partitions by mount point names or other valid/static identifiers?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is screenshot of cpvm on xenserver for example:
Screenshot from 2020-09-29 10-58-27

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Screenshot of partitions of cpvm on kvm:
Screenshot from 2020-09-29 11-01-32

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Screenshot of partitions of cpvm, on vmware:
Screenshot from 2020-09-29 11-03-39

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ustcweizhou have you tested this for KVM, and non-KVM hypervisors? It's possible the partitions are not called (hard-coded) vda; instead of this can be find/use partitions by mount point names or other valid/static identifiers?

@rhtyd good point. thanks for review

echo "" > /root/.ssh/known_hosts

if which growpart > /dev/null; then
growpart /dev/vda 2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I checked the 4.15 systemvmtemplate, growpart is not available but resize2fs is available. Can we use fdisk instead of growpart to resize partition instead, or some other alternative? This would save from creating a new systemvmtemplate as we get closer to the 4.15 release?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rhtyd root partition must be the last partition (it is linux swap for now), so this feature requires new systemvm template. let's move to 4.16.
I will work on other hypervisors except kvm.

@yadvr

Copy link
Copy Markdown
Member

@ustcweizhou I've left some review comments, please see. As we're getting closer to 4.15 release I would like to avoid anything that could add any regression, usually a systemvmtemplate change could potentially add regressions.

A new Debian 10.6 was released over the weekend so if we build a systemvmtemplate now there may be several differences due to pkg/kernel changes against the one we've been testing for months - http://download.cloudstack.org/systemvm/4.15/

@weizhouapache

Copy link
Copy Markdown
Member

@ustcweizhou I've left some review comments, please see. As we're getting closer to 4.15 release I would like to avoid anything that could add any regression, usually a systemvmtemplate change could potentially add regressions.

A new Debian 10.6 was released over the weekend so if we build a systemvmtemplate now there may be several differences due to pkg/kernel changes against the one we've been testing for months - http://download.cloudstack.org/systemvm/4.15/

@rhtyd ok, then let's not merge this into 4.15
I will fix the issues, hopefully this can be merged into 4.16

@yadvryadvr added this to the 4.16.0.0 milestone Oct 7, 2020
@weizhouapacheweizhouapache changed the title systemvm: configurable root disk size based on new systemvm template definition[DO NOT MERGE] systemvm: configurable root disk size based on new systemvm template definitionOct 30, 2020
@DaanHoogland
DaanHoogland dismissed their stale reviewOctober 30, 2020 09:12

old code, needs reconsidering

@ustcweizhou
ustcweizhouforce-pushed the 4.14-systemvm-root-disk-size branch from 112c8ab to 45828e9CompareJanuary 17, 2021 09:23
@weizhouapacheweizhouapache changed the title [DO NOT MERGE] systemvm: configurable root disk size based on new systemvm template definition[4.16] systemvm: configurable root disk size based on new systemvm template definitionJan 19, 2021

Long rootDiskSize = rootDiskOfferingInfo.getSize();
if (vm.getType().isUsedBySystem() && SystemVmRootDiskSize.value() != null) {
rootDiskSize = SystemVmRootDiskSize.value();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rhtyd added check.

@weizhouapache

Copy link
Copy Markdown
Member

ignore my approval ...

@yadvr

yadvr commented Mar 23, 2021

Copy link
Copy Markdown
Member

@weizhouapache I think @Pearl1594 has already included these changes in her #4329 PR

@weizhouapache

Copy link
Copy Markdown
Member

@weizhouapache I think @Pearl1594 has already included these changes in her #4329 PR

@rhtyd good. I closed this ticket then.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configurable disk size of systemvm

7 participants

@ustcweizhou@weizhouapache@DaanHoogland@yadvr@PaulAngus@svenvogel@ACSGitBot