You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by kollyma <gi...@git.apache.org> on 2016/03/20 21:02:23 UTC

[GitHub] cloudstack pull request: speedup iptables setup

GitHub user kollyma opened a pull request:

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

    speedup iptables setup

    PR against 4.7 as discussed with Remi Bergsma. This will speed up the iptables creation on the virtual router. 
    
    Testing showed the following: 
    with current code:
    root@kvm704:~# time /usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh vr_cfg.sh 169.254.1.176 -c /var/cache/cloud/VR-12f28879-de7e-44d2-8dbe-b93a04bd3ba4.cfg
    real    2m56.401s
    user    0m0.012s
    sys    0m0.012s
    
    modified version:
    root@kvm704:~# time /usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh vr_cfg.sh 169.254.1.176 -c /var/cache/cloud/VR-12f28879-de7e-44d2-8dbe-b93a04bd3ba4.cfg
    real    1m35.762s
    user    0m0.020s
    sys    0m0.004s

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

    $ git pull https://github.com/kollyma/cloudstack 4.7

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

    https://github.com/apache/cloudstack/pull/1449.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 #1449
    
----
commit 3c432934b2485685ee2f2ab4fbfc9340a72b4f0d
Author: kollyma <ma...@swisstxt.ch>
Date:   2016-03-20T19:52:30Z

    speedup iptables setup
    
    Testing showed the following: 
    with current code:
    root@kvm704:~# time /usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh vr_cfg.sh 169.254.1.176 -c /var/cache/cloud/VR-12f28879-de7e-44d2-8dbe-b93a04bd3ba4.cfg
    real    2m56.401s
    user    0m0.012s
    sys    0m0.012s
    
    modified version:
    root@kvm704:~# time /usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh vr_cfg.sh 169.254.1.176 -c /var/cache/cloud/VR-12f28879-de7e-44d2-8dbe-b93a04bd3ba4.cfg
    real    1m35.762s
    user    0m0.020s
    sys    0m0.004s

----


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-204985068
  
    @kollyma Can you squash the commits for me and I will then merge this in.  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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-204633944
  
    
    ### CI Results
    
    
    
    
    ### 84/85 SUCCESSFUL
    
    **1 Failure** (this and another similar test seem to fail consistently in my CI currently)
    ```
    FAIL: Test redundant router internals
    -------------------------------------
    Traceback (most recent call last):
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_routers_network_ops.py", line 483, in test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false
        "Attempt to retrieve google.com index page should be successful once rule is added!"
    AssertionError: Attempt to retrieve google.com index page should be successful once rule is added!
    ```
    
    Ran the following tests...
    ```
    echo "Running tests with required_hardware=true"
    nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=true \
    smoke/test_password_server.py \
    smoke/test_vpc_redundant.py \
    smoke/test_routers_iptables_default_policy.py \
    smoke/test_routers_network_ops.py \
    smoke/test_vpc_router_nics.py \
    smoke/test_router_dhcphosts.py \
    smoke/test_loadbalance.py \
    smoke/test_internal_lb.py \
    smoke/test_ssvm.py \
    smoke/test_vpc_vpn.py \
    smoke/test_privategw_acl.py \
    smoke/test_network.py
    
    echo "Running tests with required_hardware=false"
    nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=false \
    smoke/test_routers.py \
    smoke/test_network_acl.py \
    smoke/test_reset_vm_on_reboot.py \
    smoke/test_vm_life_cycle.py \
    smoke/test_service_offerings.py \
    smoke/test_network.py \
    component/test_vpc_offerings.py \
    component/test_vpc_routers.py
    ```
    
    
    **Associated Uploads**
    
    **`test_network_1WIKLA`:**
    
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1449/test_network_1WIKLA/results.txt)
    
    **`test_vpc_routers_8ZWKPC`:**
    
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1449/test_vpc_routers_8ZWKPC/results.txt)
    
    
    Uploads will be available until `2016-06-02 00:00:00 +0000 GMT`
    
    *Comment created by [`upr comment`](https://github.com/swill/upr).*
    



---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-199709347
  
    The third time is the charm,all checks have passed now! the first time Jenkins failed and strangely enough the second time Travis failed. 


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-204532720
  
    @kollyma Please squash the commits.
    @swill Let's run integration tests on this before merge.


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-199059690
  
    @kollyma I like what you did here. Besides apparently speeding up the iptables setup, the code looks more streamlined this way. 
    But it seems your build timed out. You might want to do another push later.


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-202819883
  
    @swill it is our responsibility as committers to make sure that it is merged against the right branch. if it is not a new feature and at the moment 4.7 is the branch to go for. merging forward to 4.8. and master/4.9 is the right procedure.


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-204500797
  
    LGTM! please merge


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-200296178
  
    Good job. Thanks.
    Works fine in my 4.8 test installation (real platform)
    Several tests on RV and RVR routers with network restart with/without clean up
    LGTM


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-199257722
  
    @cristofolini : thanks! indeed the "Apache CloudStack Plugin - Quota Service" timed out. After reviewing the PR and the Jenkins output, I can not see any relations. Should I open another pull request with the same code and check if it works? Did I understand this correctly? 


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-208777850
  
    @kollyma can you close this one if #1487 is indeed the same?


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-208506127
  
    During commit squashing something went wrong.. i could not revert. Here a new Pull request with exactly the same changes. Sorry for any inconvenience caused.. and thanks for your "LGTM"
    https://github.com/apache/cloudstack/pull/1487


---
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: speedup iptables setup

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

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


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-201531706
  
    I am pretty comfortable with this PR given the scope of the change.  Thanks @milamberspace for testing.  Should this be committed against 4.7 and then forward merged?  I am not entirely sure what the guidelines are for the types of changes that should be merged into previous releases.  I know features should be against `master`, but I am not sure the details beyond that.  @remibergsma do you have any guidance on this topic?  Obviously the branch a PR it is opened against suggests a course of action, but I would like to have a clearer understanding of 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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-204764789
  
    @swill In the logs I see this:
    
    ```
    {returns: [u'--2016-04-01 21:41:30--  http://www.google.com/', u'Resolving www.google.com... 2607:f8b0:4006:807::2004, 172.217.0.36', u'Connecting to www.google.com|2607:f8b0:4006:807::2004|:80... failed: Network is unreachable.', u'Connecting to www.google.com|172.217.0.36|:80... connected.', u'HTTP request sent, awaiting response... 302 Found', u'Location: http://www.google.ca/?gfe_rd=cr&ei=Cuv-Vo3NLuih8wfM-b6oAw [following]', u'--2016-04-01 21:41:30--  http://www.google.ca/?gfe_rd=cr&ei=Cuv-Vo3NLuih8wfM-b6oAw', u'Resolving www.google.ca... failed: Connection timed out.', u"wget: unable to resolve host address 'www.google.ca'"]}
    root: DEBUG: Result from SSH into the Virtual Machine: [u'--2016-04-01 21:41:30--  http://www.google.com/', u'Resolving www.google.com... 2607:f8b0:4006:807::2004, 172.217.0.36', u'Connecting to www.google.com|2607:f8b0:4006:807::2004|:80... failed: Network is unreachable.', u'Connecting to www.google.com|172.217.0.36|:80... connected.', u'HTTP request sent, awaiting response... 302 Found', u'Location: http://www.google.ca/?gfe_rd=cr&ei=Cuv-Vo3NLuih8wfM-b6oAw [following]', u'--2016-04-01 21:41:30--  http://www.google.ca/?gfe_rd=cr&ei=Cuv-Vo3NLuih8wfM-b6oAw', u'Resolving www.google.ca... failed: Connection timed out.', u"wget: unable to resolve host address 'www.google.ca'"]
    ```
    
    It resolved `google.com` but then gets redirected to `www.google.ca` which it cannot resolve due to a timeout. Looks like a local issue, indeed. In a newer version of the test we are testing the public gateway instead but that hasn't been merged / sent as PR I guess.



---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-204537583
  
    @remibergsma The test have already been started.  :)


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-199517201
  
    @kollyma, maybe just add a comment at the top to mention something like "pre-fetch the variables so they are not done inline" or something like that and this should prompt another build without having to create a new PR.  Since we can't close a PR without a merge, this is probably the simplest way to handle it.  Good comments are rarely a bad thing.


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-205809122
  
    @Swill: tried to squash the commits shortly from holidays. Did not worked as expected, messed up my PR. I need to debug this when I am back next week...


---
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: speedup iptables setup

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

    https://github.com/apache/cloudstack/pull/1449#issuecomment-204634361
  
    The debut of `upr`, so let me know if you want me to change the format.  Also if you want me to add more tests in future runs, please let me know...
    
    Given these results and the fact that the one failure consistently fails in my environment, I think we are ready to merge...


---
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.
---