You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by wilderrodrigues <gi...@git.apache.org> on 2015/06/25 15:01:07 UTC

[GitHub] cloudstack pull request: As an operator I want to be able to chang...

GitHub user wilderrodrigues opened a pull request:

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

    As an operator I want to be able to change the KVM hypervisor credentials

    API call (updateHostPassword) changes credentials on XenServer hypervisor but not on KVM. This should be extended also for KVM
    
    * Decouple the use of updateHostPassword
       - The code was hard to maintain because updating a host or all the hosts in a cluster was handled in the same method
       - Created updateHost and updateCluster password in both ResourceManager and ManagementServer interfaces/classes
       - The chck for whihc method to use is done in the API level
       - Started adding the support for KVM host password update
    * Fix a bug in the doUpdateHostPassword() method
      - 2 years ago a change in that method caused an issue when trying to update the hosts password: they would be updated in the database but not in the host it self, causing the hosts to disconnect after a management server restart.
    * Making possible to update KVM hosts password in the database via the API
       - Make sure the doUpdateHostPassword() method returns the result from the answer, not always true
       - Added the LibvirtUpdateHostPasswordCommandWrapper class, which will handle the changes in the KVM agent that will be further implemented in the future. The command will avoid the agent to complain about Unsupported command
       - Added a test to make sure the current implementation of the comment works. If changes in the future, it will also require changes. In that way, we make sure nobody will break it
    
    I already did basic manual tests with Management Server running on CentOS 7.1; 2 XenServer hosts; and 1 KVM host. All deployed in 2 advances zones. I was able to successfully execute:
    
    Update KVM host
    - update hostpassword hostid=95aa28ae-8038-4457-8951-504e760b53a4  username=root password=admin
    
    Update 1 XenServer host
    - update hostpassword hostid=9b236e34-0cc7-4d36-8b01-925fb10b4c25 username=root password=password
    
    Update a XenServer cluster
    - update hostpassword hostid=9b236e34-0cc7-4d36-8b01-925fb10b4c25 clusterid=fb58b84d-f9bb-4603-8b85-b56a49e1676f  username=root password=password
    
    I also created isolated networks and virtual machines. The machines deployed on the XenServer zone also used affinity groups. I deployed 1 virtual machine in the KVM zone.
    
     @DaanHoogland @remibergsma @bhaisaab 
    
    Please, do not merge yet. I will also run the integration/smoke tests.
    
    Cheers,
    Wilder

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

    $ git pull https://github.com/schubergphilis/cloudstack improvement/CS_180

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

    https://github.com/apache/cloudstack/pull/527.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 #527
    
----
commit 2ed74d51187e5e6706d6f5ea9ebb84d3cda15feb
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-24T11:04:35Z

    Initial work on improveing host password update
    
       - Formatting the code of ManagementServerImpl

commit 1eff70ad44f1df2947e47aac7e5d18e51f41a637
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-24T11:12:07Z

    Formatting the code of the ManagementService class

commit 21b8ea04bb205bc5b0c62a82eb8cb54730c49a42
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-24T11:33:04Z

    Formatting UpdateHostPasswordCmd class
    
       - Needs to be formatted before applying other changes

commit 5f6279135b8d7b58719b3791edd063cab0221d34
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-24T11:39:43Z

    Formatting the ResourceManagerImpl class
    
       - Needed to be formatted before applying the changes

commit e3ea66a507d2b32919789391106a4886a69a7d5f
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-24T11:59:50Z

    Decouple the use of updateHostPassword
    
       - The code was hard to maintain because updating a host or all the hosts in a cluster was handled in the same method
       - Created updateHost and updateCluster password in both ResourceManager and ManagementServer interfaces/classes
       - The chck for whihc method to use is done in the API level
       - Started adding the support for KVM host passwd update
    
    No API changes are needed and it will be backwards compatible.

commit d4e17e598ea74973ead2426e4aa250f9ef2fa4d4
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T06:33:52Z

    Add method implementation to the mock class.

commit 096639d90bb20364187e4b1828e6635d9c1b4ece
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T07:47:40Z

    Add supported hypervisors to a list
    
      - By doing so the check is simple and will require less effort when adding other hypervisors.

commit a8305ce3918995bfa286a1118b37590692437579
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T09:03:06Z

    Formatting the code of the AgentManagerImpl class

commit 0e2dd64addc5d561ec691210381f5bd8cc45952f
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T09:04:58Z

    Extracting local variable in order to make the code more readable.

commit 88420baabe369726d0b73e5f7a73843175551527
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T09:14:31Z

    In case we cannot update the password of all hosts in a cluster, please stop and notify the user about the failure.
    
       - The host id that failed to get updated will be informed in the error.

commit 6b5f56a8db6b2e49c7f444bd117e20a59b87b490
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T10:09:09Z

    Improving exception message
    
      - Adding both ID and UUID of the host

commit f2c55f8a7c62cefb869a4d9a7b4907c3d93d7885
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T11:29:22Z

    Fix a bug in the doUpdateHostPassword() method
    
      - 2 years ago a change in that method caused an issue when trying to update the hosts password:
        - they would be updated in the databse but not in the host it self, causing the hosts to disconnect after a management server resstart

commit 07cee4116c5a7a1b582181b20b034e8b344f6d9c
Author: wilderrodrigues <wr...@schubergphilis.com>
Date:   2015-06-25T12:32:51Z

    Making possible to update KVM hosts password in the database via the API
    
       - Make sure the doUpdateHostPassword() method returns the result from the answer, not always true
       - Added the LibvirtUpdateHostPasswordCommandWrapper class, which will handle the changes in the KVM agent
         That's will be further implemented in the future. The command will avoid the agent to complain about Unsupported command
       - Added a test to make sure the current implementation of the comment works
         - If changes in the future, it will also require changes. In that way, we make sure nobody will break 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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115250930
  
    Indeed. :)
    
    I will create the ticket on ACS Jira and add to the  PR description. 
    
    Cheers,
    Wilder


---
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: As an operator I want to be able to chang...

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

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


---
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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115258835
  
    @wilderrodrigues also squash them :) will this change passwords on the hosts?


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

Re: [GitHub] cloudstack pull request: As an operator I want to be able to chang...

Posted by Daan Hoogland <da...@gmail.com>.
John,

The descriptive message should be in the merge commit. It should state
the cause of the collective. each individual should have a smaller
message stating the reason for a simple atomic change. Taking what you
say here we could squah the whole repo with the message 'making an
cloud orchestration platform'.
What I mean by this is that at any level of squashing the intent of
the work will become meaningless at some point.


On Wed, Jul 1, 2015 at 8:58 PM, John Burwell <jo...@shapeblue.com> wrote:
> Daan,
>
> I disagree regarding the squash.  As I read through the list of commits, they read as a series of small commits that do not explain the overall change.  When this batch of small of commits is merged into the large history of master, the intent of this work will be lost.  In my opinion, squashing them into one commit with a descriptive commit message will be much more helpful to people trying to comprehend the work at some point in the future.
>
> Thanks,
> -John
>
> ---
> John Burwell (@john_burwell)
> VP of Software Engineering, ShapeBlue
> (571) 403-2411 | +44 20 3603 0542
> http://www.shapeblue.com
>
>
>
>> On Jun 29, 2015, at 5:30 AM, Daan Hoogland <da...@gmail.com> wrote:
>>
>> Do not squash commits and hide history unless it makes reading it much
>> and obviously clearer. It is only good in my not so humble opinion in
>> case of to and fro commits where a thing has been reverted and
>> reapplied and reverted again and so on.
>>
>> Also do not rebase commits but make merge commits, this make seeing
>> what happened when much easier and also make reverting of big chunks
>> of work easier: if it seems to work merge, if it turns out it doesn't
>> revert the merge fix and merge again.
>>
>> €0,02
>>
>>
>> On Mon, Jun 29, 2015 at 11:07 AM, wilderrodrigues <gi...@git.apache.org> wrote:
>>> Github user wilderrodrigues commented on the pull request:
>>>
>>>    https://github.com/apache/cloudstack/pull/527#issuecomment-116545842
>>>
>>>    I created the issue and added to the last commit. Did it last week:
>>>
>>>    https://issues.apache.org/jira/browse/CLOUDSTACK-8589
>>>
>>>    Perhaps you missed that.
>>>
>>>    Concerning squashed commits, ew should really have discussed about it when you were here. I'm 100% if I squash my atomic commits next time you guys will complain. It seems I haven't been clear enough about that yet.
>>>
>>>    @remibergsma @miguelaferreira Would you guys give your input on these "squash commits" matter? I'm a bit tired of this discussion. For me, squashed commits won't make a review easier, unless there is no review at all or if the committer has changes a couple of lines in a bash script.
>>>
>>>    What I would like to see is people testing changes the way I do.
>>>
>>>    Cheers,
>>>    Wilder
>>>
>>>
>>> ---
>>> 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.
>>> ---
>>
>>
>>
>> --
>> Daan
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
> CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
> CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
>
> This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.



-- 
Daan

Re: [GitHub] cloudstack pull request: As an operator I want to be able to chang...

Posted by John Burwell <jo...@shapeblue.com>.
Daan,

I disagree regarding the squash.  As I read through the list of commits, they read as a series of small commits that do not explain the overall change.  When this batch of small of commits is merged into the large history of master, the intent of this work will be lost.  In my opinion, squashing them into one commit with a descriptive commit message will be much more helpful to people trying to comprehend the work at some point in the future.

Thanks,
-John

---
John Burwell (@john_burwell)
VP of Software Engineering, ShapeBlue
(571) 403-2411 | +44 20 3603 0542
http://www.shapeblue.com



> On Jun 29, 2015, at 5:30 AM, Daan Hoogland <da...@gmail.com> wrote:
>
> Do not squash commits and hide history unless it makes reading it much
> and obviously clearer. It is only good in my not so humble opinion in
> case of to and fro commits where a thing has been reverted and
> reapplied and reverted again and so on.
>
> Also do not rebase commits but make merge commits, this make seeing
> what happened when much easier and also make reverting of big chunks
> of work easier: if it seems to work merge, if it turns out it doesn't
> revert the merge fix and merge again.
>
> €0,02
>
>
> On Mon, Jun 29, 2015 at 11:07 AM, wilderrodrigues <gi...@git.apache.org> wrote:
>> Github user wilderrodrigues commented on the pull request:
>>
>>    https://github.com/apache/cloudstack/pull/527#issuecomment-116545842
>>
>>    I created the issue and added to the last commit. Did it last week:
>>
>>    https://issues.apache.org/jira/browse/CLOUDSTACK-8589
>>
>>    Perhaps you missed that.
>>
>>    Concerning squashed commits, ew should really have discussed about it when you were here. I'm 100% if I squash my atomic commits next time you guys will complain. It seems I haven't been clear enough about that yet.
>>
>>    @remibergsma @miguelaferreira Would you guys give your input on these "squash commits" matter? I'm a bit tired of this discussion. For me, squashed commits won't make a review easier, unless there is no review at all or if the committer has changes a couple of lines in a bash script.
>>
>>    What I would like to see is people testing changes the way I do.
>>
>>    Cheers,
>>    Wilder
>>
>>
>> ---
>> 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.
>> ---
>
>
>
> --
> Daan

Find out more about ShapeBlue and our range of CloudStack related services

IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>

This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.

RE: [GitHub] cloudstack pull request: As an operator I want to be able to chang...

Posted by Stephen Turner <St...@citrix.com>.
I agree. One enormous change isn't useful when you try and understand the changes later. Github lets you view either the diff for the pull request as a whole or for individual commits, so there should be no reason to squash them.

-- 
Stephen Turner


-----Original Message-----
From: Daan Hoogland [mailto:daan.hoogland@gmail.com] 
Sent: 29 June 2015 10:30
To: dev
Subject: Re: [GitHub] cloudstack pull request: As an operator I want to be able to chang...

Do not squash commits and hide history unless it makes reading it much and obviously clearer. It is only good in my not so humble opinion in case of to and fro commits where a thing has been reverted and reapplied and reverted again and so on.

Also do not rebase commits but make merge commits, this make seeing what happened when much easier and also make reverting of big chunks of work easier: if it seems to work merge, if it turns out it doesn't revert the merge fix and merge again.

€0,02


On Mon, Jun 29, 2015 at 11:07 AM, wilderrodrigues <gi...@git.apache.org> wrote:
> Github user wilderrodrigues commented on the pull request:
>
>     
> https://github.com/apache/cloudstack/pull/527#issuecomment-116545842
>
>     I created the issue and added to the last commit. Did it last week:
>
>     https://issues.apache.org/jira/browse/CLOUDSTACK-8589
>
>     Perhaps you missed that.
>
>     Concerning squashed commits, ew should really have discussed about it when you were here. I'm 100% if I squash my atomic commits next time you guys will complain. It seems I haven't been clear enough about that yet.
>
>     @remibergsma @miguelaferreira Would you guys give your input on these "squash commits" matter? I'm a bit tired of this discussion. For me, squashed commits won't make a review easier, unless there is no review at all or if the committer has changes a couple of lines in a bash script.
>
>     What I would like to see is people testing changes the way I do.
>
>     Cheers,
>     Wilder
>
>
> ---
> 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.
> ---



--
Daan

Re: [GitHub] cloudstack pull request: As an operator I want to be able to chang...

Posted by Daan Hoogland <da...@gmail.com>.
Do not squash commits and hide history unless it makes reading it much
and obviously clearer. It is only good in my not so humble opinion in
case of to and fro commits where a thing has been reverted and
reapplied and reverted again and so on.

Also do not rebase commits but make merge commits, this make seeing
what happened when much easier and also make reverting of big chunks
of work easier: if it seems to work merge, if it turns out it doesn't
revert the merge fix and merge again.

€0,02


On Mon, Jun 29, 2015 at 11:07 AM, wilderrodrigues <gi...@git.apache.org> wrote:
> Github user wilderrodrigues commented on the pull request:
>
>     https://github.com/apache/cloudstack/pull/527#issuecomment-116545842
>
>     I created the issue and added to the last commit. Did it last week:
>
>     https://issues.apache.org/jira/browse/CLOUDSTACK-8589
>
>     Perhaps you missed that.
>
>     Concerning squashed commits, ew should really have discussed about it when you were here. I'm 100% if I squash my atomic commits next time you guys will complain. It seems I haven't been clear enough about that yet.
>
>     @remibergsma @miguelaferreira Would you guys give your input on these "squash commits" matter? I'm a bit tired of this discussion. For me, squashed commits won't make a review easier, unless there is no review at all or if the committer has changes a couple of lines in a bash script.
>
>     What I would like to see is people testing changes the way I do.
>
>     Cheers,
>     Wilder
>
>
> ---
> 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.
> ---



-- 
Daan

[GitHub] cloudstack pull request: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-116545842
  
    I created the issue and added to the last commit. Did it last week:
    
    https://issues.apache.org/jira/browse/CLOUDSTACK-8589
    
    Perhaps you missed that.
    
    Concerning squashed commits, ew should really have discussed about it when you were here. I'm 100% if I squash my atomic commits next time you guys will complain. It seems I haven't been clear enough about that yet.
    
    @remibergsma @miguelaferreira Would you guys give your input on these "squash commits" matter? I'm a bit tired of this discussion. For me, squashed commits won't make a review easier, unless there is no review at all or if the committer has changes a couple of lines in a bash script.
    
    What I would like to see is people testing changes the way I do.
    
    Cheers,
    Wilder


---
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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115596376
  
    XenServer 6.2 test report:
    
    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 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 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 VPN in VPC ... === TestName: test_vpc_remote_access_vpn | Status : SUCCESS ===
    ok
    Test VPN in VPC ... === TestName: test_vpc_site2site_vpn | Status : SUCCESS ===
    ok
    
    ----------------------------------------------------------------------
    Ran 22 tests in 2634.005s
    
    OK
    /tmp//MarvinLogs/test_vpc_vpn_YESELF/results.txt (END)
    
    * Updating single XenServer host:
    
    update hostpassword hostid=2ffa8219-08cb-4014-bcb6-49f58ca0c9b6 username=root password=admin
    
    2015-06-26 05:12:36,470 DEBUG [c.c.s.ManagementServerImpl] (2135942710@qtp-1568127297-46:ctx-e783efbc ctx-0852e7de ctx-9829cd2e) Changing password for host name = xen1
    2015-06-26 05:12:36,499 DEBUG [c.c.a.t.Request] (2135942710@qtp-1568127297-46:ctx-e783efbc ctx-0852e7de ctx-9829cd2e) Seq 2-8034984685182386389: Sending  { Cmd , MgmtId: 3232241213, via: 2(xen1), Ver: v1, Flags: 100011, [{"com.cloud.agent.api.UpdateHostPasswordCommand":{"
    wait":0}}] }
    
    * Updating XenServer cluster:
    
    update hostpassword username=root password=admin clusterid=63b8ff8c-4d0c-4911-aad4-6c30a3257a29
    
    2015-06-26 05:14:47,596 DEBUG [c.c.s.ManagementServerImpl] (2135942710@qtp-1568127297-46:ctx-5144239f ctx-36c6878b ctx-d371bef2) Changing password for host name = xen1
    2015-06-26 05:14:47,599 DEBUG [c.c.s.ManagementServerImpl] (2135942710@qtp-1568127297-46:ctx-5144239f ctx-36c6878b ctx-d371bef2) Changing password for host name = xen2
    2015-06-26 05:14:47,614 DEBUG [c.c.a.t.Request] (2135942710@qtp-1568127297-46:ctx-5144239f ctx-36c6878b ctx-d371bef2) Seq 2-8034984685182386394: Sending  { Cmd , MgmtId: 3232241213, via: 2(xen1), Ver: v1, Flags: 100011, [{"com.cloud.agent.api.UpdateHostPasswordCommand":{"wait":0}}] }
    2015-06-26 05:14:47,614 DEBUG [c.c.a.t.Request] (2135942710@qtp-1568127297-46:ctx-5144239f ctx-36c6878b ctx-d371bef2) Seq 2-8034984685182386394: Executing:  { Cmd , MgmtId: 3232241213, via: 2(xen1), Ver: v1, Flags: 100011, [{"com.cloud.agent.api.UpdateHostPasswordCommand":{"wait":0}}] }
    
    I will now test the KVM host


---
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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115249502
  
    Would it be too much to ask for a corresponding Jira ticket? Definitely makes it easier to track changes when writing release notes :-)


---
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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115289744
  
    Hi @bhaisaab 
    
    No, it doesn't change pwd in the hosts. I will implement that in a separate story/issue.
    
    Concerning the squash, I won't do it. Why? The commits are atomic, which makes easier to review (e.g. one doesn't need t review commits that say "formatting code..."). I split code formatting from the fixes. Another point, if there is a change in a given commit, I will change just that commit and push it, which reduces the work for the person behind the fixes as well. On a third point, if we squash atomic commits, the reviewer will have the whole huge thing to look at, as everything has just be done together.
    
    In short, squashing those atomic commits will bring no benefit at all.
    
    Squash would only help if someone messes up with the commits and have everything tangled, which is not the case with this PR.
    
    If you look at the bullet point on the PR description, those are the most relevant commits to look at. I even took care of using the same commit title for the bullet points.
    
    I will start testing this evening.
    
    Cheers,
    Wilder


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

Re: [GitHub] cloudstack pull request: As an operator I want to be able to chang...

Posted by Daan Hoogland <da...@gmail.com>.
Wilder, then rebase and amend the comment as well to include the ticket number

On Thu, Jun 25, 2015 at 3:12 PM, terbolous <gi...@git.apache.org> wrote:
> Github user terbolous commented on the pull request:
>
>     https://github.com/apache/cloudstack/pull/527#issuecomment-115251794
>
>     Excellent, 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.
> ---



-- 
Daan

[GitHub] cloudstack pull request: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115251794
  
    Excellent, 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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-116535759
  
    I was with @bhaisaab on this, I would have like to see a JIRA issue. We did not call it so yet, but we should consider being in master freeze right now, and only things with proper JIRA ticket should be merged. I would also prefer squashed commits...


---
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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115416654
  
    @wilderrodrigues it's difficult to track changes, against JIRA which is why I asked for them to be squashed and use the CLOUDSTACK-8589. I think a single commit to capture the changes would be cleaner, but that just my aesthetics preferences. Please merge when you're done testing, 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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-115569008
  
    Hi @bhaisaab 
    
    Unfortunately I forgot to create the issue before the commits, otherwise it wouldn't be a problem now.
    
    Since I added the issue number in the last commit, I hope it won't cause pain during 4.6 release. In any case, I will double the attention before the PRs and will also look closer to the 4.6 release and help with any thing the pops up under my name.
    
    Cheers,
    Wilder 


---
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: As an operator I want to be able to chang...

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

    https://github.com/apache/cloudstack/pull/527#issuecomment-117186219
  
    As long as people make good commits, that are atomic, capture relevant changes together, and have descriptive messages, IMHO squashing only make things worse.
    
    If you squash commits, all you get is less commits. Some people might say a cleaner master branch.
    I won't discuss aesthetics here. I will try to be objective.
    
    With less commits you also get larger commits. That means that in a change set (e.g. a PR) where a large number of changes are made, these changes are all bundled together. That forces the reviewer to take all the context at once. 
    With more commits you get smaller commits. That allows the reviewer to take chunks of context at a time. And, in addition, if the reviewer wants/needs to get all the context at once, it is easy to make a diff between the first and the last commit in the change set (Github offers both per commit view and total diff view, just see the tabs on top of this PR).
    
    If you will, this is comparable to reviewing a class will several small methods or reviewing a class with one big method.
    
    Another interesting aspect that should be considered is reverting commits. As you may all know this can happen quite frequently. With smaller, atomic commits, it is easer to revert a change (e.g a bug introducing change) in a particular part of the code (while leaving the rest of the code untouched). It is easier because (1) the small commit isolates the change; (2) in the presence of conflicts these can be fixed iteratively, in small steps.
    
    Finally, I would like to point out that smaller commits allow the developer to convey more information about each change. If you squash a set of commits you loose the description of all the small changes.
    Do we really want to go towards less information about code changes? Or do we want to have the developers write a bullet list of the changes they made and assign them to parts of the code? There simply isn't any other way to preserve this information.
    
    Of course that, as I already said, all this only makes sense if:
    * commits are small;
    * atomic;
    * have good descriptions.
    
    And that is, simply put, entry-level development discipline.



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