multi local storage handling for kvm

[DK101010]

Description

Is currently a POC, if someone have additional ideas or hints.

Motivation
Users sometimes only have the option to use local storages, so they should have the option to use local storage like other storage types.

Implementation
Decided to use already existing code to prevent breaks in current behavior(adapted agent code by user to add local storage)
With this implementation it is possible to add and use local storage like other storage types.

Key properties to add a local storage
Scope: HOST
Protocol: Filesytem
Path: mount path in KVM

Condition:
Enable local storage for User VMs = true in zone detail settings

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Currently only tested in own environment

[weizhouapache]

@DK101010

multiple local storage is supported since #6147
can you test it ?

[DK101010]

@DK101010

multiple local storage is supported since #6147 can you test it ?

@weizhouapache Damn, the last time I checked the PRs, I didn't see that one.... never mind

Hmm ... you implemented this via agent properties and my implementation works via UI like the other storages.
For user acceptance reasons I prefer my solution. User can use local storages like other storage types without touch agent properties. Also no restarts required.
Excuse me for insisting, but It will be hard to explain my users that they have to manipulate the agent properties instead to use the ui to add local storages but for other storages they have to use the ui. :/

Perhaps we find a compromise. But I'm afraid that our implementations are disjunct. Do you have an idea?

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[weizhouapache]

@DK101010
multiple local storage is supported since #6147 can you test it ?

@weizhouapache Damn, the last time I checked the PRs, I didn't see that one.... never mind

Hmm ... you implemented this via agent properties and my implementation works via UI like the other storages. For user acceptance reasons I prefer my solution. User can use local storages like other storage types without touch agent properties. Also no restarts required. Excuse me for insisting, but It will be hard to explain my users that they have to manipulate the agent properties instead to use the ui to add local storages but for other storages they have to use the ui. :/

Perhaps we find a compromise. But I'm afraid that our implementations are disjunct. Do you have an idea?

@DK101010

The reason I choose agent properties is, this can only be operated by administrator. they can do it by automation tools like chef, ansible, etc.

agree with you it would be nice to support it via api and UI, it will be more user-friendly. for example
(1) add host with multiple local storage pools on UI
(2) list all local storage pools of a host on UI (of course API as well)
(3) add local storage pool to a host (create libvirt storage pool, update db and agent.properties)
(4) remove local storage from a host (remove libvirt storage pool, update db and agent.properties)

1 is optional if 2,3,4 are supported. we can start by adding a host with only 1 local storage pool.

[DK101010]

@weizhouapache

I would also prefer to ignore number 1.
For everything else, I can adapt the source code.

But I still have two questions.
(on point 3. and 4. - create/remove libvirt storage pool )
Does it make sense that Cloudstack can create local storage? I had thought about that, but I came to the conclusion that it would be a break in logic. As far as I know (KVM, VMware) cloudstack does not create storages. CS only adds them to its own infrastructure.

Why do the agent properties have to be adapted?
My PR works completely without the agent properties and so far I have not noticed any errors that would require an adjustment in the agent properties.
I can add/remove local storages. Create/delete/migrate VM's.

[weizhouapache]

@weizhouapache

I would also prefer to ignore number 1. For everything else, I can adapt the source code.

But I still have two questions. (on point 3. and 4. - create/remove libvirt storage pool ) Does it make sense that Cloudstack can create local storage? I had thought about that, but I came to the conclusion that it would be a break in logic. As far as I know (KVM, VMware) cloudstack does not create storages. CS only adds them to its own infrastructure.

Why do the agent properties have to be adapted? My PR works completely without the agent properties and so far I have not noticed any errors that would require an adjustment in the agent properties. I can add/remove local storages. Create/delete/migrate VM's.

@DK101010
when cloudstack agent is started or restarted, it will
(1) read the storage pools (uuid and path) in agent.properties
(2) create libvirt storage pools (not the directory itself) if not exist.

you can check createLocalStoragePool method in
https://github.com/apache/cloudstack/blob/main/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java#L3419

[weizhouapache]

@blueorangutan package

[blueorangutan]

@weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 4284

[weizhouapache]

@DK101010
when the coding is done, please mark this "Ready for review"

[DK101010]

@DK101010 when the coding is done, please mark this "Ready for review"

@weizhouapache it isn't done. Currently I try to find out, why it is important to use the agent.properties file instead to create entries directly in the db via rest api.
Also open the libvirt part at these points. (Until now it is possible to attach, detach storages)
(3) add local storage pool to a host (create libvirt storage pool, update db and agent.properties)
(4) remove local storage from a host (remove libvirt storage pool, update db and agent.properties)

[shwstppr]

@blueorangutan package

[blueorangutan]

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7675

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[blueorangutan]

[SF] Trillian test result (tid-8276)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45281 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6699-t8276-kvm-centos7.zip
Smoke tests completed. 115 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	Failure	682.70	test_kubernetes_clusters.py
test_08_migrate_vm	Error	0.07	test_vm_life_cycle.py

[rajujith]

@DK101010 Unfortunately I do not see an option to select the 'filesystem' protocol. Neither do I get an option to add via API. 'Enable local storage for User Instances' is enabled. Restarted the management server as well.

[DK101010]

@DK101010 Unfortunately I do not see an option to select the 'filesystem' protocol. Neither do I get an option to add via API. 'Enable local storage for User Instances' is enabled. Restarted the management server as well.

Strange, we use it already in our environment without problems. hmm ... I will check this again next week.

[DaanHoogland]

@DK101010 Unfortunately I do not see an option to select the 'filesystem' protocol. Neither do I get an option to add via API. 'Enable local storage for User Instances' is enabled. Restarted the management server as well.

Strange, we use it already in our environment without problems. hmm ... I will check this again next week.

@DK101010 , is there maybe a prereq procedure that needs documentation? like installing/enabling a provider and or setting global configurations.

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[DK101010]

@rajujith @DaanHoogland many thanks for testing. The problem was that the filesystem protocol is dependent on scope and zone. If the zone was not selected or the zone was selected before the scope, the protocol is not updated. With the new scope event it should now also work for you

[blueorangutan]

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7731

[rajujith]

LGTM.
Steps I followed:
Enable 'Enable local storage for User Instances' at zone.

Create Local storage:

1. Create a directory in one of the KVM host.
2. Click on add primary storage in the cloudstack UI. 
3. Fill the form selecting the scope as 'HOST" and the above host. Selected the protocol 'Filesystem'.
4. Input the directory path from the above step 1. 

If the directory is not present the primary storage addition fails, I believe this is expected.
Deleted the above primary storage, the primary storage is not visible anymore in the UI. The directory remains in the KVM host filesystem.
Verified volume deployments, and migrations.

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[DaanHoogland]

@JoaoJandre can you approve now?

[shwstppr]

Code LGTM

[DaanHoogland]

api-rate limit again:
Smoke tests completed. 115 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_trigger_shutdown	Failure	336.69	test_safe_shutdown.py
test_02_upgrade_kubernetes_cluster	Failure	551.92	test_kubernetes_clusters.py

I think we can call this a success

[JoaoJandre]

Code LGTM

[andrijapanicsb]

Whoever is reviewing the PR, can we please try to ensure that the proper documentation is also provided for a feature? I'll add the related docs for this one. Thanks.