You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rodrigo93 <gi...@git.apache.org> on 2015/10/17 22:16:55 UTC

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

GitHub user rodrigo93 opened a pull request:

    https://github.com/apache/cloudstack/pull/943

    Removed unused adapters from async-job-component.xml.

    I was looking the file /cloud-server/test/async-job-component.xml, and I found an adapter configuration that seems to have no use. The reason for that is explained as follows.
    
    The adapter configuration is the following:
    <adapters key="com.cloud.agent.manager.allocator.StorageAllocator">
          <adapter name="Storage"
            class="com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator">
            <param name="storage.overprovisioning.factor">2</param>
          </adapter>
          <adapter name="
            class="com.cloud.agent.manager.allocator.impl.RandomStoragePoolAllocator">
            <param name="storage.overprovisioning.factor">2</param>
          </adapter>
    </adapters>
    
    •	class="com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator"
    
    The class "com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator" does not exist. The only reference for it is found in the following file:
              -	/cloud-server/test/async-job-component.xml
    Therefore, we can conclude that there is no need for this line at that file.
    
    •	class="com.cloud.agent.manager.allocator.impl.RandomStoragePoolAllocator"
    
    Additionally, the class RandomStoragePoolAllocator.java is never used. The only reference is found in the following file:
    -	/cloud-server/test/async-job-component.xml
    
    I found a project called “cloud-plugin-storage-allocator-random”. This project has only one package that contains only one class, which is the RandomStoragePoolAllocator.java. Despite the names that are the same, the class in “cloud-plugin-storage-allocator-random”  project and the class referenced in -	/cloud-server/test/async-job-component.xml have different packages. Therefore, I removed that configuration from async-job-component.xml and the project that contains only the RandomStoragePoolAllocator class that is never used.
    
    Those changes leave us with an adapter configuration empty with the following key:
    •	key="com.cloud.agent.manager.allocator.StorageAllocator"
    
    Therefore, I removed it.
    
    Furthermore, after I removed that configuration I noticed that there is no such class StorageAllocator.java. However, it appears that exists test for it, like the following classes: 
    -	StorageAllocatorTestConfiguration.java
    -	StorageAllocatorTest.java. 
    I am not sure if these classes are tests for the class StorageAllocator.java and for the possible configuration I have just removed. If they are, we can remove both classes. Can someone help me on checking that? 
    
    I also removed the following configuration from /cloudstack-plugins/pom.xml: 
    -	<module>storage-allocators/random</module>


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rafaelweingartner/cloudstack lrg-cs-hackday-009

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/943.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #943
    
----
commit a387cd06e0a3e8ee6285bd0ed18cd09b53482440
Author: Rodrigo Marques <ro...@gmail.com>
Date:   2015-10-17T19:35:57Z

    Removed unused adapters that begins with "<adapters
    key="com.cloud.agent.manager.allocator.StorageAllocator">".

commit bb23cdf1a460d484541eb2d6d384b3a68c1f50ef
Author: Rodrigo Marques <ro...@gmail.com>
Date:   2015-10-17T20:08:22Z

    Removed unused configuration and project.
    
    Unused configuration was removed from /cloudstack-plugins/pom.xml.
    
    Removed unused project “cloud-plugin-storage-allocator-random”.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-149059531
  
    @rafaelweingartner Thanks for the advice! I am squashing them right now. I will commit again as soon as possible.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160297495
  
    @Daanhooglan, I got your point.
    So, what about if we just fix the reference for that class in “async-job-component.xml”. We could also remove the reference for “com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator” that does not seem to exist.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/943#discussion_r46436064
  
    --- Diff: server/test/async-job-component.xml ---
    @@ -108,11 +108,7 @@
         </adapters>
         <adapters key="com.cloud.agent.manager.allocator.StorageAllocator">
           <adapter name="Storage"
    -        class="com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator">
    --- End diff --
    
    Not sure about this, anyone using this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-170073454
  
    Many thanks for the help everyone! 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused classes and project.

Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-150802394
  
    @rodrigo93 You changed the PR title, nog thr commit title. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160895245
  
    [943.vm.results.txt](https://github.com/apache/cloudstack/files/48256/943.vm.results.txt)
    Here's the test that fails. It failed the second time as well. I won't have time to investigate today. I'll start another one from your list in a minute.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160704405
  
    there is a merge conflict @rodrigo93 . can you resolve it please?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161060067
  
    @rafaelweingartner I tested #942 with the setting of expunge * 6 but the same test still failed on that PR. I am convinced it has nothing to do with the code of these PRs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-150672917
  
    @rodrigo93 Thanks for squashing! Could you also rename the commit title to something more sensible?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-150824034
  
    Hi @remibergsma, thanks for the advice! 
    I have changed the title of the last commit already as you both suggest. 
    I will provide a JIRA issue and prepend the ticket as you suggested.
    
    Thanks once again! 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161043712
  
    @DaanHoogland with your explanation I now understand that we want to test if the expunge thread is being executed and if it is doing its job. Therefore, I believe that properly setting the wait period before checking if the VM was remove should do the trick. 
    
    Before changing anything, I would like to check the configuration of expunge interval and expunge delay that are being used in the test MS. However, I did not find the file they are being configured (I am really not good reading perl).



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-158675017
  
    Hi @rodrigo93 in the comments above it was suggested to change the commit title, can you do that please? The commit title is the first line of the commit message, and that should be max about 70 characters long. Followed by an empty line, and then whatever it is you want to say more (probably what you start with now). In short: add a short descriptive title and an empty line and force push the commit again. Ping me and I'll run soms tests on this branch. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160294688
  
    I am not sure if this allocator is used any where. As it is injected by it's interface StoragePoolAllocator, it will be a choice to use from a list. It is likely not to be used as it is clearly not properly configured. Someone might have fixed it locally however. I am not keen on deletion an option for users.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160966699
  
    @DaanHoogland, 
    I checked the logs you sent me.
    
    The VMs were marked as destroyed, but it seems that they have not been “destroyed” or removed/expunged yet. I looked at the code, and the only way that they are removed from the response of the list VMs methods is after the expunge thread execution that fills out the “removed” field in the database.
     
    I also looked at the code of the integration tests, my perl is a little rusty, but I noticed that the code waits a few cycles (2) of the expunge delay to execute; therefore, there is no way to guarantee that the expunge thread has already been executed and the VM has passed the expunge delay and has been removed.
    
    If I recall properly, there are mainly three (3) variables in play, the time that the VM was destroyed, the expunge delay per se and the expunge interval (the interval of the expunge thread execution). 
    
    So, if the expunge thread runs, but the VM has been destroyed too recently and has not passed the expunge delay, it will not be marked as destroyed. That is what seems to have happened there. I know some people may come and say, “the test worked a lot of time”. And yes it can work, but it depends if you are luck or not. I personally do not like tests that may present this kind of behavior.  Moreover, the expunge interval depends on the time that the MS has been started.
    
    I will illustrate it with an example that we have seen happening here.
    Giving that our expunge interval is 24 hours, and our expunge delay is also 24 hours. Suppose the MS server was started and got up and running at some day at 23:59 and that the first time the expunge thread runs is 00:00. If we are unlucky and we destroy the VM at 00:01, next day (second run of the expunge thread) when the thread runs at 00:00, the VM will not be removed and will continue appearing, since the expunge delay that cotrols the VMs removal is 24 hours and the VM has been destroyed for 23:59 (almost there, but not yet). Therefore, the VM will only be removed in the third execution of the expunge thread.
    
    Having said that, I have the following questions, what do we want with that test? We want to test the expunge thread? Or just test If the destroyed VM is not listed? If we want the second, why don’t we force the expunging (using expungeVirtualMachine command) instead of waiting the expunge thread?
    
    If the idea is to let the test as it is, to avoid the problem I have just described, we could just change a "bit" of the file test_vm_life_cycle the multiplier, in line 632 from “4” to “6” . That change would guarantee to wait till the third execution of the expunge thread, and avoid cases as the one described.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/943#discussion_r46455242
  
    --- Diff: server/test/async-job-component.xml ---
    @@ -108,11 +108,7 @@
         </adapters>
         <adapters key="com.cloud.agent.manager.allocator.StorageAllocator">
           <adapter name="Storage"
    -        class="com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator">
    --- End diff --
    
    @bhaisaab , probably note (I would say that no one uses it) that class does not exist.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-151489875
  
    @rodrigo93 it helps when running the tests. But never mind, I will rebase myself and then run the tests :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160705037
  
    @DaanHoogland , where did you see the merge conflict Daan ? I did not find it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused classes and project.

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-150744688
  
    @borisroman Hi borisroman, sure! What about this one?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/943


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160714102
  
    @DaanHoogland Can you tell us where the conflict is? It seems ok to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-158671944
  
    PR merged with master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160321659
  
    looks good, waiting on jenkins and will ask @remibergsma to schedule integration tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161066414
  
    That is odd, because the log you sent me earlier, the VM was marked as destroyed, so it was just a matter of executing the expunge thread. Is it possible for you to send me your MS logs? So I can tried to follow the execution of the expunge thread code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161555776
  
    LGTM, ran the tests as well and reviewed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160294766
  
    @rodrigo93 Jenkins can not merge this to master. please have a look.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-151206169
  
    Hi @remibergsma, the files that I am changing were not altered in the current master. Why do we need to rebase?
    Thanks for your time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused classes and project.

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-150802690
  
    I'm with @borisroman the commit title "Squashing last commits" is not descriptive of what this commit is about so please change it. You can use the same as the title of the PR. Consider making a Jira issue and prepend the ticket in the title of the PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-158790894
  
    Hi @remibergsma. Thanks for the reminding! I was merging that commit with that actual master and I forgot to put the title in the commit.
    I have fixed that already. Thanks once again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161048740
  
    Oh sorry,  I do not know why, but I thought it was perl.
    
    Well, if those configurations parameter are being taken from the MS, they are not important. I thought they were being taken from a configuration file, that is why I tried to look for them. 
    
    I have found a file called configGenerator (line542) that seems to set those configurations to 60, but that maybe just a default configuration.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-149024165
  
    @rodrigo93, nice work. However, you forgot an entry in /cloud-client-ui/pom.xml.
    If that “cloud-plugin-storage-allocator-random” is not needed you should remove the dependency from “cloud-client-ui” project.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160884629
  
    @rafaelweingartner @rodrigo93 my bad I was merging against 4.6, not master. So far it has 0ne failure. I think it is a false positive so I am rerunning that test.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160291603
  
    PR rebased and commits squashed for @rodrigo93 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161045285
  
    @rafaelweingartner it is python not perl :) it is being read from the ms, and for the proper execution of the test it is not important what exactly it is, is it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161077855
  
    I ran #942 with a slimmed down version of the test file containing only the expunge test with a factor of 9. I will do some looking into 943 now and edit the test back to the original until it fails. If it does I'll share the stuff.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161552200
  
    @DaanHoogland @rafaelweingartner The failed expunge test was a false alarm due to our testing environment. It passes again now. Please ping me when there's another review so I can merge. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161551497
  
    LGTM as these integration tests still pass:
    
    ```
    nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=true \
    component/test_vpc_redundant.py \
    component/test_routers_iptables_default_policy.py \
    component/test_routers_network_ops.py \
    component/test_vpc_router_nics.py \
    smoke/test_loadbalance.py \
    smoke/test_internal_lb.py \
    smoke/test_ssvm.py \
    smoke/test_network.py
    
    ```
    
    Result:
    
    ```
    Create a redundant VPC with two networks with two VMs in each network ... === TestName: test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Status : SUCCESS ===
    ok
    Create a redundant VPC with two networks with two VMs in each network and check default routes ... === TestName: test_02_redundant_VPC_default_routes | Status : SUCCESS ===
    ok
    Create a redundant VPC with two networks with two VMs in each network ... === TestName: test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Status : SUCCESS ===
    ok
    Test iptables default INPUT/FORWARD policy on RouterVM ... === TestName: test_02_routervm_iptables_policies | Status : SUCCESS ===
    ok
    Test iptables default INPUT/FORWARD policies on VPC router ... === TestName: test_01_single_VPC_iptables_policies | Status : SUCCESS ===
    ok
    Test redundant router internals ... === TestName: test_01_isolate_network_FW_PF_default_routes_egress_true | Status : SUCCESS ===
    ok
    Test redundant router internals ... === TestName: test_02_isolate_network_FW_PF_default_routes_egress_false | Status : SUCCESS ===
    ok
    Test redundant router internals ... === TestName: test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | Status : SUCCESS ===
    ok
    Test redundant router internals ... === TestName: test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | Status : SUCCESS ===
    ok
    Create a VPC with two networks with one VM in each network and test nics after destroy ... === TestName: test_01_VPC_nics_after_destroy | Status : SUCCESS ===
    ok
    Create a VPC with two networks with one VM in each network and test default routes ... === TestName: test_02_VPC_default_routes | Status : SUCCESS ===
    ok
    Check the password file in the Router VM ... === TestName: test_isolate_network_password_server | Status : SUCCESS ===
    ok
    Check that the /etc/dhcphosts.txt doesn't contain duplicate IPs ... === TestName: test_router_dhcphosts | Status : SUCCESS ===
    ok
    Test to create Load balancing rule with source NAT ... === TestName: test_01_create_lb_rule_src_nat | Status : SUCCESS ===
    ok
    Test to create Load balancing rule with non source NAT ... === TestName: test_02_create_lb_rule_non_nat | Status : SUCCESS ===
    ok
    Test for assign & removing load balancing rule ... === TestName: test_assign_and_removal_lb | Status : SUCCESS ===
    ok
    Test to verify access to loadbalancer haproxy admin stats page ... === TestName: test02_internallb_haproxy_stats_on_all_interfaces | Status : SUCCESS ===
    ok
    Test create, assign, remove of an Internal LB with roundrobin http traffic to 3 vm's ... === TestName: test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Status : SUCCESS ===
    ok
    Test SSVM Internals ... === TestName: test_03_ssvm_internals | Status : SUCCESS ===
    ok
    Test CPVM Internals ... === TestName: test_04_cpvm_internals | Status : SUCCESS ===
    ok
    Test stop SSVM ... === TestName: test_05_stop_ssvm | Status : SUCCESS ===
    ok
    Test stop CPVM ... === TestName: test_06_stop_cpvm | Status : SUCCESS ===
    ok
    Test reboot SSVM ... === TestName: test_07_reboot_ssvm | Status : SUCCESS ===
    ok
    Test reboot CPVM ... === TestName: test_08_reboot_cpvm | Status : SUCCESS ===
    ok
    Test destroy SSVM ... === TestName: test_09_destroy_ssvm | Status : SUCCESS ===
    ok
    Test destroy CPVM ... === TestName: test_10_destroy_cpvm | Status : SUCCESS ===
    ok
    Test Remote Access VPN in VPC ... === TestName: test_vpc_remote_access_vpn | Status : SUCCESS ===
    ok
    Test VPN in VPC ... === TestName: test_vpc_site2site_vpn | Status : SUCCESS ===
    ok
    Test for port forwarding on source NAT ... === TestName: test_01_port_fwd_on_src_nat | Status : SUCCESS ===
    ok
    Test for port forwarding on non source NAT ... === TestName: test_02_port_fwd_on_non_src_nat | Status : SUCCESS ===
    ok
    Test for reboot router ... === TestName: test_reboot_router | Status : SUCCESS ===
    ok
    Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_1_static_nat_rule | Status : SUCCESS ===
    ok
    Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_2_nat_rule | Status : SUCCESS ===
    ok
    Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Status : SUCCESS ===
    ok
    
    ----------------------------------------------------------------------
    Ran 34 tests in 16239.372s
    
    OK
    ```
    
    
    And:
    
    ```
    nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=false \
    smoke/test_routers.py \
    smoke/test_network_acl.py \
    smoke/test_privategw_acl.py \
    smoke/test_reset_vm_on_reboot.py \
    smoke/test_vm_life_cycle.py \
    smoke/test_vpc_vpn.py \
    smoke/test_service_offerings.py \
    component/test_vpc_offerings.py \
    component/test_vpc_routers.py
    ```
    
    Result:
    
    ```
    Test router internal advanced zone ... === TestName: test_02_router_internal_adv | Status : SUCCESS ===
    ok
    Test restart network ... === TestName: test_03_restart_network_cleanup | Status : SUCCESS ===
    ok
    Test router basic setup ... === TestName: test_05_router_basic | Status : SUCCESS ===
    ok
    Test router advanced setup ... === TestName: test_06_router_advanced | Status : SUCCESS ===
    ok
    Test stop router ... === TestName: test_07_stop_router | Status : SUCCESS ===
    ok
    Test start router ... === TestName: test_08_start_router | Status : SUCCESS ===
    ok
    Test reboot router ... === TestName: test_09_reboot_router | Status : SUCCESS ===
    ok
    test_privategw_acl (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_privategw_acl | Status : SUCCESS ===
    ok
    Test reset virtual machine on reboot ... === TestName: test_01_reset_vm_on_reboot | Status : SUCCESS ===
    ok
    Test advanced zone virtual router ... === TestName: test_advZoneVirtualRouter | Status : SUCCESS ===
    ok
    Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : SUCCESS ===
    ok
    Test Multiple Deploy Virtual Machine ... === TestName: test_deploy_vm_multiple | Status : SUCCESS ===
    ok
    Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : SUCCESS ===
    ok
    Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : SUCCESS ===
    ok
    Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : SUCCESS ===
    ok
    Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status : SUCCESS ===
    ok
    Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status : SUCCESS ===
    ok
    Test migrate VM ... === TestName: test_08_migrate_vm | Status : SUCCESS ===
    ok
    Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm | Status : SUCCESS ===
    ok
    Test to create service offering ... === TestName: test_01_create_service_offering | Status : SUCCESS ===
    ok
    Test to update existing service offering ... === TestName: test_02_edit_service_offering | Status : SUCCESS ===
    ok
    Test to delete service offering ... === TestName: test_03_delete_service_offering | Status : SUCCESS ===
    ok
    Test for delete account ... === TestName: test_delete_account | Status : SUCCESS ===
    ok
    Test for Associate/Disassociate public IP address for admin account ... === TestName: test_public_ip_admin_account | Status : SUCCESS ===
    ok
    Test for Associate/Disassociate public IP address for user account ... === TestName: test_public_ip_user_account | Status : SUCCESS ===
    ok
    Test for release public IP address ... === TestName: test_releaseIP | Status : SUCCESS ===
    ok
    Test create VPC offering ... === TestName: test_01_create_vpc_offering | Status : SUCCESS ===
    ok
    Test VPC offering without load balancing service ... === TestName: test_03_vpc_off_without_lb | Status : SUCCESS ===
    ok
    Test VPC offering without static NAT service ... === TestName: test_04_vpc_off_without_static_nat | Status : SUCCESS ===
    ok
    Test VPC offering without port forwarding service ... === TestName: test_05_vpc_off_without_pf | Status : SUCCESS ===
    ok
    Test VPC offering with invalid services ... === TestName: test_06_vpc_off_invalid_services | Status : SUCCESS ===
    ok
    Test update VPC offering ... === TestName: test_07_update_vpc_off | Status : SUCCESS ===
    ok
    Test list VPC offering ... === TestName: test_08_list_vpc_off | Status : SUCCESS ===
    test_09_create_redundant_vpc_offering (integration.component.test_vpc_offerings.TestVPCOffering) ... === TestName: test_09_create_redundant_vpc_offering | Status : SUCCESS ===
    ok
    Test start/stop of router after addition of one guest network ... === TestName: test_01_start_stop_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test reboot of router after addition of one guest network ... === TestName: test_02_reboot_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test to change service offering of router after addition of one guest network ... === TestName: test_04_chg_srv_off_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test destroy of router after addition of one guest network ... === TestName: test_05_destroy_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test to stop and start router after creation of VPC ... === TestName: test_01_stop_start_router_after_creating_vpc | Status : SUCCESS ===
    ok
    Test to reboot the router after creating a VPC ... === TestName: test_02_reboot_router_after_creating_vpc | Status : SUCCESS ===
    ok
    Tests to change service offering of the Router after ... === TestName: test_04_change_service_offerring_vpc | Status : SUCCESS ===
    ok
    Test to destroy the router after creating a VPC ... === TestName: test_05_destroy_router_after_creating_vpc | Status : SUCCESS ===
    ok
    
    ----------------------------------------------------------------------
    Ran 42 tests in 8144.343s
    
    OK
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-151547183
  
    @remibergsma  I see. Next time I will try to rebase it with master before commiting. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-161027097
  
    @rafaelweingartner I agree with you in general lines. In this case the test had not just succeeded so far, it also started failing a lot since a few days. So something has definitely changed. I had not looked at the implemetation of the test to see the root cause yet but will have a look.
    As for the important bit: The integration tests are maybe a misleading name as I see them as functional enduser tests (for API users, not UI users). and not integration tests. In that sense we don't care about the expunge thread but we do want to see that VMs are expunged after a certain time. In fact the use case you mention as an alternative is not that, an alternative but another usecase that should also be tested.
    Your solution sounds fine to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8988: Removed unused adapters ...

Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160323858
  
    Hi guys.
    Sorry for the delay. Thank you all for helping me out. I had some personal problems.
    My bad.
    Thanks once again! I hope this commit will help in some way. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Removed unused adapters from async-job-co...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-150835745
  
    Thanks @rodrigo93, please also rebase with current master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---