You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/01/27 03:55:31 UTC
[GitHub] [cloudstack] bwsw opened a new pull request #3839: FEATURE-3823:
kvm agent hooks
bwsw opened a new pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839
## Description
<!--- Describe your changes in detail -->
The PR introduces the new KVM agent extension interface in the form of hooks. Every hook is implemented in Groovy like
```
package groovy
class BaseTransform {
String transform(Object logger, String xml) {
File file = new File("/tmp/hooks.txt")
file.append("------------------------------------------------------------------------\n")
file.append(xml)
return xml
}
}
new BaseTransform()
```
There are 3 hooks are implemented now:
1. `XML transformer` which is called right before VM is launched and it allows modifying XML somehow (example above)
2. `onStart` and `onStop` hooks which are called right after VM state changes to started/stopped.
```
package groovy
class StateHandler {
Object onStart(Object logger, String vmName) {
File file = new File("/tmp/hooks-start.txt")
file.append(vmName)
return null
}
Object onStop(Object logger, String vmName) {
File file = new File("/tmp/hooks-stop.txt")
file.append(vmName)
return null
}
}
new StateHandler()
```
Every hook is run inside `try {} catch (Exception e) {}`, so it can not cause agent misbehavior. If hooks are not defined, they are skipped.
Also, the PR adds initial support for GitLab CICD for those, who does WIPs with GitLab, not in GitHub.
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
Initial RFC/Proposal: https://github.com/apache/cloudstack/issues/3823
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] 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?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Tested in unit tests and manually for master.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r386049212
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java
##########
@@ -92,6 +93,13 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource
libvirtComputingResource.destroyNetworkRulesForVM(conn, vmName);
final String result = libvirtComputingResource.stopVM(conn, vmName, command.isForceStop());
+ try {
+ LibvirtKvmAgentHook onStopHook = libvirtComputingResource.getStopHook();
+ onStopHook.handle(vmName);
+ } catch (Exception e) {
+ s_logger.warn("Exception occured when handling LibVirt VM onStop hook: {}", e);
+ }
Review comment:
ping, did you manage?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r383843836
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -1065,6 +1089,48 @@ public boolean configure(final String name, final Map<String, Object> params) th
value = (String) params.get("vm.migrate.pauseafter");
_migratePauseAfter = NumbersUtil.parseInt(value, -1);
+ value = (String) params.get("agent.hooks.basedir");
+ if (null != value) {
+ _agentHooksBasedir = value;
+ }
+ s_logger.debug("agent.hooks.basedir is " + _agentHooksBasedir);
+
+ value = (String) params.get("agent.hooks.libvirt_vm_xml_transformer.script");
+ if (null != value) {
+ _agentHooksLibvirtXmlScript = value;
+ }
+ s_logger.debug("agent.hooks.libvirt_vm_xml_transformer.script is " + _agentHooksLibvirtXmlScript);
+
+ value = (String) params.get("agent.hooks.libvirt_vm_xml_transformer.method");
+ if (null != value) {
+ _agentHooksLibvirtXmlMethod = value;
+ }
+ s_logger.debug("agent.hooks.libvirt_vm_xml_transformer.method is " + _agentHooksLibvirtXmlMethod);
+
+ value = (String) params.get("agent.hooks.libvirt_vm_on_start.script");
+ if (null != value) {
+ _agentHooksVmOnStartScript = value;
+ }
+ s_logger.debug("agent.hooks.libvirt_vm_on_start.script is " + _agentHooksVmOnStartScript);
+
+ value = (String) params.get("agent.hooks.libvirt_vm_on_start.method");
+ if (null != value) {
+ _agentHooksVmOnStartMethod = value;
+ }
+ s_logger.debug("agent.hooks.libvirt_vm_on_start.method is " + _agentHooksVmOnStartMethod);
+
+ value = (String) params.get("agent.hooks.libvirt_vm_on_stop.script");
+ if (null != value) {
+ _agentHooksVmOnStopScript = value;
+ }
+ s_logger.debug("agent.hooks.libvirt_vm_on_stop.script is " + _agentHooksVmOnStopScript);
+
+ value = (String) params.get("agent.hooks.libvirt_vm_on_stop.method");
+ if (null != value) {
+ _agentHooksVmOnStopMethod = value;
+ }
+ s_logger.debug("agent.hooks.libvirt_vm_on_stop.method is " + _agentHooksVmOnStopMethod);
Review comment:
can you add a call configureAgentHooks()?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589002717
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-919
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598675120
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588910047
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-917
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-583288734
@poussa Wow! Great you have found this useful. We hoped that some other interested users exist. Hope it will be merged as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589478158
@DaanHoogland
sorry about being slightly nervous about that. This is my fault.
Just would like to add, that initially created an issue #3823 and invited the influencers to participate. You can see the thread - the answer is "oh, we have that", without the deeper thoughts and design discussion ... silence. Next, I created a PR and again "oh, we have that"... Well, I know it's open-source, but I believe it is nice to have a piece of attention to community efforts.
So, my PR is #3839
Compared PR is #3510
Now, not a lot of companies use CS as a public-cloud platform, so a lot of efforts are private cloud, corporate-centric. I suppose this is the reason why #3823 is not understood properly. But it's very useful for people who use CS beyond the release-implemented features.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590115081
> sorry about being slightly nervous about that. This is my fault.
That's alright, your frustration is real and understandable but please do not let it take control.
> I just would like to add, that initially created an issue #3823 and invited the influencers to participate. You can see the thread - the answer is "oh, we have that", without the deeper thoughts and design discussion ... silence. Next, I created a PR and again "oh, we have that"... Well, I know it's open-source, but I believe it is nice to have a piece of attention to community efforts.
I know and I understand that your requirements were not met by that other PR. If you PR does not cause any regressiion we should probably merge it (imnsho). I do want to see how it doubles technical implementation with the rolling maintenance for KVM however. And maybe with other PRs. We should try to unify the efforts. if not before merging than certainly in the future.
@weizhouapache is going to test the patch for regressions as he wants it in (he runs a public cloud!). if he gives his ok, we can merge.
> So, my PR is #3839
> Compared PR is #3510
yes, i guessed so much.
> Now not a lot of companies use CS as a public-cloud platform, so may efforts are private cloud, corporate-centric. I suppose this is the reason why #3823 is not understood properly. But it's very useful for people who use CS beyond the release-implemented features.
Now this is not true. I do not know the exact numbers but neither the number of public clouds nor the number of private/enterprise installations is very low. For the benelux/france region they are about the same I would say.
As for the functionality I am still to understand a few things; How do administrators enter or change hooks? Do they have to go to the host?
Is there a way to control the functionality from management server?
I have some remarks on code style as well. Noticeable the code added is added to existing methods, growing sizes and complexity. I would like to see the new code in separate methods. This is not a breakpoint!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588851139
@andrijapanicsb
@svenvogel
```groovy
package groovy
import com.sun.deploy.xml.XMLNode
import groovy.util.XmlParser
import groovy.util.Node
import groovy.xml.XmlUtil
class CacheDeviceAdder {
def highSpeedDir = "/mnt/nvme0/"
final def ramGBDivider = 1024
Node diskXmlSpec(String swapFile) {
return new XmlParser().parseText(
" <disk type='file' device='disk'>\n" +
" <driver name='qemu' type='qcow2' cache='none'/>\n" +
" <source file='$swapFile'/>\n" +
" <target dev='vdb' bus='ide'/>\n" +
" <alias name='ide0-0-1'/>\n" +
" <address type='drive' controller='0' bus='0' target='0' unit='1'/>\n" +
" </disk>")
}
String transform(Object logger, String xml) {
def vmDef = new XmlParser().parseText(xml)
// get VM ram amount
def memory = Integer.parseInt(vmDef.memory.text()) / ramGBDivider
// get VM ram name
def name = vmDef.name.text()
def swapFile = highSpeedDir + name + ".qcow2"
// remove swap device if exists
"rm -f ${swapFile}".execute()
def volCmd = "qemu-img create -f qcow2 ${swapFile} ${memory}G"
// create new swap device
volCmd.execute()
def diskSpec = diskXmlSpec(swapFile)
// update XML definition
vmDef.devices[0].append(diskSpec)
// return new XML definition
return groovy.xml.XmlUtil.serialize(vmDef)
}
Object stop(Object logger, String vmName) {
def swapFile = highSpeedDir + vmName.toString() + ".qcow2"
// remove unused swap device
"rm -f ${swapFile}".execute()
return null
}
Object start(Object logger, String vmName) {
return null
}
}
new CacheDeviceAdder()
```
This is a very simple hook which utilizes stop and transform cases:
- When transform is used, it adds a highspeed volume which size is eq to VM RAM size, which can be used as a swap.
- When stop, it removes an unused swap device from FS.
- Start is not used, no idea how to use it in the current case.
It's very primitive and avoids many checks, but allows getting a general idea.
Again, I'll add the documentation, when somebody wants to approve the idea... Because, right now there is very simple documentation:
- Configure hook paths and methods in the agent.properties
- Implement hooks in the form:
```groovy
package groovy
class AnyNameNoMatter {
String method(Object logger, String xml) {
// your code
return null // for onStart, onStop
return xml // for xml transformation
}
}
new AnyNameNoMatter()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598964166
@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589003402
> When is code freeze date of 4.14 ?
@weizhouapache It was supposed to be last friday. but there is/ws to much open still, including this. Even though this does not have the milestone attached, i suppose you want it in?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589478158
@DaanHoogland
sorry about being slightly nervous about that. This is my fault.
I just would like to add, that initially created an issue #3823 and invited the influencers to participate. You can see the thread - the answer is "oh, we have that", without the deeper thoughts and design discussion ... silence. Next, I created a PR and again "oh, we have that"... Well, I know it's open-source, but I believe it is nice to have a piece of attention to community efforts.
So, my PR is #3839
Compared PR is #3510
Now not a lot of companies use CS as a public-cloud platform, so a lot of efforts are private cloud, corporate-centric. I suppose this is the reason why #3823 is not understood properly. But it's very useful for people who use CS beyond the release-implemented features.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-599378312
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1052
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on a change in pull request #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r386197715
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -284,6 +284,18 @@
protected String _rngPath = "/dev/random";
protected int _rngRatePeriod = 1000;
protected int _rngRateBytes = 2048;
+ protected String _agentHooksBasedir = "/etc/cloudstack/agent/hooks";
Review comment:
@rhtyd no it's not exactly what does qemu-hooks:
1. there is no qemu hook to override/mangle XML
2. qemu hooks deadlocks in certain cases (for onStart/onStop):
```
A hook script must not call back into libvirt, as the libvirt daemon is already waiting for the script to exit.
A deadlock is likely to occur.
```
From: https://www.libvirt.org/hooks.html
So, libvirt qemu hooks are pretty tough stuff:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] svenvogel commented on issue #3839: FEATURE-3823: kvm
agent hooks
Posted by GitBox <gi...@apache.org>.
svenvogel commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-578680311
@bwsw can you add a documentation too, how to use?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598963097
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1051
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598956319
@andrijapanicsb a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-585549981
@DaanHoogland @rhtyd
@nvazquez Hi, the XML functionality is similar but differs very much.
Read that discussion:
https://github.com/apache/cloudstack/issues/3823
It may look similar from the way it acts, but completely different from the perspective of the ISP operation, especially for public cloud. This PR allows extending CS at scale beyond the CS functionality implemented in the system. Your PR allows users adding the options to XML. There is no way to select any of the available NVME drives in the platform during VM deployment and attach it to the VM.
* My PR is for public cloud operators, who percept CS as a constructor;
* Your PR is for private cloud operators who percept CS as a final system.
The PR is ready to be merged from my point of view. I suppose It's better to merge it to avoid diff accumulation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590817690
@DaanHoogland
> So if I understand correctly, they are not activated in the agent.properties by cloudstack, nor deactivated?
Sure, they must be enabled/disabled by the person who manages the agent after it was connected to the CloudStack, the same way, say, you change RNG and activate watchdog. ACS doesn't do that, I do it after the agent is connected and initial configuration is produced by the agent deployment/integration script.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on a change in pull request #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r386197715
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -284,6 +284,18 @@
protected String _rngPath = "/dev/random";
protected int _rngRatePeriod = 1000;
protected int _rngRateBytes = 2048;
+ protected String _agentHooksBasedir = "/etc/cloudstack/agent/hooks";
Review comment:
@rhtyd no it's not exactly what does qemu-hooks:
1. there is no qemu hook to override XML
2. qemu hooks deadlocks in certain cases (for onStart/onStop):
```
A hook script must not call back into libvirt, as the libvirt daemon is already waiting for the script to exit.
A deadlock is likely to occur.
```
From: https://www.libvirt.org/hooks.html
So, libvirt qemu hooks are pretty tough stuff:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598648611
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan removed a comment on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588242519
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on a change in pull request #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r392129113
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java
##########
@@ -92,6 +93,13 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource
libvirtComputingResource.destroyNetworkRulesForVM(conn, vmName);
final String result = libvirtComputingResource.stopVM(conn, vmName, command.isForceStop());
+ try {
+ LibvirtKvmAgentHook onStopHook = libvirtComputingResource.getStopHook();
+ onStopHook.handle(vmName);
+ } catch (Exception e) {
+ s_logger.warn("Exception occured when handling LibVirt VM onStop hook: {}", e);
+ }
Review comment:
@DaanHoogland fixed every your notes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598964055
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan removed a comment on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588998933
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-918
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-596456997
@weizhouapache did you test and can you approve? If so i think we can merge...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598869586
Waiting for tests results, and if all good, will merge regardless of the freeze date (as approvals and everything else is good atm)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-599028415
<b>Trillian test result (tid-1244)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33321 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3839-t1244-kvm-centos7.zip
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r390162557
##########
File path: .gitlab-ci.yml
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review comment:
@bwsw could you explain what this file is used for and how to use it ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw opened a new pull request #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw opened a new pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839
## Description
<!--- Describe your changes in detail -->
The PR introduces the new KVM agent extension interface in the form of hooks. Every hook is implemented in Groovy like
```
package groovy
class BaseTransform {
String transform(Object logger, String xml) {
File file = new File("/tmp/hooks.txt")
file.append("------------------------------------------------------------------------\n")
file.append(xml)
return xml
}
}
new BaseTransform()
```
There are 3 hooks are implemented now:
1. `XML transformer` which is called right before VM is launched and it allows modifying XML somehow (example above)
2. `onStart` and `onStop` hooks which are called right after VM state changes to started/stopped.
```
package groovy
class StateHandler {
Object onStart(Object logger, String vmName) {
File file = new File("/tmp/hooks-start.txt")
file.append(vmName)
return null
}
Object onStop(Object logger, String vmName) {
File file = new File("/tmp/hooks-stop.txt")
file.append(vmName)
return null
}
}
new StateHandler()
```
Every hook is run inside `try {} catch (Exception e) {}`, so it can not cause agent misbehavior. If hooks are not defined, they are skipped.
Also, the PR adds initial support for GitLab CICD for those, who does WIPs with GitLab, not in GitHub.
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
Initial RFC/Proposal: https://github.com/apache/cloudstack/issues/3823
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Tested in unit tests and manually for master.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598956232
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590332057
@bwsw thanks for the PR.
> sorry about being slightly nervous about that. This is my fault.
Sorry if you had problems to align the proposal. In a complex project such as CloudStack with all stacks involved (management and agents, connecting storage, network, hypervisors, etc) with multiple user cases, it is common to have a few misunderstandings. I am sure that all reviewers had good intention.
With that said, personally I see value on adding this to ACS codebase and the proposal looks very interesting. I had no time yet to review the code but it is good to see that it has been manually tested and it works as expected.
Going in the same direction as @DaanHoogland, it would be great to add a few examples describing how one can configure agent hooks. Is there any documentation regarding how a system admin can configure agent hooks that could be added to the [CloudStack documentation](https://github.com/apache/cloudstack-documentation)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland removed a comment on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588260874
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] rhtyd commented on a change in pull request #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r385577409
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -284,6 +284,18 @@
protected String _rngPath = "/dev/random";
protected int _rngRatePeriod = 1000;
protected int _rngRateBytes = 2048;
+ protected String _agentHooksBasedir = "/etc/cloudstack/agent/hooks";
Review comment:
@bwsw I did not review the whole PR, is it related or similar to http://docs.cloudstack.apache.org/en/latest/adminguide/hosts.html#kvm-libvirt-hook-script-include ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588994398
ok @weizhouapache I agree that the principle is good and should be possible. I'll pass it on regression alone and let user experience (and history), make sure amend it if needed.
BTW i do not expect ACS to manage everything on a platform, just to know what choices to give to a user. I miss the link between available options and the user. Than again, I am not leveraging this option. so if you approve that's good enough for me (provided no regressions)
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588851139
@andrijapanicsb
@svenvogel
```groovy
// these hooks allow adding a temporary high-speed device for local VM cache device.
// simple error-prone implementation.
package groovy
import com.sun.deploy.xml.XMLNode
import groovy.util.XmlParser
import groovy.util.Node
import groovy.xml.XmlUtil
class CacheDeviceAdder {
def highSpeedDir = "/mnt/nvme0/"
final def ramGBDivider = 1024
Node diskXmlSpec(String swapFile) {
return new XmlParser().parseText(
" <disk type='file' device='disk'>\n" +
" <driver name='qemu' type='qcow2' cache='none'/>\n" +
" <source file='$swapFile'/>\n" +
" <target dev='vdb' bus='ide'/>\n" +
" <alias name='ide0-0-1'/>\n" +
" <address type='drive' controller='0' bus='0' target='0' unit='1'/>\n" +
" </disk>")
}
String transform(Object logger, String xml) {
def vmDef = new XmlParser().parseText(xml)
// get VM ram amount
def memory = Integer.parseInt(vmDef.memory.text()) / ramGBDivider
// get VM ram name
def name = vmDef.name.text()
def swapFile = highSpeedDir + name + ".qcow2"
// remove swap device if exists
"rm -f ${swapFile}".execute()
def volCmd = "qemu-img create -f qcow2 ${swapFile} ${memory}G"
// create new swap device
volCmd.execute()
def diskSpec = diskXmlSpec(swapFile)
// update XML definition
vmDef.devices[0].append(diskSpec)
// return new XML definition
return groovy.xml.XmlUtil.serialize(vmDef)
}
Object stop(Object logger, String vmName) {
def swapFile = highSpeedDir + vmName.toString() + ".qcow2"
// remove unused swap device
"rm -f ${swapFile}".execute()
return null
}
Object start(Object logger, String vmName) {
return null
}
}
new CacheDeviceAdder()
```
This is a very simple hook which utilizes stop and transform cases:
- When transform is used, it adds a highspeed volume which size is eq to VM RAM size, which can be used as a swap.
- When stop, it removes an unused swap device from FS.
- Start is not used, no idea how to use it in the current case.
It's very primitive and avoids many checks, but allows getting a general idea.
Again, I'll add the documentation, when somebody wants to approve the idea... Because, right now there is very simple documentation:
- Configure hook paths and methods in the agent.properties
- Implement hooks in the form:
```groovy
package groovy
class AnyNameNoMatter {
String method(Object logger, String xml) {
// your code
return null // for onStart, onStop
return xml // for xml transformation
}
}
new AnyNameNoMatter()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589300468
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-596487854
@bwsw freeze is planned on friday end of day.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r383844922
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java
##########
@@ -92,6 +93,13 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource
libvirtComputingResource.destroyNetworkRulesForVM(conn, vmName);
final String result = libvirtComputingResource.stopVM(conn, vmName, command.isForceStop());
+ try {
+ LibvirtKvmAgentHook onStopHook = libvirtComputingResource.getStopHook();
+ onStopHook.handle(vmName);
+ } catch (Exception e) {
+ s_logger.warn("Exception occured when handling LibVirt VM onStop hook: {}", e);
+ }
Review comment:
can you put this in a separate method?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588242377
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588948810
@DaanHoogland
cloudstack cannot manage everything on a platform.
If user has some special requirements and have to make some customization on vm, this PR provides a option. It is user's responsibility to write the groovy scripts and make it work. We do not need to discuss about details, for example, how to map SRIOV pci device or make sure vxlan have not conflicts with CS or add GPU, etc.
in my opinion, this change is very valuable for advanced cloudstack users.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588851139
@andrijapanicsb
@svenvogel
```groovy
package groovy
import com.sun.deploy.xml.XMLNode
import groovy.util.XmlParser
import groovy.util.Node
import groovy.xml.XmlUtil
class CacheDeviceAdder {
def highSpeedDir = "/mnt/nvme0/"
final def ramGBDivider = 1024
Node diskXmlSpec(String swapFile) {
return new XmlParser().parseText(
" <disk type='file' device='disk'>\n" +
" <driver name='qemu' type='qcow2' cache='none'/>\n" +
" <source file='$swapFile'/>\n" +
" <target dev='vdb' bus='ide'/>\n" +
" <alias name='ide0-0-1'/>\n" +
" <address type='drive' controller='0' bus='0' target='0' unit='1'/>\n" +
" </disk>")
}
String transform(Object logger, String xml) {
def vmDef = new XmlParser().parseText(xml)
def memory = Integer.parseInt(vmDef.memory.text()) / ramGBDivider
def name = vmDef.name.text()
def swapFile = highSpeedDir + name + ".qcow2"
"rm -f ${swapFile}".execute()
def volCmd = "qemu-img create -f qcow2 ${swapFile} ${memory}G"
volCmd.execute()
def diskSpec = diskXmlSpec(swapFile)
vmDef.devices[0].append(diskSpec)
return groovy.xml.XmlUtil.serialize(vmDef)
}
Object stop(Object logger, String vmName) {
def swapFile = highSpeedDir + vmName.toString() + ".qcow2"
"rm -f ${swapFile}".execute()
}
Object start(Object logger, String vmName) {
}
}
new CacheDeviceAdder()
```
This is a very simple hook which utilizes stop and transform cases:
- When transform is used, it adds a highspeed volume which size is eq to VM RAM size, which can be used as a swap.
- When stop, it removes an unused swap device from FS.
- Start is not used, no idea how to use it in the current case.
It's very primitive and avoids many checks, but allows getting a general idea.
Again, I'll add the documentation, when somebody wants to approve the idea... Because, right now there is very simple documentation:
- Configure hook paths and methods in the agent.properties
- Implement hooks in the form:
```groovy
package groovy
class AnyNameNoMatter {
String method(Object logger, String xml) {
return null // for onStart, onStop
return xml // for xml transformation
}
}
new AnyNameNoMatter()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588242519
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland merged pull request #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r383844292
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java
##########
@@ -79,7 +79,28 @@ public Answer execute(final StartCommand command, final LibvirtComputingResource
libvirtComputingResource.createVifs(vmSpec, vm);
s_logger.debug("starting " + vmName + ": " + vm.toString());
- libvirtComputingResource.startVM(conn, vmName, vm.toString());
+ String vmInitialSpecification = vm.toString();
+ String vmFinalSpecification;
+ try {
+ // if transformer fails, everything must go as it's just skipped.
+ LibvirtKvmAgentHook t = libvirtComputingResource.getTransformer();
+ vmFinalSpecification = (String) t.handle(vmInitialSpecification);
+ if (null == vmFinalSpecification) {
+ s_logger.warn("Libvirt XML transformer returned NULL, will use XML specification unchanged.");
+ vmFinalSpecification = vmInitialSpecification;
+ }
+ } catch(Exception e) {
+ s_logger.warn("Exception occured when handling LibVirt XML transformer hook: {}", e);
+ vmFinalSpecification = vmInitialSpecification;
+ }
Review comment:
can you add a call performAgentTransformHook(..) here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan removed a comment on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588256897
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-907
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588998097
> ok @weizhouapache I agree that the principle is good and should be possible. I'll pass it on regression alone and let user experience (and history), make sure amend it if needed.
>
> BTW i do not expect ACS to manage everything on a platform, just to know what choices to give to a user. I miss the link between available options and the user. Than again, I am not leveraging this option. so if you approve that's good enough for me (provided no regressions)
> @blueorangutan package
@DaanHoogland 👍 I will check if there are potential regression, and test it. maybe next week.
When is code freeze date of 4.14 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-579103926
> @bwsw can you add a documentation too, how to use?
@svenvogel I'll do for sure, just would like to do it after there are no more design concerns about PR and it is approved for merge.
As far as I know, there is no specific place for documentation like that. Where we would like to have it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589300895
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589503943
<b>Trillian test result (tid-1071)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29654 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3839-t1071-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 79 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_vpc_privategw_static_routes | `Failure` | 226.40 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 178.64 | test_privategw_acl.py
test_04_rvpc_privategw_static_routes | `Failure` | 268.05 | test_privategw_acl.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r383844434
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java
##########
@@ -79,7 +79,28 @@ public Answer execute(final StartCommand command, final LibvirtComputingResource
libvirtComputingResource.createVifs(vmSpec, vm);
s_logger.debug("starting " + vmName + ": " + vm.toString());
- libvirtComputingResource.startVM(conn, vmName, vm.toString());
+ String vmInitialSpecification = vm.toString();
+ String vmFinalSpecification;
+ try {
+ // if transformer fails, everything must go as it's just skipped.
+ LibvirtKvmAgentHook t = libvirtComputingResource.getTransformer();
+ vmFinalSpecification = (String) t.handle(vmInitialSpecification);
+ if (null == vmFinalSpecification) {
+ s_logger.warn("Libvirt XML transformer returned NULL, will use XML specification unchanged.");
+ vmFinalSpecification = vmInitialSpecification;
+ }
+ } catch(Exception e) {
+ s_logger.warn("Exception occured when handling LibVirt XML transformer hook: {}", e);
+ vmFinalSpecification = vmInitialSpecification;
+ }
+ libvirtComputingResource.startVM(conn, vmName, vmFinalSpecification);
+
+ try {
+ LibvirtKvmAgentHook onStartHook = libvirtComputingResource.getStartHook();
+ onStartHook.handle(vmName);
+ } catch (Exception e) {
+ s_logger.warn("Exception occured when handling LibVirt VM onStart hook: {}", e);
+ }
Review comment:
and performAgentStartHook(..) here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598977985
<b>Trillian test result (tid-1242)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44040 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3839-t1242-kvm-centos7.zip
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r371312961
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -1015,11 +1015,12 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
int currentCpu = currentServiceOffering.getCpu();
int currentMemory = currentServiceOffering.getRamSize();
+ Account owner = _accountMgr.getActiveAccountById(vmInstance.getAccountId());
if (newCpu > currentCpu) {
- _resourceLimitMgr.checkResourceLimit(caller, ResourceType.cpu, newCpu - currentCpu);
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu);
}
if (newMemory > currentMemory) {
- _resourceLimitMgr.checkResourceLimit(caller, ResourceType.memory, newMemory - currentMemory);
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory);
}
// Check that the specified service offering ID is valid
Review comment:
@DennisKonrad I do not think so.
@bwsw could you use "git rebase" with latest master, instead of "git merge" ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-585549981
@DaanHoogland @rhtyd
@nvazquez Hi, the XML functionality is similar but differs very much.
Read that discussion:
https://github.com/apache/cloudstack/issues/3823
It may look similar in the way it acts, but completely different from the perspective of the ISP operation, especially for public cloud. This PR allows extending CS at a scale beyond the CS functionality implemented in the system. Your PR allows users adding the options to XML.
E.g. In your PR there is no way to select *any* of the available NVME drives installed in the platform during VM deployment and attach it to the VM using some advanced logic.
* My PR is for public cloud operators, who percept CS as a constructor;
* Your PR is for private cloud operators who percept CS as a final system.
The PR is ready to be merged from my point of view. I suppose It's better to merge it to avoid diff accumulation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590817690
@DaanHoogland
> So if I understand correctly, they are not activated in the agent.properties by cloudstack, nor deactivated?
Sure, they must be enabled/disabled by the person who manages the agent after it was connected to the CloudStack, the same way, say, I change RNG and activate watchdog. ACS doesn't do that, I do it after the agent is connected and initial configuration is produced by the agent deployment/integration script.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588994430
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588851139
// these hooks allow adding a temporary high-speed device for local VM cache device.
// simple error-prone implementation.
@andrijapanicsb
@svenvogel
```groovy
package groovy
import com.sun.deploy.xml.XMLNode
import groovy.util.XmlParser
import groovy.util.Node
import groovy.xml.XmlUtil
class CacheDeviceAdder {
def highSpeedDir = "/mnt/nvme0/"
final def ramGBDivider = 1024
Node diskXmlSpec(String swapFile) {
return new XmlParser().parseText(
" <disk type='file' device='disk'>\n" +
" <driver name='qemu' type='qcow2' cache='none'/>\n" +
" <source file='$swapFile'/>\n" +
" <target dev='vdb' bus='ide'/>\n" +
" <alias name='ide0-0-1'/>\n" +
" <address type='drive' controller='0' bus='0' target='0' unit='1'/>\n" +
" </disk>")
}
String transform(Object logger, String xml) {
def vmDef = new XmlParser().parseText(xml)
// get VM ram amount
def memory = Integer.parseInt(vmDef.memory.text()) / ramGBDivider
// get VM ram name
def name = vmDef.name.text()
def swapFile = highSpeedDir + name + ".qcow2"
// remove swap device if exists
"rm -f ${swapFile}".execute()
def volCmd = "qemu-img create -f qcow2 ${swapFile} ${memory}G"
// create new swap device
volCmd.execute()
def diskSpec = diskXmlSpec(swapFile)
// update XML definition
vmDef.devices[0].append(diskSpec)
// return new XML definition
return groovy.xml.XmlUtil.serialize(vmDef)
}
Object stop(Object logger, String vmName) {
def swapFile = highSpeedDir + vmName.toString() + ".qcow2"
// remove unused swap device
"rm -f ${swapFile}".execute()
return null
}
Object start(Object logger, String vmName) {
return null
}
}
new CacheDeviceAdder()
```
This is a very simple hook which utilizes stop and transform cases:
- When transform is used, it adds a highspeed volume which size is eq to VM RAM size, which can be used as a swap.
- When stop, it removes an unused swap device from FS.
- Start is not used, no idea how to use it in the current case.
It's very primitive and avoids many checks, but allows getting a general idea.
Again, I'll add the documentation, when somebody wants to approve the idea... Because, right now there is very simple documentation:
- Configure hook paths and methods in the agent.properties
- Implement hooks in the form:
```groovy
package groovy
class AnyNameNoMatter {
String method(Object logger, String xml) {
// your code
return null // for onStart, onStop
return xml // for xml transformation
}
}
new AnyNameNoMatter()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588260874
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589005691
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588851139
@andrijapanicsb
@svenvogel
```groovy
package groovy
import com.sun.deploy.xml.XMLNode
import groovy.util.XmlParser
import groovy.util.Node
import groovy.xml.XmlUtil
class CacheDeviceAdder {
def highSpeedDir = "/mnt/nvme0/"
final def ramGBDivider = 1024
Node diskXmlSpec(String swapFile) {
return new XmlParser().parseText(
" <disk type='file' device='disk'>\n" +
" <driver name='qemu' type='qcow2' cache='none'/>\n" +
" <source file='$swapFile'/>\n" +
" <target dev='vdb' bus='ide'/>\n" +
" <alias name='ide0-0-1'/>\n" +
" <address type='drive' controller='0' bus='0' target='0' unit='1'/>\n" +
" </disk>")
}
String transform(Object logger, String xml) {
def vmDef = new XmlParser().parseText(xml)
// get VM ram amount
def memory = Integer.parseInt(vmDef.memory.text()) / ramGBDivider
// get VM ram name
def name = vmDef.name.text()
def swapFile = highSpeedDir + name + ".qcow2"
// remove swap device if exists
"rm -f ${swapFile}".execute()
def volCmd = "qemu-img create -f qcow2 ${swapFile} ${memory}G"
// create new swap device
volCmd.execute()
def diskSpec = diskXmlSpec(swapFile)
// update XML definition
vmDef.devices[0].append(diskSpec)
// return new XML definition
return groovy.xml.XmlUtil.serialize(vmDef)
}
Object stop(Object logger, String vmName) {
def swapFile = highSpeedDir + vmName.toString() + ".qcow2"
// remove unused swap device
"rm -f ${swapFile}".execute()
}
Object start(Object logger, String vmName) {
}
}
new CacheDeviceAdder()
```
This is a very simple hook which utilizes stop and transform cases:
- When transform is used, it adds a highspeed volume which size is eq to VM RAM size, which can be used as a swap.
- When stop, it removes an unused swap device from FS.
- Start is not used, no idea how to use it in the current case.
It's very primitive and avoids many checks, but allows getting a general idea.
Again, I'll add the documentation, when somebody wants to approve the idea... Because, right now there is very simple documentation:
- Configure hook paths and methods in the agent.properties
- Implement hooks in the form:
```groovy
package groovy
class AnyNameNoMatter {
String method(Object logger, String xml) {
return null // for onStart, onStop
return xml // for xml transformation
}
}
new AnyNameNoMatter()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] poussa commented on issue #3839: FEATURE-3823: kvm
agent hooks
Posted by GitBox <gi...@apache.org>.
poussa commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588987898
I was able to use this feature to expose Intel QAT crypto/compression SRIOV VF to CS VM via the following snippet (never used groovy before so not pretty but works for my PoC)
```java
class BaseTransform {
String transform(Object logger, String xml) {
def parser = new XmlParser()
def domain = parser.parseText(xml)
def devices = domain.devices[1]
def hostDevAttr = [:]
hostDevAttr."mode" = "subsystem"
hostDevAttr."type" = "pci"
hostDevAttr."managed" = "yes"
def hostdev = parser.createNode(devices,
new QName("hostdev"), hostDevAttr)
def source = hostdev.appendNode(new QName("source"))
def addressAttr = [:]
addressAttr."domain" = "0x0000"
addressAttr."bus" = "0x3d"
addressAttr."slot" = "0x01"
addressAttr."function" = "0x1"
def address = source.appendNode(new QName("address"), addressAttr)
return XmlUtil.serialize(domain)
}
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-596478203
@DaanHoogland @weizhouapache please, give me one-two days to complete the refactoring as Daan proposed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588888276
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on a change in pull request #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r371630278
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -1015,11 +1015,12 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
int currentCpu = currentServiceOffering.getCpu();
int currentMemory = currentServiceOffering.getRamSize();
+ Account owner = _accountMgr.getActiveAccountById(vmInstance.getAccountId());
if (newCpu > currentCpu) {
- _resourceLimitMgr.checkResourceLimit(caller, ResourceType.cpu, newCpu - currentCpu);
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu);
}
if (newMemory > currentMemory) {
- _resourceLimitMgr.checkResourceLimit(caller, ResourceType.memory, newMemory - currentMemory);
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory);
}
// Check that the specified service offering ID is valid
Review comment:
@weizhouapache @DennisKonrad resolved.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588270544
@bwsw can I kindly join the effort on asking for some documentation - at the bare minimum I would like to see an example of how to use this feature (some specific, very-simple scenario) here on the PR description - so we can test it - i.e. I can't test it by going through the code as I'm not a developer myself, but would like to help testing it. i.e. expand on "How Has This Been Tested" section please.
thx
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590397819
@GabrielBrascher @bwsw , Don't misunderstand me, I am not demanding any documentation. If others have good use of this code *and* it causes no regressions for me or my users I am fine with it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589022140
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-921
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588241546
@bwsw I'm happy to put some effort into getting this merged. Yur code looks neat, even though I don't undertand it fully yet. A few things putting me off in your latest reply. Please have patience with me?
Firstly:
> My PR is for public cloud operators, who percept CS as a constructor;
I imagine you mean *this* PR, right?
> Your PR is for private cloud operators who percept CS as a final system.
What PR is that? Not "who's!", can you please refer to bits of code and not people? I can see some frustration in this comment and I have been involved in this conflict between private clouds an public clouds a lot. My question is genuine. On a side note, It's all apache's PRs ;)
> Case1. In your PR there is no way to select any of the available NVME drives installed in the platform during VM deployment and attach it to the VM using some advanced logic.
(what PR?) can you add/explain how you are adding that info? I would expect some kind of dialog going back to the MS to provide the user with a list of available resources but can not see that in the code.
> Case2. when VM is deployed, for every CS account, I create VXLAN unmanaged by CS. I create a bridge and would like to add a NIC into configuration dynamically.
So how do you guarantee the same vxlan's aren't also used by ACS? (And also, parallel to case 1 above, how is the vxlan id fed back into ACS)
> Case 3. I would like to find any of available Quadro RTX 4000 GPU and pass it into VM.
In this case you do not want the user to add it, right? but have it automagically added?
> Case 4. Read Sakari Poussa's request from mailing list:
Can you add a reference to the mail thread in lists.apache.org please? The snippet contains references to context.
@svenvogel is asking for documentation above. Are you already using this in production? Can you add a PR to https://github.com/apache/cloudstack-documentation, please?
@weizhouapache , @GabrielBrascher , @kiwiflyer thoughts/reviews?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] rhtyd closed pull request #3839: FEATURE-3823: kvm
agent hooks
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] poussa commented on issue #3839: FEATURE-3823: kvm
agent hooks
Posted by GitBox <gi...@apache.org>.
poussa commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-582959410
I tried this feature and it provided a solution my use case (map SRIOV pci device to KVM host).
Is this going to be merged to the master anytime soon?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on a change in pull request #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r386198318
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -284,6 +284,18 @@
protected String _rngPath = "/dev/random";
protected int _rngRatePeriod = 1000;
protected int _rngRateBytes = 2048;
+ protected String _agentHooksBasedir = "/etc/cloudstack/agent/hooks";
Review comment:
Initially I tried to use qemu-hooks for my task, but lately moved my code to security groups python code, because, otherwise, it works unstable because of the reasons listed above.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-579157609
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-674
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590817690
@DaanHoogland
> So if I understand correctly, they are not activated in the agent.properties by cloudstack, nor deactivated?
Sure, they must be enabled/disabled by the person who manages the agent after it was connected to the CloudStack, the same way, say, you change RNG and activate watchdog. ACS doesn't do that, you do it after the agent is connected and initial configuration is produced by the agent deployment/integration script.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598648306
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588887665
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590793862
@DaanHoogland
> As for the functionality I am still to understand a few things; How do administrators enter or change hooks? Do they have to go to the host? Is there a way to control the functionality of the management server?
These are system-wide scripts, which are deployed based on certain logic implemented for a cloud, they are not per-user, per-vm, etc. The hooks are deployed to agents with Ansible or another way. After being deployed, they substitute the old ones, so it helps to upgrade transparently.
Administrators never write hooks by themselves, they are implemented by cloud architectors/developers according to the cloud expected properties, which are beyond the logic implemented in ACS standard functionality.
The hooks are activated in Agent with following options:
```ini
# Libvirt XML transformer hook does XML-to-XML transformation which provider can use to add/remove/modify some
# sort of attributes in Libvirt XML domain specification.
agent.hooks.libvirt_vm_xml_transformer.script=libvirt-vm-xml-transformer.groovy
agent.hooks.libvirt_vm_xml_transformer.method=transform
#
# The hook is called right after libvirt successfuly launched VM
agent.hooks.libvirt_vm_on_start.script=libvirt-vm-state-change.groovy
agent.hooks.libvirt_vm_on_start.method=onStart
#
# The hook is called right after libvirt successfuly stopped VM
agent.hooks.libvirt_vm_on_stop.script=libvirt-vm-state-change.groovy
agent.hooks.libvirt_vm_on_stop.method=onStop
```
Hope it clarifies things a little bit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-579147864
@nvazquez this resembles something you did. Can you have look?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589005465
@bwsw I removed the import of List in the startcommandwrapper, hope you don't mind.
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] nvazquez commented on issue #3839: FEATURE-3823: kvm
agent hooks
Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-585547146
Hi @bwsw, I find the XML use case pretty similar to what's been introduced by this PR: #3510. Start and stop VM hooks looks good. In PR #3610 I'm working on adding hooks scripts for rolling maintenance and similarly to this PR, an admin can define its own scripts and has to specify a directory where to find these scripts on the agent.properties file, perhaps this property can be shared between both PRs
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland removed a comment on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588242377
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588256897
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-907
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590793862
@DaanHoogland
> As for the functionality I am still to understand a few things; How do administrators enter or change hooks? Do they have to go to the host? Is there a way to control the functionality of the management server?
These are system-wide scripts, which are deployed based on certain logic implemented for a cloud, they are not per-user, per-vm, etc. The hooks are deployed to agents with Ansible or another way. After being deployed, they substitute the old ones, so it helps to upgrade transparently.
Administrators never write hooks by themselves, they are implemented by cloud architectors/developers according to the cloud expected properties, which are beyond the logic implemented in ACS standard functionality.
The hooks are activated in Agent with following options:
```ini
# Libvirt XML transformer hook does XML-to-XML transformation which provider can use to add/remove/modify some
# sort of attributes in Libvirt XML domain specification.
agent.hooks.libvirt_vm_xml_transformer.script=libvirt-vm-xml-transformer.groovy
agent.hooks.libvirt_vm_xml_transformer.method=transform
#
# The hook is called right after libvirt successfuly launched VM
agent.hooks.libvirt_vm_on_start.script=libvirt-vm-state-change.groovy
agent.hooks.libvirt_vm_on_start.method=onStart
#
# The hook is called right after libvirt successfuly stopped VM
agent.hooks.libvirt_vm_on_stop.script=libvirt-vm-state-change.groovy
agent.hooks.libvirt_vm_on_stop.method=onStop
```
Hope it clarifies things a little bit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-585549981
@DaanHoogland @rhtyd
@nvazquez Hi, the XML functionality is similar but differs very much.
Read that discussion:
https://github.com/apache/cloudstack/issues/3823
It may look similar in the way it acts, but completely different from the perspective of the ISP operation, especially for public cloud. This PR allows extending CS at a scale beyond the CS functionality implemented in the system. Your PR allows users adding the options to XML.
E.g.
Case1. In your PR there is no way to select *any* of the available NVME drives installed in the platform during VM deployment and attach it to the VM using some advanced logic.
Case2. when VM is deployed, for every CS account, I create VXLAN unmanaged by CS. I create a bridge and would like to add a NIC into configuration dynamically. It's impossible to implement with your PR.
Case 3. I would like to find any of available Quadro RTX 4000 GPU and pass it into VM.
Case 4. Read Sakari Poussa's request from mailing list:
```
Thanks for the information. It was useful.
Let me elaborate by use cases a bit more. I have PCI host devices with
sriov capabilities. I may have 48 virtual functions (VF) on a host. I want
to assign the VFs to some VM but not all. So I need some control which VMs
gets the VF and others don't. Also, I need to keep track which VFs are
already assigned and which are free. Lastly, I want to expose the VFs to
containers running on VMs created by the upcoming Cloudstack Kubernetes
Service (AKS, pull #3680).
Looking at the first feature you mentioned, I don't think I can use that.
It has no control on which VMs to add the extraconfig. It is all or nothing.
The second feature, which you started to work on seems to have more
potential. Do you see it can support my use case?
Thanks, Sakari
```
None of those cases are possible with PR you mention. To sum up:
* My PR is for public cloud operators, who percept CS as a constructor;
* Your PR is for private cloud operators who percept CS as a final system.
The PR is ready to be merged from my point of view. I suppose It's better to merge it to avoid diff accumulation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598660103
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1048
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw edited a comment on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw edited a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589478158
@DaanHoogland
sorry about being slightly nervous about that. This is my fault.
I just would like to add, that initially created an issue #3823 and invited the influencers to participate. You can see the thread - the answer is "oh, we have that", without the deeper thoughts and design discussion ... silence. Next, I created a PR and again "oh, we have that"... Well, I know it's open-source, but I believe it is nice to have a piece of attention to community efforts.
So, my PR is #3839
Compared PR is #3510
Now not a lot of companies use CS as a public-cloud platform, so may efforts are private cloud, corporate-centric. I suppose this is the reason why #3823 is not understood properly. But it's very useful for people who use CS beyond the release-implemented features.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589478158
@DaanHoogland
sorry about being slightly nervous about that. This is my fault.
Just would like to add, that initially created an issue #3823 and invited the influencers to participate. You can see the thread - the answer is "oh, we have that", without the deeper thoughts and design discussion ... silence. Next, I created a PR and again "oh, we have that"... Well, I know it's open-source, but I believe it is nice to have a piece of attention to community efforts.
So, my PR is #3823
Compared PR is #3510
Now, not a lot of companies use CS as a public-cloud platform, so a lot of efforts are private cloud, corporate-centric. I suppose this is the reason why #3823 is not understood properly. But it's very useful for people who use CS beyond the release-implemented features.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan removed a comment on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-589002717
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-919
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on a change in pull request #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r384304031
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java
##########
@@ -92,6 +93,13 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource
libvirtComputingResource.destroyNetworkRulesForVM(conn, vmName);
final String result = libvirtComputingResource.stopVM(conn, vmName, command.isForceStop());
+ try {
+ LibvirtKvmAgentHook onStopHook = libvirtComputingResource.getStopHook();
+ onStopHook.handle(vmName);
+ } catch (Exception e) {
+ s_logger.warn("Exception occured when handling LibVirt VM onStop hook: {}", e);
+ }
Review comment:
@DaanHoogland I'll handle all your comments tomorrow.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-598675501
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588998361
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DennisKonrad commented on a change in pull request
#3839: FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DennisKonrad commented on a change in pull request #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#discussion_r371242703
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -1015,11 +1015,12 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
int currentCpu = currentServiceOffering.getCpu();
int currentMemory = currentServiceOffering.getRamSize();
+ Account owner = _accountMgr.getActiveAccountById(vmInstance.getAccountId());
if (newCpu > currentCpu) {
- _resourceLimitMgr.checkResourceLimit(caller, ResourceType.cpu, newCpu - currentCpu);
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu);
}
if (newMemory > currentMemory) {
- _resourceLimitMgr.checkResourceLimit(caller, ResourceType.memory, newMemory - currentMemory);
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory);
}
// Check that the specified service offering ID is valid
Review comment:
I previously saw code like this in https://github.com/apache/cloudstack/pull/3759/files. Does this belong into this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588998933
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-918
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3839: FEATURE-3823: kvm agent
hooks
Posted by GitBox <gi...@apache.org>.
bwsw commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588851139
@andrijapanicsb
@svenvogel
```
package groovy
import com.sun.deploy.xml.XMLNode
import groovy.util.XmlParser
import groovy.util.Node
import groovy.xml.XmlUtil
class CacheDeviceAdder {
def highSpeedDir = "/mnt/nvme0/"
final def ramGBDivider = 1024
Node diskXmlSpec(String swapFile) {
return new XmlParser().parseText(
" <disk type='file' device='disk'>\n" +
" <driver name='qemu' type='qcow2' cache='none'/>\n" +
" <source file='$swapFile'/>\n" +
" <target dev='vdb' bus='ide'/>\n" +
" <alias name='ide0-0-1'/>\n" +
" <address type='drive' controller='0' bus='0' target='0' unit='1'/>\n" +
" </disk>")
}
String transform(Object logger, String xml) {
def vmDef = new XmlParser().parseText(xml)
def memory = Integer.parseInt(vmDef.memory.text()) / ramGBDivider
def name = vmDef.name.text()
def swapFile = highSpeedDir + name + ".qcow2"
"rm -f ${swapFile}".execute()
def volCmd = "qemu-img create -f qcow2 ${swapFile} ${memory}G"
volCmd.execute()
def diskSpec = diskXmlSpec(swapFile)
vmDef.devices[0].append(diskSpec)
return groovy.xml.XmlUtil.serialize(vmDef)
}
Object stop(Object logger, String vmName) {
def swapFile = highSpeedDir + vmName.toString() + ".qcow2"
"rm -f ${swapFile}".execute()
}
Object start(Object logger, String vmName) {
}
}
new CacheDeviceAdder()
```
This is a very simple hook which utilizes stop and transform cases:
- When transform is used, it adds a highspeed volume which size is eq to VM RAM size, which can be used as a swap.
- When stop, it removes an unused swap device from FS.
- Start is not used, no idea how to use it in the current case.
It's very primitive and avoids many checks, but allows getting a general idea.
Again, I'll add the documentation, when somebody wants to approve the idea... Because, right now there is very simple documentation:
- Configure hook paths and methods in the agent.properties
- Implement hooks in the form:
```groovy
package groovy
class AnyNameNoMatter {
String method(Object logger, String xml) {
return null // for onStart, onStop
return xml // for xml transformation
}
}
new AnyNameNoMatter()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan removed a comment on issue #3839:
FEATURE-3823: kvm agent hooks
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-588998361
@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3839: FEATURE-3823:
kvm agent hooks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3839: FEATURE-3823: kvm agent hooks
URL: https://github.com/apache/cloudstack/pull/3839#issuecomment-590805317
thanks
> These are system-wide scripts, which are deployed based on certain logic implemented for a cloud, they are not per-user, per-vm, etc. The hooks are deployed to agents with Ansible or another way. After being deployed, they substitute the old ones, so it helps to upgrade transparently.
So if I understand correctly, they are not activated in the agent.properties by cloudstack, nor deactivated?
Why I am asking is that this means, to guarantee their working a mechanism must be in place out of band that guards the contents in case of updates from ACS, like the agent lb feature.
> Administrators never write hooks by themselves, they are implemented by cloud architectors/developers according to the cloud expected properties, which are beyond the logic implemented in ACS standard functionality.
clear
...
> Hope it clarifies things a little bit.
it does ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services