You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by geomacy <gi...@git.apache.org> on 2017/01/19 17:48:47 UTC
[GitHub] brooklyn-server pull request #530: JcloudsLocation is not releasing its cust...
GitHub user geomacy opened a pull request:
https://github.com/apache/brooklyn-server/pull/530
JcloudsLocation is not releasing its customizers (BROOKLYN-427)
preRelease and postRelease of JcloudsLocationCustomizers are not being
called as expected during the release of a Jclouds machine location.
The problem is that the "setup" used in JcloudsLocation.obtainOnce includes a copy of the flags passed from MachineLifecycleEffectorTasks#stopAnyProvisionedMachines,
which includes the customizers. However, the "setup" used in the "release"
method of JcloudsLocation is taken from the JcloudsLocation itself, which
is the provisioning (i.e. parent) location, which doesn't have those
customizers in it.
It would actually make more sense to get the location customizers from
the machine location, as a given parent could have mutiple machines
with different customizers.
The changes here include an addition to the
`JcloudsCustomizerInstantiationYamlDslTest#testCustomizers`
to demonstrate the problem, and a fix that adds the config to
the machine location.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/geomacy/brooklyn-server jclc-release
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/530.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 #530
----
commit 7b805a73da1b75de1f0f0b981f377c83ffb37bb9
Author: Geoff Macartney <ge...@cloudsoftcorp.com>
Date: 2017-01-19T17:10:45Z
preRelease and postRelease of JcloudsLocationCustomizers are not called on stop
commit 432823a333929ba58cad7d5219679c78e3e92ac8
Author: Geoff Macartney <ge...@cloudsoftcorp.com>
Date: 2017-01-19T17:28:31Z
Fix broken test for preRelease/postRelease
----
---
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] brooklyn-server issue #530: JcloudsLocation is not releasing its customizers...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/530
let's try 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] brooklyn-server pull request #530: JcloudsLocation is not releasing its cust...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/530
---
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] brooklyn-server pull request #530: JcloudsLocation is not releasing its cust...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/530#discussion_r97329773
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -1065,6 +1066,13 @@ protected MachineLocation obtainOnce(ConfigBag setup) throws NoMachinesAvailable
LOG.warn("Problem generating log message summarising completion of jclouds machine provisioning "+machineLocation+" by "+this, e);
}
+ if (customizers.size() > 0) {
+ machineLocation.config().set(JCLOUDS_LOCATION_CUSTOMIZERS, customizers);
+ }
+ if (machineCustomizers.size() > 0) {
+ machineLocation.config().set(MACHINE_LOCATION_CUSTOMIZERS, machineCustomizers);
+ }
--- End diff --
Can you instead add the config for `JCLOUDS_LOCATION_CUSTOMIZERS` at [construction time](https://github.com/apache/brooklyn-server/blob/master/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java#L2056-L2099) so migrated settings are all at one place.
---
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] brooklyn-server pull request #530: JcloudsLocation is not releasing its cust...
Posted by geomacy <gi...@git.apache.org>.
GitHub user geomacy reopened a pull request:
https://github.com/apache/brooklyn-server/pull/530
JcloudsLocation is not releasing its customizers (BROOKLYN-427)
preRelease and postRelease of JcloudsLocationCustomizers are not being
called as expected during the release of a Jclouds machine location.
The problem is that the "setup" used in JcloudsLocation.obtainOnce includes a copy of the flags passed from MachineLifecycleEffectorTasks#stopAnyProvisionedMachines,
which includes the customizers. However, the "setup" used in the "release"
method of JcloudsLocation is taken from the JcloudsLocation itself, which
is the provisioning (i.e. parent) location, which doesn't have those
customizers in it.
It would actually make more sense to get the location customizers from
the machine location, as a given parent could have mutiple machines
with different customizers.
The changes here include an addition to the
`JcloudsCustomizerInstantiationYamlDslTest#testCustomizers`
to demonstrate the problem, and a fix that adds the config to
the machine location.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/geomacy/brooklyn-server jclc-release
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/530.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 #530
----
commit 6fef9147b10cdf24b2b18d9c49ee8076d60c94bd
Author: Geoff Macartney <ge...@cloudsoftcorp.com>
Date: 2017-01-19T17:10:45Z
preRelease and postRelease of JcloudsLocationCustomizers are not called on stop
commit 588ef4b97789770cb2fa5c48b267b35cedcc8ae3
Author: Geoff Macartney <ge...@cloudsoftcorp.com>
Date: 2017-01-19T17:28:31Z
Fix broken test for preRelease/postRelease
commit 645d59ba4ab3fa85cbf07449dbfaf4a1d0a396c9
Author: Geoff Macartney <ge...@cloudsoftcorp.com>
Date: 2017-01-25T14:55:54Z
Add the config for JCLOUDS_LOCATION_CUSTOMIZERS at construction time
so migrated settings are all at one place.
----
---
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] brooklyn-server issue #530: JcloudsLocation is not releasing its customizers...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/530
Thanks for updates, merging.
---
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] brooklyn-server issue #530: JcloudsLocation is not releasing its customizers...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/530
test failure unrelated?
---
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] brooklyn-server pull request #530: JcloudsLocation is not releasing its cust...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/530#discussion_r97330165
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -2219,7 +2227,7 @@ public void release(MachineLocation rawMachine) {
Exception tothrow = null;
- ConfigBag setup = config().getBag();
+ ConfigBag setup = ((LocationInternal)machine).config().getBag();
--- End diff --
IIRC it is needed to be able to do the `getBag()`
---
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] brooklyn-server pull request #530: JcloudsLocation is not releasing its cust...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy closed the pull request at:
https://github.com/apache/brooklyn-server/pull/530
---
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] brooklyn-server issue #530: JcloudsLocation is not releasing its customizers...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/530
Addressed review [comment](https://github.com/apache/brooklyn-server/pull/530#discussion_r97329773) "Can you instead add the config for JCLOUDS_LOCATION_CUSTOMIZERS..."
---
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] brooklyn-server pull request #530: JcloudsLocation is not releasing its cust...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/530#discussion_r97329117
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -2219,7 +2227,7 @@ public void release(MachineLocation rawMachine) {
Exception tothrow = null;
- ConfigBag setup = config().getBag();
+ ConfigBag setup = ((LocationInternal)machine).config().getBag();
--- End diff --
Is the cast needed?
---
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.
---