You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by ProjectMoon <gi...@git.apache.org> on 2015/11/23 18:20:40 UTC
[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus
GitHub user ProjectMoon opened a pull request:
https://github.com/apache/cloudstack/pull/1111
Fix event UUIDS missing on event bus
The fixing of CLOUDSTACK-8816 introduced a regression that removed the first class entities in the event bus description property. This is because everything was changed to use the Class as a key... Everything but the populateFirstClassEntities method in ActionEventUtils.
This commit tries to load the class key instead of the String key, which re-enables the populateFirstClassEntities method.
Likely this was not caught because of the trace exception handling... maybe some better logging/unit tests would be good for this PR.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/greenqloud/cloudstack pr-eventbus-entity-uuids
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1111.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 #1111
----
commit a528c396d718f0ea6eecbdda7c23c8d7f670d99c
Author: jeff <je...@greenqloud.com>
Date: 2015-11-23T17:15:57Z
Fix event UUIDS missing on event bus
The fixing of CLOUDSTACK-8816 introduced a regression that removed the
first class entities in the event bus description property. This is
because everything was changed to use the Class as a key... Everything
but the populateFirstClassEntities method in ActionEventUtils.
----
---
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: Fix event UUIDS missing on event bus
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159568824
@remibergsma @DaanHoogland can you take a look at this please? Its a good fix to have in 4.6.1
I didnt run the test suite. But, manually tested this fix.
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159024273
Integration tests use Marvin, right?
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159001385
To test, remote debug an in-memory event bus with a command like disable or enable static NAT. Check event description property before and after. If this commit is applied, it will have the UUIDs of the first class entities involved in the API call.
I think I will write a unit test for this to make sure this method is working. What file should I put it in?
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159889056
Closed in favor of #1127.
---
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: Fix event UUIDS missing on event bus
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159833976
@ProjectMoon there was one failure in the network tests for the password server. I am rerunning that one to make sure.
[1111.network.results.txt](https://github.com/apache/cloudstack/files/44693/1111.network.results.txt)
[1111.vpc.results.txt](https://github.com/apache/cloudstack/files/44694/1111.vpc.results.txt)
---
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: Fix event UUIDS missing on event bus
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159586201
@karuturi I don't think there is a test for this in the suites we use. I'll them anyway to at least see about regression in some other places.
---
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: Fix event UUIDS missing on event bus
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159834901
```
Check the password file in the Router VM ... === TestName: test_isolate_network_password_server | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 1 test in 367.955s
OK
```
So it was a fluke
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon closed the pull request at:
https://github.com/apache/cloudstack/pull/1111
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159231415
For now I will cherry-pick that commit and use it locally to build my tests. It will not become part of this pull request of course.
---
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: Fix event UUIDS missing on event bus
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159834948
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: Fix event UUIDS missing on event bus
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159318964
Hi @ProjectMoon Thanks for taking this up. If possible, can you put in a sample event before and after the fix?
I will test this tomorrow with rabbitmq
---
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: Fix event UUIDS missing on event bus
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159883325
I just realised the target branch should be 4.6 meaning the PR should be for 4.6 and fwd-merged to master. Can you take care of that please @ProjectMoon
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159277838
The commit has been updated with a unit test. I also rebased this branch to latest master since my apparent accidental review of #1110 got the syntax error fix into the master branch.
---
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: Fix event UUIDS missing on event bus
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159884682
No cherry-picking. The PR should be only for 4.6 which will be fwd-merged to master(and hence will be available in 4.7 and up)
I wish it was as easy as editing this PR and changing the target branch. But, github doesnt provide that and a new PR is required. Since the commit ids are same, when the 4.6 PR is merged both will be marked merged. (or you could close this now itself and just reference this PR in the new PR)
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Release+principles+for+Apache+CloudStack+4.6+and+up
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159341450
Example with the fix:
```
{
eventDateTime=2015-11-24 16:57:58 +0000,
status=Completed,
description=Successfully completed enabling static nat. Ip Id: 4,
event=STATICNAT.ENABLE,
entityuuid=null,
entity=null,
account=7622afee-8ec9-11e5-9cba-54ee754a4435,
IpAddress=4073023a-1734-4af9-b4e7-79999b39d176,
VirtualMachine=7ea2776f-d8e7-42da-8be1-d93d161b613a,
user=7622d527-8ec9-11e5-9cba-54ee754a4435
}
```
Without the fix:
```
{
eventDateTime=2015-11-24 17:01:38 +0000,
status=Completed,
description=Successfully completed enabling static nat. Ip Id: 4,
event=STATICNAT.ENABLE,
entityuuid=null,
entity=null,
account=7622afee-8ec9-11e5-9cba-54ee754a4435,
user=7622d527-8ec9-11e5-9cba-54ee754a4435
}
```
The first class entities found in the case of static NAT operations are VM ID and IP ID.
---
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: Fix event UUIDS missing on event bus
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159001728
@ProjectMoon server/test/com/cloud/event/ActionEventUtilsTest.java
and if you can maybe an integration 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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159219262
Currently unable to build the branch due to https://github.com/apache/cloudstack/pull/1110. What's the status of that request being merged in?
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159887285
Since the branches are different, I will create a new branch off of 4.6, cherry-pick my commit to it, and then submit that as a new PR, which can then be forward merged.
---
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: Fix event UUIDS missing on event bus
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159883807
Well, the target branch for this is master so it will be put in 4.7-SNAPSHOT, but it's also intended to be cherry-picked back into 4.6 (which I obviously do not have the access to do).
I can create a second pull request on the 4.6 branch as well if you want. What is the proper procedure for 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: Fix event UUIDS missing on event bus
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159042018
Yes, preferably but not necessarily. you can create your own if you want, based on scripts and cloudmonkey for instance or a similar tool.
---
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: Fix event UUIDS missing on event bus
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159230031
It is missing a review and LGTM, (with proper motivation)
---
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: Fix event UUIDS missing on event bus
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/1111#issuecomment-159519154
manually tested it with rabbitmq integration
Here are the events before and after the fix
Before:
{"eventDateTime":"2015-11-25 11:39:27 +0530","status":"Completed","description":"Successfully completed enabling static nat. Ip Id: 4","event":"STATICNAT.ENABLE","account":"bd73dc2e-35c0-11e5-b094-d4ae52cb9af0","user":"bd7ea748-35c0-11e5-b094-d4ae52cb9af0"}
After:
{"eventDateTime":"2015-11-25 12:03:40 +0530","status":"Completed","description":"Successfully completed enabling static nat. Ip Id: 4","event":"STATICNAT.ENABLE","account":"bd73dc2e-35c0-11e5-b094-d4ae52cb9af0","IpAddress":"0d1e8fd3-e48d-43d7-b2c0-f76ba0cbcf5a","VirtualMachine":"aa9d4cfe-f42a-4b27-acd5-c5ded84ae668","user":"bd7ea748-35c0-11e5-b094-d4ae52cb9af0"}
:+1:
---
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.
---