Re: [jclouds/jclouds-labs] JCLOUDS-1362: Proper password generation with custom constraints for each cloud (#430)
LGTM -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/430#issuecomment-355903790
Re: [jclouds/jclouds] JCLOUDS-1362: Better password generation utility (#1165)
Nice change, LGTM. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1165#issuecomment-355903651
Re: [jclouds/jclouds-cli] WIP Upgrade Karaf (#38)
Closing this. We'll still need to update the API at some point, but for now we got what we needed by using a compatibility layer. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/38#issuecomment-329744977
Re: [jclouds/jclouds-cli] WIP Upgrade Karaf (#38)
Closed #38. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/38#event-1250824891
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
neykov approved this pull request. :+1: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1128#pullrequestreview-55171544
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
neykov commented on this pull request.
> for (String securityGroupName : templateOptions.getGroups()) {
-
checkNotNull(novaApi.getSecurityGroupApi(region).get().get(securityGroupName),
"security group %s doesn't exist", securityGroupName);
+checkState(securityGroupNames.contains(securityGroupName), "Cannot
find security group with name " + securityGroupName + ". \nSecurity groups
available are: \n" + Iterables.toString(securityGroupNames)); // {
}
Wouldn't give as nice error message as the above one - not clear which security
group is missing with your suggestion.
LGTM overall.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1128#discussion_r131720327
Re: [jclouds/jclouds-labs] JCLOUDS-1304: add azure customdata, JCLOUDS-1305: add support for ssh keys in keyvault (#398)
@VRanga000 what do you think about splitting the PR into `customData` support and another one for keyvault support? The two don't seem to depend on each other, and we can merge `customData` without waiting for the rest. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/398#issuecomment-320494861
Re: [jclouds/jclouds-karaf] Fix vagrant feature dependencies (#105)
Thanks @nacx, missed that. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/105#issuecomment-320172616
Re: [jclouds/jclouds-labs] Vagrant support for multiple providers (#403)
Merged in [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/83418324c0f5c74f4ebdc3cecf11d345877e96f9) and [2.0.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/dfe70318d2ed993ed2548005cf0f0db210bdfd50). -- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/403#issuecomment-319352068
Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)
Merged in [master](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=c9af1b248e3cfd0ce312823436e986726e826ccd). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/404#issuecomment-319347263
Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)
Tested azure with the max password length (50), merging. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/404#issuecomment-319327205
Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)
retest this please Dependent changes merged in jclouds, next build should be successful. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/404#issuecomment-319082215
Re: [jclouds/jclouds] JCLOUDS-1321: Use separate credential stores per context (#1119)
Merged to [master](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=2487b0c5132191de5e68413e33dfa551b5679ebb). -- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1119#issuecomment-318342375
Re: [jclouds/jclouds] Consume Unicode byte order mark in XML parser (#1124)
neykov approved this pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1124#pullrequestreview-52592661
Re: [jclouds/jclouds] Consume Unicode byte order mark in XML parser (#1124)
The change was surprising to me, I expected Java to handle the BOM. After digging deeper I found [[1]](https://stackoverflow.com/questions/4897876/reading-utf-8-bom-marker) which points to two JDK bugs [[2]](http://bugs.java.com/view_bug.do?bug_id=4508058) and [[3]](http://bugs.java.com/view_bug.do?bug_id=6378911). Turns out they fixed it at some point to consume the BOM but then reverted because it breaks backwards compatibility. Also of interest, the UTF character `0xFEFF` is serialized as `EF BB BF` in the UTF-8 byte sequence [[4]](http://www.unicode.org/faq/utf_bom.html#BOM) \[1\] https://stackoverflow.com/questions/4897876/reading-utf-8-bom-marker \[2\] http://bugs.java.com/view_bug.do?bug_id=4508058 \[3\] http://bugs.java.com/view_bug.do?bug_id=6378911 \[4\] http://www.unicode.org/faq/utf_bom.html#BOM -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1124#issuecomment-318299404
Re: [jclouds/jclouds] JCLOUDS-1321: Use separate credential stores per context (#1119)
+1. I'll merge this and publish the changes by the end of the week due to passive consensus. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1119#issuecomment-317720168
[jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)
Depends on https://github.com/jclouds/jclouds/pull/1123 As discussed in https://github.com/jclouds/jclouds-labs/pull/402 You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/404 -- Commit Summary -- * Re-use the just added Passwords from jclouds-core -- File Changes -- M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/strategy/CreateResourcesThenCreateNodes.java (2) D azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/Passwords.java (32) M oneandone/src/main/java/org/apache/jclouds/oneandone/rest/compute/OneandoneComputeServiceAdapter.java (2) D oneandone/src/main/java/org/apache/jclouds/oneandone/rest/util/Passwords.java (64) M profitbricks-rest/src/main/java/org/apache/jclouds/profitbricks/rest/compute/ProfitBricksComputeServiceAdapter.java (2) M profitbricks-rest/src/main/java/org/apache/jclouds/profitbricks/rest/domain/Volume.java (2) D profitbricks-rest/src/main/java/org/apache/jclouds/profitbricks/rest/util/Passwords.java (64) D profitbricks-rest/src/test/java/org/apache/jclouds/profitbricks/rest/util/PasswordsTest.java (53) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/404.patch https://github.com/jclouds/jclouds-labs/pull/404.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/404
[jclouds/jclouds] Move Passwords implementation to jclouds-core to be reused by providers (#1123)
As discussed in https://github.com/jclouds/jclouds-labs/pull/402 You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/1123 -- Commit Summary -- * Move Passwords implementation to jclouds-core to be reused by providers -- File Changes -- R core/src/main/java/org/jclouds/util/Passwords.java (2) R core/src/test/java/org/jclouds/util/PasswordsTest.java (8) M providers/profitbricks/src/main/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapter.java (2) M providers/profitbricks/src/main/java/org/jclouds/profitbricks/util/Preconditions.java (2) -- Patch Links -- https://github.com/jclouds/jclouds/pull/1123.patch https://github.com/jclouds/jclouds/pull/1123.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1123
Re: [jclouds/jclouds] [JCLOUDS-1318] fix based on nodeTerminatePredicate (#1117)
Added a jira as a reminder for the release notes - https://issues.apache.org/jira/browse/JCLOUDS-1323. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1117#issuecomment-316984706
Re: [jclouds/jclouds] [JCLOUDS-1318] fix based on nodeTerminatePredicate (#1117)
Heads up, the commit changes how security groups are set in `templateOptions`. Previously the human readable name was accepted. Now the UUID of the security group is required. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1117#issuecomment-316953433
Re: [jclouds/jclouds] Make sure Jetty doesn't terminate on ssh disconnect (#1122)
Closed #1122. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1122#event-1172143924
Re: [jclouds/jclouds] Make sure Jetty doesn't terminate on ssh disconnect (#1122)
Took some time to get to the bottom of it :). Merged to [master](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=0c054c18359fc2bd0f1de2344b6e021a38aed695) and [2.0.x](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=5b57b73cf744cd316c028edd2e5d765324ec6d6f) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1122#issuecomment-316709900
Re: [jclouds/jclouds-labs] Vagrant support for multiple providers (#403)
@neykov pushed 1 commit. db197f2 JCLOUDS-1256: Remove unused code -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds-labs/pull/403/files/cc130d12019c8c4adb9c4c978db109f7e95e4809..db197f226dfb4dfa883e000c717dd7ee0c9a841e
[jclouds/jclouds-labs] Vagrant support for multiple providers (#403)
Support machines where multiple providers are installed. Will pass the provider to use to vagrant, based on the selected image. For example Fedora installs libvirt by default with virtualbox as an optional dependency. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/403 -- Commit Summary -- * Support for multiple vagrant providers * Don't hardcode specific networks to skip -- File Changes -- M vagrant/README.md (25) M vagrant/pom.xml (2) M vagrant/src/main/java/org/jclouds/vagrant/api/VagrantApiFacade.java (2) M vagrant/src/main/java/org/jclouds/vagrant/compute/VagrantComputeServiceAdapter.java (11) M vagrant/src/main/java/org/jclouds/vagrant/internal/VagrantCliFacade.java (4) M vagrant/src/main/resources/Vagrantfile (28) M vagrant/src/test/resources/logback-test.xml (7) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/403.patch https://github.com/jclouds/jclouds-labs/pull/403.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/403
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
neykov commented on this pull request.
> + * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azurecompute.arm.util;
+
+import com.google.common.io.BaseEncoding;
+
+import java.util.Random;
+
+// Seems to be a common theme between providers, perhaps should be provided by
core (see other 'Passwords' classes)
+public class Passwords {
+1 to refactor and add tests. I can give it a try.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#discussion_r127899519
Re: [jclouds/jclouds-site] Remove unsupported sections (clojure, Ant, GAE) (#199)
Updated the site. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/199#issuecomment-315980868
Re: [jclouds/jclouds-site] Remove unsupported sections (clojure, Ant, GAE) (#199)
[Merged](https://git1-us-west.apache.org/repos/asf?p=jclouds-site.git;a=commit;h=94b98475f77b0ed6b9d9e3db2e05209b19c523f4) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/199#issuecomment-315778282
Re: [jclouds/jclouds-site] Remove unsupported sections (clojure, Ant, GAE) (#199)
Merged #199. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/199#event-1166533045
Re: [jclouds/jclouds] JCLOUDS-1321: Use separate credential stores per context (#1119)
* Jira (to be included in release notes): https://issues.apache.org/jira/browse/JCLOUDS-1321 * Heads up to the mailing lists: * [user@j.a.o](https://lists.apache.org/thread.html/c65711e3ca3b3ffe068fc6562f0db98336772984924f24857ee0bdcd@%3Cuser.jclouds.apache.org%3E) * [dev@j.a.o](https://lists.apache.org/thread.html/b3edeca6b742d122908c570dd779af4bb1c93c6405e0bf5b52c17267@%3Cdev.jclouds.apache.org%3E) * Documenting a work around in the [compute guide](https://github.com/jclouds/jclouds-site/pull/200) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1119#issuecomment-315765265
[jclouds/jclouds-site] Document workaround to revert to shared credential store (#200)
Depends on https://github.com/jclouds/jclouds/pull/1119 being released. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-site/pull/200 -- Commit Summary -- * Workaround to revert to shared credential store -- File Changes -- M start/compute.md (15) -- Patch Links -- https://github.com/jclouds/jclouds-site/pull/200.patch https://github.com/jclouds/jclouds-site/pull/200.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/200
Re: [jclouds/jclouds-site] Remove unsupported sections (clojure, Ant, GAE) (#199)
Done @nacx -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/199#issuecomment-315760120
[jclouds/jclouds-site] Remove unsupported sections (clojure & ant) (#199)
* clojure support removed from core * reference to non-existent `ApacheAntComputeGuide` You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-site/pull/199 -- Commit Summary -- * Remove Clojure references, support removed from core * Remove reference to the non-existent ApacheAntComputeGuide page -- File Changes -- M start/compute.md (72) -- Patch Links -- https://github.com/jclouds/jclouds-site/pull/199.patch https://github.com/jclouds/jclouds-site/pull/199.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/199
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
Closed #402. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/402#event-1166369934
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
Merged to [master](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=3641cdb44c192be381f82d8886f966c248474f4d) and [2.0.x](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=f27b5f944887bdefba78d7bbd339ea68db76725c) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/402#issuecomment-315747187
Re: [jclouds/jclouds] Use separate credential stores per context (#1119)
These are all great ideas @nacx. I'll do the necessary updates. I'll send an email to the mailing list so the change gets wider exposure. It's also a breaking change so should be merged only on master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1119#issuecomment-315331196
Re: [jclouds/jclouds] Use separate credential stores per context (#1119)
@nacx That's a great idea, thanks for the example. Don't disagree having the shared cache could be useful. It's the way it's used in [GetLoginForProviderFromPropertiesAndStoreCredentialsOrReturnNull](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/config/GetLoginForProviderFromPropertiesAndStoreCredentialsOrReturnNull.java) that I don't like. And that's used in all providers. Having the configuration of one compute service affect all remaining compute services even when they have different configuration is surprising to say the least :) I still think that it's the wrong thing to do. Perhaps it should be the other way around - users which really need the cross-service cache should inject it themselves. Can you give some good examples of using the cache? From looking at the code it seems that its not even needed - no heavy computation is done to extract the username/password in the default case. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1119#issuecomment-315095395
Re: [jclouds/jclouds] Use separate credential stores per context (#1119)
neykov commented on this pull request.
> @@ -59,13 +59,8 @@ protected void configure() {
}).to(CredentialsToJsonByteSource.class);
bind(new TypeLiteral>() {
}).to(CredentialsFromJsonByteSource.class);
- if (backing != null) {
- bind(new TypeLiteral>() {
- }).toInstance(backing);
- } else {
- bind(new TypeLiteral>() {
- }).toInstance(BACKING);
Good spot, removed.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1119#discussion_r127129277
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
neykov commented on this pull request.
> @@ -130,6 +136,20 @@ protected CreateResourcesThenCreateNodes(
return super.execute(group, count, template, goodNodes, badNodes,
customizationResponses);
}
+ // Azure requires that we pass it the VM password. Need to generate one if
not overriden by the user.
+ private void generatePasswordIfNoneProvided(Template template) {
+ TemplateOptions options = template.getOptions();
+ if (options.getLoginPassword() == null) {
+ Optional passwordOptional =
template.getImage().getDefaultCredentials().getOptionalPassword();
+ if (passwordOptional.isPresent()) {
Didn't like it initially because it will generate a password regardless of
whether used. But +1 for cleaner code. Updated.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/402#discussion_r126933224
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
Thanks @nacx, merging after build completes. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/402#issuecomment-314746485
Re: [jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
neykov commented on this pull request. > + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.azurecompute.arm.util; + +import com.google.common.io.BaseEncoding; + +import java.util.Random; + +// Seems to be a common theme between providers, perhaps should be provided by core (see other 'Passwords' classes) Will leave that for another PR. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/402#discussion_r126933309
[jclouds/jclouds-labs] Generate Azure VM password on the fly (#402)
Used to be a fixed password which anyone can use to login to the newly provisioned machines. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/402 -- Commit Summary -- * Generate Azure VM password on the fly -- File Changes -- M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeProviderMetadata.java (5) M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/AzureComputeServiceAdapter.java (12) M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/strategy/CreateResourcesThenCreateNodes.java (21) A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/Passwords.java (32) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/402.patch https://github.com/jclouds/jclouds-labs/pull/402.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/402
[jclouds/jclouds] Use separate credential stores per context (#1119)
With a shared credential store the configuration of one compute service leaks in all others, causing the wrong credentials to be used when not overridden. The particular problem this fixes - [Azure sets default username/password for images](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeProviderMetadata.java#L98) which leaks to other providers (through [GetLoginForProviderFromPropertiesAndStoreCredentialsOrReturnNull](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/config/GetLoginForProviderFromPropertiesAndStoreCredentialsOrReturnNull.java#L51)), causing the wrong username/password to be used. Even if the credentials are scoped to a specific provider in the map, it still will cause cross-talk between differently configured compute services. The cache should only apply to the current compute service. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/1119 -- Commit Summary -- * Use separate credential stores per context -- File Changes -- M core/src/main/java/org/jclouds/rest/config/CredentialStoreModule.java (11) M core/src/test/java/org/jclouds/rest/CredentialStoreModuleTest.java (42) -- Patch Links -- https://github.com/jclouds/jclouds/pull/1119.patch https://github.com/jclouds/jclouds/pull/1119.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1119
Re: [jclouds/jclouds-labs] Fix SGE.removeSecurityGroup for when the SG doesn't exist (#399)
Closed #399. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/399#event-1154029335
Re: [jclouds/jclouds-labs] Fix SGE.removeSecurityGroup for when the SG doesn't exist (#399)
Thanks @nacx, merging. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/399#issuecomment-313610076
[jclouds/jclouds-labs] Fix SGE.removeSecurityGroup for when the SG doesn't exist (#399)
@nacx reported the problem in the `Re: [DISCUSS] Release Apache jclouds 2.0.2 RC1` email thread. Failure: ``` testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeSecurityGroupExtensionLiveTest) Time elapsed: 2.928 sec <<< FAILURE! java.lang.NullPointerException: uri cannot be null at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:229) at org.jclouds.azurecompute.arm.compute.config.AzureComputeServiceContextModule$ActionDonePredicate.apply(AzureComputeServiceContextModule.java:230) at org.jclouds.azurecompute.arm.compute.config.AzureComputeServiceContextModule$ActionDonePredicate.apply(AzureComputeServiceContextModule.java:219) at org.jclouds.util.Predicates2$RetryablePredicate.apply(Predicates2.java:117) at org.jclouds.azurecompute.arm.compute.extensions.AzureComputeSecurityGroupExtension.removeSecurityGroup(AzureComputeSecurityGroupExtension.java:193) at org.jclouds.compute.extensions.internal.BaseSecurityGroupExtensionLiveTest.testSecurityGroupCacheInvalidatedWhenDeletedExternally(BaseSecurityGroupExtensionLiveTest.java:417) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85) at org.testng.internal.Invoker.invokeMethod(Invoker.java:696) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:882) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1189) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108) at org.testng.TestRunner.privateRun(TestRunner.java:767) at org.testng.TestRunner.run(TestRunner.java:617) at org.testng.SuiteRunner.runTest(SuiteRunner.java:348) at org.testng.SuiteRunner.access$000(SuiteRunner.java:38) at org.testng.SuiteRunner$SuiteWorker.run(SuiteRunner.java:382) at org.testng.internal.thread.ThreadUtil$2.call(ThreadUtil.java:64) at java.util.concurrent.FutureTask.run(FutureTask.java:262) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745) ``` You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/399 -- Commit Summary -- * Fix SGE.removeSecurityGroup for when the SG doesn't exist -- File Changes -- M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/extensions/AzureComputeSecurityGroupExtension.java (10) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/399.patch https://github.com/jclouds/jclouds-labs/pull/399.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/399
Re: [jclouds/jclouds] JCLOUDS-1307: Invalidate security groups even if already externally deleted. (#1111)
Closed #. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/#event-1140247250
Re: [jclouds/jclouds] JCLOUDS-1307: aws-ec2: Invalidate SG on removal even if already externally deleted (#1113)
Thanks @nacx, merged in [master](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=65ba26eca7d63b6b7cef70386f00425438318825) and [2.0.x](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=502d749680e98fbe0d32c14faea8017da40f3030) -- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1113#issuecomment-311320190
Re: [jclouds/jclouds] JCLOUDS-1307: Invalidate security groups even if already externally deleted. (#1111)
Thanks @nacx, merged in [master](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=624e4cb6af605be6bbb11637c4b60324dd6f5a11) and [2.0.x](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=a2863b2a3c3d57e9c9909c5b9f4bba8ac071b3dc) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/#issuecomment-311320236
Re: [jclouds/jclouds] JCLOUDS-1307: Invalidate security groups even if already externally deleted. (#1111)
I've split the `aws-ec2` changes into a sperate PR https://github.com/jclouds/jclouds/pull/1113 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/#issuecomment-308454066
[jclouds/jclouds] JCLOUDS-1307: aws-ec2: Invalidate SG on removal even if already externally deleted (#1113)
Splitting the amazon changes from https://github.com/jclouds/jclouds/pull/ into this PR to make it easier to merge. I've already tested these changes. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/1113 -- Commit Summary -- * JCLOUDS-1307: Invalidate SG on removal even if already externally deleted -- File Changes -- M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/extensions/AWSEC2SecurityGroupExtension.java (12) -- Patch Links -- https://github.com/jclouds/jclouds/pull/1113.patch https://github.com/jclouds/jclouds/pull/1113.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1113
[jclouds/jclouds] JCLOUDS-1307: Invalidate security groups even if already externally deleted. (#1111)
https://issues.apache.org/jira/browse/JCLOUDS-1307 You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/ -- Commit Summary -- * [JCLOUDS-1306] Fix SG cache invalidation when deleting * JCLOUDS-1307: Invalidate SG on removal even if already externally deleted -- File Changes -- M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtension.java (20) M apis/ec2/src/main/java/org/jclouds/ec2/compute/extensions/EC2SecurityGroupExtension.java (11) M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java (18) M compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java (66) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/extensions/AWSEC2SecurityGroupExtension.java (12) -- Patch Links -- https://github.com/jclouds/jclouds/pull/.patch https://github.com/jclouds/jclouds/pull/.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/
Re: [jclouds/jclouds] [JCLOUDS-1306] Fix SG cache invalidation when deleting (#1110)
Yes, I think it would be useful to backport. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1110#issuecomment-307339354
Re: [jclouds/jclouds] [JCLOUDS-1306] Fix SG cache invalidation when deleting (#1110)
neykov commented on this pull request.
> @@ -191,14 +191,20 @@ public boolean removeSecurityGroup(String id) {
return false;
}
- if (sgApi.get().get(groupId) == null) {
- return false;
+ // Would be nice to delete the group and invalidate the cache atomically
- i.e. use a mutex.
+ // Will make sure that a create operation in parallel won't see
inconsistent state.
+
+ boolean deleted = sgApi.get().delete(groupId);
+
+ for (SecurityGroupInRegion cachedSg : groupCreator.asMap().values()) {
Yes - the security group could've been deleted externally and the local cache
still holds a reference to the group. This tries to avoid the situation
described in https://issues.apache.org/jira/browse/JCLOUDS-1307.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1110#discussion_r121081636
Re: [jclouds/jclouds] [JCLOUDS-1306] Fix SG cache invalidation when deleting (#1110)
neykov commented on this pull request.
> @@ -191,14 +191,20 @@ public boolean removeSecurityGroup(String id) {
return false;
}
- if (sgApi.get().get(groupId) == null) {
- return false;
+ // Would be nice to delete the group and invalidate the cache atomically
- i.e. use a mutex.
+ // Will make sure that a create operation in parallel won't see
inconsistent state.
+
+ boolean deleted = sgApi.get().delete(groupId);
The return value is false. This code path is covered in the test
`testSecurityGroupCacheInvalidatedWhenDeletedExternally`
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1110#discussion_r121081494
Re: [jclouds/jclouds] [JCLOUDS-1306] Fix SG cache invalidation when deleting (#1110)
The test `deleteSecurityGroupFromAnotherView ` will fail on aws-ec2 and probably other implementations, will address those separately. See https://issues.apache.org/jira/browse/JCLOUDS-1307. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1110#issuecomment-307337452
[jclouds/jclouds] [JCLOUDS-1306] Fix SG cache invalidation when deleting (#1110)
When deleting a security group on openstack-nova the cache is not invalidated correctly, so the next create operation will fetch the group from local cache instead of re-creating it in the cloud. Subsequent operations will fail due to the missing group and stale ID. https://issues.apache.org/jira/browse/JCLOUDS-1306 You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/1110 -- Commit Summary -- * [Openstack] Fix SG cache invalidation when deleting -- File Changes -- M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java (18) M apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtensionLiveTest.java (63) -- Patch Links -- https://github.com/jclouds/jclouds/pull/1110.patch https://github.com/jclouds/jclouds/pull/1110.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1110
Re: [jclouds/jclouds-cli] Reference super features in assembly module (#37)
Closed #37. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37#event-1103788916
Re: [jclouds/jclouds] JCLOUDS-1295: Support deprecated date formats in the Expires header (#1107)
Merged into [master](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=db2f86bcecfe0d68183a166ad07a7f996d332539) and [2.0.x](https://git1-us-west.apache.org/repos/asf/jclouds/?p=jclouds.git;a=commit;h=db4f191b5f8a4d2e77e999f3f6824b2e531bb5e8). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1107#issuecomment-304669359
Re: [jclouds/jclouds] JCLOUDS-1295: Support deprecated date formats in the Expires header (#1107)
Agree, changes look good. Merging. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1107#issuecomment-304667087
Re: [jclouds/jclouds] JCLOUDS-1295: Support deprecated date formats in the Expires header (#1107)
@nacx there's no change, the fallback fails to parse the header value which is `-1`. I captured the traffic with wireshark and indeed the value is `-1`, contrary to what the headers log show. Here's the wireshark capture: ``` HTTP/1.1 200 OK Cache-Control: no-cache Pragma: no-cache Content-Type: application/json; charset=utf-8 Content-Encoding: gzip Expires: -1 Vary: Accept-Encoding x-ms-ratelimit-remaining-subscription-reads: 14998 x-ms-request-id: f6cce7d2-63df-4d29-b03c-c4858e10a8ee x-ms-correlation-request-id: f6cce7d2-63df-4d29-b03c-c4858e10a8ee x-ms-routing-request-id: WESTEUROPE:20170529T124513Z:f6cce7d2-63df-4d29-b03c-c4858e10a8ee Strict-Transport-Security: max-age=31536000; includeSubDomains Date: Mon, 29 May 2017 12:45:13 GMT Content-Length: 1850 ``` And the headers log: ``` 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << HTTP/1.1 200 OK 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << Date: Mon, 29 May 2017 12:45:13 GMT 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << OkHttp-Received-Millis: 1496061913957 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << OkHttp-Selected-Protocol: http/1.1 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << OkHttp-Sent-Millis: 1496061913849 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << Pragma: no-cache 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << Strict-Transport-Security: max-age=31536000; includeSubDomains 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << Vary: Accept-Encoding 2017-05-29 15:45:13,958 DEBUG 107 j.headers [ager-DOccEjYP-24] << x-ms-correlation-request-id: f6cce7d2-63df-4d29-b03c-c4858e10a8ee 2017-05-29 15:45:13,959 DEBUG 107 j.headers [ager-DOccEjYP-24] << x-ms-ratelimit-remaining-subscription-reads: 14998 2017-05-29 15:45:13,959 DEBUG 107 j.headers [ager-DOccEjYP-24] << x-ms-request-id: f6cce7d2-63df-4d29-b03c-c4858e10a8ee 2017-05-29 15:45:13,959 DEBUG 107 j.headers [ager-DOccEjYP-24] << x-ms-routing-request-id: WESTEUROPE:20170529T124513Z:f6cce7d2-63df-4d29-b03c-c4858e10a8ee 2017-05-29 15:45:13,959 DEBUG 107 j.headers [ager-DOccEjYP-24] << Cache-Control: no-cache 2017-05-29 15:45:13,959 DEBUG 107 j.headers [ager-DOccEjYP-24] << Content-Type: application/json; charset=utf-8 2017-05-29 15:45:13,959 DEBUG 107 j.headers [ager-DOccEjYP-24] << Expires: Thu Jan 01 02:00:00 EET 1970 ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1107#issuecomment-304653809
Re: [jclouds/jclouds-labs] Adding support for provisioning of Windows VMs with enabled WinRM (#393)
Added to [2.0.x](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=c4a91d2d909a4edf52adf1367b5353d81cde681b). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/393#issuecomment-304024872
Re: [jclouds/jclouds-labs] Adding support for provisioning of Windows VMs with enabled WinRM (#393)
Merged to [master](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=6e0cbc915cfb1e3f0cb9cdafd3b56b948e0e80a8). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/393#issuecomment-304011560
Re: [jclouds/jclouds-labs] Adding support for provisioning of Windows VMs with enabled WinRM (#393)
Thanks @ygy and all reviewers. Changes look good. Merging. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/393#issuecomment-304010462
Re: [jclouds/jclouds-karaf] Fix Chef commands (#104)
LGTM -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/104#issuecomment-301000767
Re: [jclouds/jclouds-karaf] JCLOUDS-944: JCLOUDS-1272: Update B2 and GCS deps (#101)
neykov requested changes on this pull request.
Need to update tests as well, see build failures and
https://github.com/jclouds/jclouds-karaf/blob/master/itests/src/test/java/org/jclouds/karaf/itests/FeatureLabsInstallationTest.java
> @@ -549,7 +551,7 @@ limitations under the License.
jclouds-blobstore
jclouds-api-oauth
mvn:org.apache.jclouds.common/googlecloud/${jclouds.version}
-
mvn:org.apache.jclouds.labs/google-cloud-storage/${jclouds.version}
+
mvn:org.apache.jclouds.feature/google-cloud-storage/${jclouds.version}
Should be `provider` instead of `feature`.
> @@ -18,11 +18,6 @@ limitations under the License.
http://karaf.apache.org/xmlns/features/v1.0.0;>
mvn:org.apache.jclouds.karaf/jclouds-karaf/${jclouds.version}/xml/features
-
-jclouds-b2
-jclouds-google-cloud-storage
-
-
I suggest we keep this empty. This will let projects depending on it to:
* not break
* not care when modules are added/removed
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/101#pullrequestreview-36949031
Re: [jclouds/jclouds] JCLOUDS-1269; JCLOUDS-1120: ec2 subnet/securityGroups in non-default vpc (#1091)
neykov commented on this pull request.
> public AWSSubnetApiLiveTest() {
provider = "aws-ec2";
}
+
+ private AWSEC2Api api;
+ private Subnet subnet;
+
+ @Override
+ @BeforeClass(groups = { "integration", "live" })
+ public void setupContext() {
+ super.setupContext();
+ api = view.unwrapApi(AWSEC2Api.class);
+ }
+
+ @Test
+ public void testCreateSubnetInRegion() {
+// subnet = subnetApi().createSubnetInRegion();
+// assertNotNull(subnet);
Why is the test code commented out?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1091#pullrequestreview-35386111
Re: [jclouds/jclouds] JCLOUDS-1269; JCLOUDS-1120: ec2 subnet/securityGroups in non-default vpc (#1091)
neykov commented on this pull request.
> @@ -102,7 +104,17 @@ private String createSecurityGroupInRegion(String
> region, String name, String vp
.build());
}
-String myOwnerId =
Iterables.get(securityApi.describeSecurityGroupsInRegion(region, name),
0).getOwnerId();
+// Use filter (as per `SecurityGroupPresent`, in
securityGroupEventualConsistencyDelay)
+Set securityGroups =
securityApi.describeSecurityGroupsInRegionWithFilter(region,
+ ImmutableMultimap.of("group-name", name));
You could use the `id` of the security group with a `group-id` filter here.
Same for `securityGroupEventualConsistencyDelay`, by passing `new
RegionAndName(region, id)`. The ID is unique for the region, while the name can
be the same between VPCs - will guarantee that you don't get multiple results
here.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1091#pullrequestreview-35385538
[jclouds/jclouds-cli] WIP Upgrade Karaf (#38)
That's my current progress with upgrading to latest Karaf. A lot of things to take care of, but wanted to get this visible. Not even worth a review. Most interesting are the changes in `Main.java`. Currently uses Karaf `4.0.1` - later patch versions make API changes which need further investigation. I got it to the point to compile and start Karaf, but it fails loading the features, again not recognizing the `1.4.0` namespace version. It's not even [documented](https://karaf.apache.org/manual/latest/#_namespaces) so it's not clear which version supports it. This is an indication we need to downgrade the `jclouds-karaf` namespace to something more widely supported. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-cli/pull/38 -- Commit Summary -- * WIP Upgrade karaf dependency -- File Changes -- M assembly/pom.xml (280) M assembly/src/main/assembly/unix.xml (6) M assembly/src/main/assembly/win.xml (6) D assembly/src/main/filtered-resources/etc/custom.properties (33) D assembly/src/main/filtered-resources/etc/log4j.properties (27) D assembly/src/main/filtered-resources/etc/org.apache.jclouds.shell.cfg (53) D assembly/src/main/filtered-resources/etc/org.apache.karaf.features.cfg (19) D assembly/src/main/filtered-resources/etc/org.apache.karaf.features.repos.cfg (19) D assembly/src/main/filtered-resources/etc/shell.init.script (21) D assembly/src/main/filtered-resources/etc/startup.properties (50) D assembly/src/main/filtered-resources/unix/bin/chef (91) D assembly/src/main/filtered-resources/unix/bin/jclouds (55) D assembly/src/main/filtered-resources/unix/bin/shell (332) D assembly/src/main/filtered-resources/win/bin/chef.bat (134) D assembly/src/main/filtered-resources/win/bin/jclouds.bat (102) D assembly/src/main/filtered-resources/win/bin/shell.bat (133) M project/pom.xml (30) M runner/pom.xml (7) M runner/src/main/java/org/jclouds/cli/runner/Main.java (230) -- Patch Links -- https://github.com/jclouds/jclouds-cli/pull/38.patch https://github.com/jclouds/jclouds-cli/pull/38.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/38
Re: [jclouds/jclouds-cli] Reference super features in assembly module (#37)
@nacx you beat me to it. I'll give updating the commands a go. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37#issuecomment-289388616
Re: [jclouds/jclouds-labs] Improve vagrant docs (#373)
Addressed comments. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/373#issuecomment-287872812
[jclouds/jclouds] Create and attach tests sources as artifacts (#1079)
Will create `-test-sources.jar` corresponding to the `-tests.jar` binaries. Analogously to the sources generated for the release jar. Very useful when writing tests in the IDE, needing to consult other test files we are depending on (say `BaseContextLiveTest`). You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/1079 -- Commit Summary -- * Create and attach tests sources as artifacts -- File Changes -- M project/pom.xml (1) -- Patch Links -- https://github.com/jclouds/jclouds/pull/1079.patch https://github.com/jclouds/jclouds/pull/1079.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1079
Re: [jclouds/jclouds-cli] Reference super features in assembly module (#37)
Tested successfully on top of both https://github.com/jclouds/jclouds-karaf/pull/100 and https://github.com/jclouds/jclouds-karaf/pull/99. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37#issuecomment-287800676
Re: [jclouds/jclouds-cli] Reference super features in assembly module (#37)
Nice find @demobox, @nacx - added it back in https://github.com/jclouds/jclouds-karaf/pull/100. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37#issuecomment-287789846
[jclouds/jclouds-karaf] Attach config files referenced in the features as artifacts (#100)
Adds back (a bit simplified) the maven config which creates artifacts out of the config files used in the feature.xml. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-karaf/pull/100 -- Commit Summary -- * Attach config files referenced in the features as artifacts -- File Changes -- M feature/pom.xml (28) -- Patch Links -- https://github.com/jclouds/jclouds-karaf/pull/100.patch https://github.com/jclouds/jclouds-karaf/pull/100.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/100
Re: [jclouds/jclouds-karaf] Add back the cloudsigma-api feature for backwards compatibility (#99)
Good spot @demobox. Updated. Now testing with jclouds-cli, though "it shouldn't matter (tm)" :). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/99#issuecomment-287786218
[jclouds/jclouds-labs] Improve vagrant docs (#373)
Also makes the windows image optional for tests - skips the tests if not found. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/373 -- Commit Summary -- * Make the windows image optional * Improve vagrant docs -- File Changes -- M vagrant/README.md (17) M vagrant/pom.xml (2) M vagrant/src/main/java/org/jclouds/vagrant/functions/BoxToImage.java (4) M vagrant/src/main/java/org/jclouds/vagrant/internal/VagrantNodeRegistry.java (69) M vagrant/src/test/java/org/jclouds/vagrant/compute/WindowsLiveTest.java (15) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/373.patch https://github.com/jclouds/jclouds-labs/pull/373.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/373
Re: [jclouds/jclouds-cli] Reference super features in assembly module (#37)
Correction on my comment above @andreaturli. This does require https://github.com/jclouds/jclouds-karaf/pull/98 to build successfully. And https://github.com/jclouds/jclouds-karaf/pull/99 is not needed, it's just good to have. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37#issuecomment-287624286
Re: [jclouds/jclouds-karaf] Add back the cloudsigma-api feature for backwards compatibility (#99)
@andreaturli Either this PR or https://github.com/jclouds/jclouds-cli/pull/37 will fix the build. Thought it's useful to add anyway for backwards compatibility, in case other projects depend on it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/99#issuecomment-287624207
Re: [jclouds/jclouds-karaf] Add back the cloudsigma-api feature for backwards compatibility (#99)
neykov commented on this pull request. > @@ -516,32 +521,27 @@ limitations under the License. -jclouds-compute Goot point @demobox. I reverted the changes for the sake of consistency. Still think it's the right thing to do though. Perhaps something to take care of in another PR. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/99#discussion_r106809148
Re: [jclouds/jclouds-cli] Reference super features in assembly module (#37)
neykov commented on this pull request. > config -jclouds -jclouds-commands -jclouds-compute -jclouds-guice `jclouds-guice`, `jclouds` is included by `jclouds-compute` and it is included by all the compute features. `jclouds-commands` is kept further below. It's like maven transitive features, don't need to mention them if we don't directly depend on them. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37#discussion_r106787537
Re: [jclouds/jclouds-cli] Reference super features in assembly module (#37)
@andreaturli This doesn't need `jclouds-karaf#98`. It's just for backwards compatibility. @demobox I've tried the distribution with `bin/jclouds` - it's working as expected. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37#issuecomment-287564428
[jclouds/jclouds-karaf] Add back the cloudsigma-api feature for backwards compatibility (#99)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-karaf/pull/99 -- Commit Summary -- * Add back the cloudsigma-api feature for backwards compatibility -- File Changes -- M feature/src/main/feature/feature.xml (20) -- Patch Links -- https://github.com/jclouds/jclouds-karaf/pull/99.patch https://github.com/jclouds/jclouds-karaf/pull/99.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/99
[jclouds/jclouds-cli] Reference super features in assembly module (#37)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-cli/pull/37 -- Commit Summary -- * Reference super features in assembly module -- File Changes -- M assembly/pom.xml (60) -- Patch Links -- https://github.com/jclouds/jclouds-cli/pull/37.patch https://github.com/jclouds/jclouds-cli/pull/37.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-cli/pull/37
[jclouds/jclouds-karaf] Group labs features in super features (#98)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-karaf/pull/98 -- Commit Summary -- * Group labs features in super features -- File Changes -- M feature-labs/src/main/feature/feature.xml (16) -- Patch Links -- https://github.com/jclouds/jclouds-karaf/pull/98.patch https://github.com/jclouds/jclouds-karaf/pull/98.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/98
Re: [jclouds/jclouds-karaf] Improvements to feature (#92)
LGTM. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/92#issuecomment-287320844
Re: [jclouds/jclouds-karaf] Improvements to feature (#92)
neykov commented on this pull request.
> + featuresService.installFeature("jclouds-rackspace-cloudfiles-us");
+ }
+
+ @Test
+ public void testServerloveZ1ManFeature() throws Exception {
+ featuresService.installFeature("jclouds-serverlove-z1-man");
+ }
+
+ @Test
+ public void testSkalicloudSdgMyFeature() throws Exception {
+ featuresService.installFeature("jclouds-skalicloud-sdg-my");
+ }
+
+ @Test
+ public void testSoftlayerFeature() throws Exception {
+ featuresService.installFeature("jclouds-softlayer");
It doesn't use a container - just does the bundle wiring step to make sure its
dependencies can be met. Which is essentially what we are doing here. These
tests would still be needed for more complex integration testing.
This is just FYI, no action needed. It's already a huge improvement to what was
there before.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/92#discussion_r104717822
Re: [jclouds/jclouds-karaf] Improvements to feature (#92)
neykov approved this pull request.
All good - some minor cleanups noted.
> -
+
+jclouds-compute
+jclouds-api-sts
+
mvn:org.apache.jclouds.api/cloudwatch/${jclouds.version}
+
+
+
+jclouds-compute
+jclouds-driver-bouncycastle
+jclouds-driver-okhttp
+jclouds-api-oauth
+mvn:org.apache.jclouds.api/docker/${jclouds.version}
+
+
+
Leftover `resolver` attribute here and elsewhere in the file.
> +jclouds-cloudsigma2-hnl
+jclouds-cloudsigma2-lvs
+jclouds-cloudsigma2-sjc
+jclouds-cloudsigma2-wdc
+jclouds-cloudsigma2-zrh
+jclouds-google-cloud-storage
+jclouds-packet
+jclouds-vagrant
+
+target/features-repo
+
+
+
+
+
+
That's probably a historic baggage. All that's needed in a feature pom is:
```
...
.
feature
org.apache.karaf.tooling
karaf-maven-plugin
${karaf.plugin.version}
true
```
To verify it's still working after the change make sure you still have the
following two in your logs:
```
[INFO] Installing .../jclouds-karaf/feature-labs/pom.xml to
/Users/svet/.m2/repository/org/apache/jclouds/karaf/jclouds-karaf-labs/2.1.0-SNAPSHOT/jclouds-karaf-labs-2.1.0-SNAPSHOT.pom
[INFO] Installing .../jclouds-karaf/feature-labs/target/feature-labs.xml to
/Users/svet/.m2/repository/org/apache/jclouds/karaf/jclouds-karaf-labs/2.1.0-SNAPSHOT/jclouds-karaf-labs-2.1.0-SNAPSHOT-features.xml
```
Same for the non-labs feature pom.
> + dependency='true'>mvn:org.apache.jclouds.common/googlecloud/${jclouds.version}
+
mvn:org.apache.jclouds.labs/google-cloud-storage/${jclouds.version}
+
+
+
+jclouds-compute
+mvn:org.apache.jclouds.labs/packet/${jclouds.version}
+
+
+
+jclouds-compute
+mvn:name.neykov/vagrant-java-bindings/${vagrant-java-bindings.version}
+mvn:org.apache.jclouds.labs/vagrant/${jclouds.version}
+
+
+
You can remove these now that they are in feature-labs.
> + featuresService.installFeature("jclouds-rackspace-cloudfiles-us");
+ }
+
+ @Test
+ public void testServerloveZ1ManFeature() throws Exception {
+ featuresService.installFeature("jclouds-serverlove-z1-man");
+ }
+
+ @Test
+ public void testSkalicloudSdgMyFeature() throws Exception {
+ featuresService.installFeature("jclouds-skalicloud-sdg-my");
+ }
+
+ @Test
+ public void testSoftlayerFeature() throws Exception {
+ featuresService.installFeature("jclouds-softlayer");
For future updates - I believe these "install" tests can be done more
efficiently with the `verify` goal of the `maven-karaf-plugin`.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/92#pullrequestreview-25492747
Re: [jclouds/jclouds-labs] Fix NPE in building node metadata (#367)
neykov requested changes on this pull request.
> @@ -179,8 +180,15 @@ public String apply(Status input) {
Credentials credentials = credentialStore.get("node#" +
virtualMachine.name());
builder.credentials(LoginCredentials.fromCredentials(credentials));
-
builder.publicAddresses(getPublicIpAddresses(virtualMachine.properties().networkProfile().networkInterfaces()));
-
builder.privateAddresses(getPrivateIpAddresses(virtualMachine.properties().networkProfile().networkInterfaces()));
+ Iterable publicIpAddresses =
getPublicIpAddresses(virtualMachine.properties().networkProfile().networkInterfaces());
+ Iterable privateIpAddresses =
getPrivateIpAddresses(virtualMachine.properties().networkProfile().networkInterfaces());
+
+ if (!Iterables.all(publicIpAddresses, Predicates.isNull())) {
Better fix the implementation of `getPublicIpAddresses` and
`getPrivateIpAddresses` to skip over null values. This will reject the whole
collection and there could be valid values in it.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/367#pullrequestreview-24781128
Re: [jclouds/jclouds-labs] Fix NPE in building node metadata (#367)
For completeness here's the exception this is trying to fix: ``` Caused by: java.lang.NullPointerException at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:210) at com.google.common.collect.SingletonImmutableSet.(SingletonImmutableSet.java:47) at com.google.common.collect.ImmutableSet.of(ImmutableSet.java:94) at com.google.common.collect.ImmutableSet.construct(ImmutableSet.java:185) at com.google.common.collect.ImmutableSet.copyOf(ImmutableSet.java:375) at com.google.common.collect.ImmutableSet.copyOf(ImmutableSet.java:300) at org.jclouds.compute.domain.NodeMetadataBuilder.publicAddresses(NodeMetadataBuilder.java:73) at org.jclouds.azurecompute.arm.compute.functions.VirtualMachineToNodeMetadata.apply(VirtualMachineToNodeMetadata.java:180) at org.jclouds.azurecompute.arm.compute.functions.VirtualMachineToNodeMetadata.apply(VirtualMachineToNodeMetadata.java:78) at com.google.common.base.Functions$FunctionComposition.apply(Functions.java:216) at com.google.common.collect.Iterators$8.transform(Iterators.java:799) at com.google.common.collect.TransformedIterator.next(TransformedIterator.java:48) at com.google.common.collect.Iterators$7.computeNext(Iterators.java:651) at com.google.common.collect.AbstractIterator.tryToComputeNext(AbstractIterator.java:143) at com.google.common.collect.AbstractIterator.hasNext(AbstractIterator.java:138) at com.google.common.collect.Iterators.indexOf(Iterators.java:776) at com.google.common.collect.Iterators.any(Iterators.java:684) at com.google.common.collect.Iterables.any(Iterables.java:623) at org.jclouds.compute.strategy.impl.CreateNodesWithGroupEncodedIntoNameThenAddToSet.getNextNames(CreateNodesWithGroupEncodedIntoNameThenAddToSet.java:199) at org.jclouds.compute.strategy.impl.CreateNodesWithGroupEncodedIntoNameThenAddToSet.execute(CreateNodesWithGroupEncodedIntoNameThenAddToSet.java:123) at org.jclouds.azurecompute.arm.compute.strategy.CreateResourceGroupThenCreateNodes.execute(CreateResourceGroupThenCreateNodes.java:133) at org.jclouds.compute.internal.BaseComputeService.createNodesInGroup(BaseComputeService.java:217) ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/367#issuecomment-283707609
Re: [jclouds/jclouds-labs] Improve image lookup when getting node info (#359)
LGTM. Could still use the image cache and fall back to `getImage` only if empty. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/359#issuecomment-282737375
[jclouds/jclouds-labs] vagrant: Tests can be executed concurrently - fix test assumptions (#364)
Tests from the same class are executed concurrently so don't share state in the class. Additionally use temporary files/folders to allow for parallel builds. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/364 -- Commit Summary -- * Tests can be executed concurrently - fix test assumptions -- File Changes -- M vagrant/src/test/java/org/jclouds/vagrant/internal/BoxConfigTest.java (14) M vagrant/src/test/java/org/jclouds/vagrant/internal/MachineConfigTest.java (53) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/364.patch https://github.com/jclouds/jclouds-labs/pull/364.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/364
Re: [jclouds/jclouds-karaf] [DO NOT MERGE YET] Improvements to feature (#92)
neykov commented on this pull request.
> + dependency='true'>mvn:org.apache.jclouds.common/googlecloud/${jclouds.version}
+
mvn:org.apache.jclouds.labs/google-cloud-storage/${jclouds.version}
+
+
+
+jclouds-compute
+mvn:org.apache.jclouds.labs/packet/${jclouds.version}
+
+
+
+jclouds-compute
+mvn:name.neykov/vagrant-java-bindings/${vagrant-java-bindings.version}
+mvn:org.apache.jclouds.labs/vagrant/${jclouds.version}
+
+
+
Yes, another module with another artifactId.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/92
Re: [jclouds/jclouds-karaf] [DO NOT MERGE YET] Improvements to feature (#92)
neykov commented on this pull request.
> + private static final String FEATURES_XML = "features.xml";
+
+ @Inject
+ FeaturesService featuresService;
+
+ static File getFeaturesFile() throws URISyntaxException {
+ String featuresXml = System.getProperty(FEATURES_XML);
+ Assert.assertNotNull(featuresXml);
+ File featuresFile = new File(featuresXml);
+ Assert.assertNotNull(featuresFile.exists());
+ return featuresFile;
+ }
+
+ @Test
+ public void testJcloudsFeature() throws Exception {
+ featuresService.addRepository(getFeaturesFile().toURI());
Could move this line in the `setUp` stage.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/92#pullrequestreview-20722915
Re: [jclouds/jclouds-karaf] [DO NOT MERGE YET] Improvements to feature (#92)
neykov commented on this pull request.
> + dependency='true'>mvn:org.apache.jclouds.common/googlecloud/${jclouds.version}
+
mvn:org.apache.jclouds.labs/google-cloud-storage/${jclouds.version}
+
+
+
+jclouds-compute
+mvn:org.apache.jclouds.labs/packet/${jclouds.version}
+
+
+
+jclouds-compute
+mvn:name.neykov/vagrant-java-bindings/${vagrant-java-bindings.version}
+mvn:org.apache.jclouds.labs/vagrant/${jclouds.version}
+
+
+
These are still in labs (and included in feature-labs) so remove from here?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/92#pullrequestreview-20722694
Re: [jclouds/jclouds-karaf] [DO NOT MERGE YET] Improvements to feature (#92)
neykov commented on this pull request.
> @@ -52,38 +52,56 @@ limitations under the License.
mvn:org.apache.jclouds.api/atmos/${jclouds.version}
Yes, it's not included in the latest schema above.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/92
Re: [jclouds/jclouds] [JCLOUDS-1233] bump jax-rs version (#1057)
LGTM -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1057#issuecomment-277221478
Re: [jclouds/jclouds-labs] vagrant: remove terminated nodes cache (#357)
TL;DR - We should keep the registry and expire the status only. Could save some of it in the machine description yaml. --- That's correct @nacx. We see only machines we've created (with the exception of the initial load). The provider serves the following purposes: 1. holds information that's not available elsewhere - hostname and IPs 2. cache of available machines 3. cache of the machine state To expand on each one: 1. Hostname and IPs of the machine are fetched on boot using vagrant's provisioning scripts. There are a couple of reasons for that. First vagrant calls are very expensive (relatively for a local call). Each exec takes at least several seconds. Integrating them with the boot saves a couple of ssh calls. Second the windows winrm transport is available only through the provisioning scripts. Can't call "vagrant powershell " later. This makes the information available only when creating the machine. Possible solutions: * Store those in the yaml file that's already being created, containing the machine specs to be passed to the `Vagrantfile` script. We are already storing some additional info in there like `imageId` and `hardwareId`. What I don't like about it is that the information could change so those values would become stale. * Query the machine when we need them. Not possible with Windows; not possible if the machine is halted. * Keep them as is (in memory only). When the process stops the information is lost. On a new start if doesn't know the hostname and IPs. Depending on the intended usage of the provider could be enough. Possibly a combination of the above would work best, depending on the OS. Perhaps coupled with a refreshing mechanism. The file could store other information like the tags and the metadata. 2. Existing machines list can be reconstructed easily - just listing the files on the disk. Here's the place to mention that machines created by jclouds follow some conventions. Manually created machines are not considered. The provider creates a `yaml` file describing the requirements and some meta, then a generic `Vagrantfile` reads those and creates a machine based on them. I'd say the provider is only interested in machines it creates though. Or machines created from previous runs. That's a local "service" and no concurrent modifications of the machines is expected. There could be parallel processes running the provider but still each one would manage its own machines. I'd even strongly discourage that since virtualbox (vboxmanage) has problems when it's executed in parallel. Currently the vagrant bindings explicitly serialise execs of `vagrant`. So while it's possible to make this "live" I don't see it being useful. 3. Turns out we don't ever need to query the status of a machine. The key here is that the vagrant commands are synchronous. If `vagrant up` completes successfully then the status is `RUNNING`. If it fails an exception propagates and signals an error. This makes it possible to save on expensive state polling. It gets more obvious when several machines are spun up in parallel. Since `vagrant` commands are executed sequentially a `vagrant up` would block other `vagrant status` commands for quite a while. Possible improvements: * time out the status value, refreshing it after some period on request The registry allows us to really streamline machine creation. All it takes is a single `vagrant up`. It needs around a minute to return a usable machine (obviously depends on the image and is dominated by OS boot). Whereas before introducing the registry it would take at least 50% more. And it gets worse with the more machines being created in parallel. And now to answer your questions :) > we won't see the change. Is this correct? That's correct. > And is this what we want? I think so. We are interested only in machines created by the same process. In future we could add support for adopting external machines but don't see much value in that. > (Is it that expensive to list the existing machines, that we need a cache?). Listing is pretty quick, getting the status is not instantaneous. Worst is that long commands block shorter commands. Vagrant explicitly disables parallel provisioning for machines in the same `Vagrantfile` for Virtualbox. When testing I also found that there are frequent and unexplained failures if I let it execute in parallel for different machines in different `Vagrantfile`s. That's why currently all execs are serialised. > If it turns the cache is necessary, I'd suggest to add an expiration to the > the node memoized supplier according to the session timeout property. Not absolutely needed, but very much welcome. Not quite convinced we should refresh machine list while running - wouldn't want to mess with another's process machines. Don't think Virtualbox will be happy with that. Agree we should expire the status. --- The provider makes no assumptions about the
[jclouds/jclouds-labs] vagrant: remove terminated nodes cache (#357)
Used to keep terminated nodes meta for a short while, but not needed. Also adds missing state transitions on halt, reboot. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/357 -- Commit Summary -- * Remove terminated nodes cache -- File Changes -- M vagrant/src/main/java/org/jclouds/vagrant/compute/VagrantComputeServiceAdapter.java (2) M vagrant/src/main/java/org/jclouds/vagrant/config/VagrantComputeServiceContextModule.java (13) M vagrant/src/main/java/org/jclouds/vagrant/internal/VagrantNodeRegistry.java (77) M vagrant/src/test/java/org/jclouds/vagrant/internal/VagrantNodeRegistryTest.java (20) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/357.patch https://github.com/jclouds/jclouds-labs/pull/357.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/357
Re: [jclouds/jclouds-labs] Load existing machines info on startup (#355)
neykov commented on this pull request.
> @@ -73,36 +93,41 @@ public long getDelay(TimeUnit unit) {
return unit.convert(expiryTime - timeSupplier.get(),
TimeUnit.MILLISECONDS);
}
}
- private Map nodes = new ConcurrentHashMap();
- private DelayQueue terminatedNodes = new
DelayQueue();
+
+ private final DelayQueue terminatedNodes = new
DelayQueue();
Interesting. I'll check that - agree we shouldn't keep it if not needed.
Not related to this PR, so shouldn't block merge.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/355
Re: [jclouds/jclouds-labs] Load existing machines info on startup (#355)
>Could you also elaborate a bit the motivation behind having split the Vagrant >Facade in two different interfaces? The `` started leaking in classes that don't really deal with boxes at all. Looking at the changes that must've been code that I've removed since as it was additional to `VagrantComputeServiceAdapter`. Still I think it's useful as `` is now confined just to `ImageSupplier` which indeed does handle `B`'s. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/355#issuecomment-276513508
