You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2018/01/19 13:05:15 UTC

[jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

cc @nacx 
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/1175

-- Commit Summary --

  * [JCLOUDS-1377] add support for injectable Neutron Context into Nova

-- File Changes --

    M apis/openstack-nova/pom.xml (5)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/NovaApiMetadata.java (2)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java (2)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/config/NovaComputeServiceContextModule.java (52)
    A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NeutronSecurityGroupExtension.java (382)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java (3)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CleanupResources.java (38)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CreateSecurityGroupIfNeeded.java (187)
    A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CreateSecurityGroupInRegionIfNeeded.java (89)
    A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/loaders/FindSecurityGroupInRegionOrCreate.java (83)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/loaders/FindSecurityGroupOrCreate.java (20)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java (1)
    M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java (65)
    A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/regionscoped/NeutronSecurityGroupInRegion.java (80)
    M apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceLiveTest.java (26)
    A apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NeutronSecurityGroupExtensionLiveTest.java (76)
    M apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtensionLiveTest.java (1)
    R apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/loaders/FindSecurityGroupInRegionOrCreateTest.java (12)
    M apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/CreateSecurityGroupIfNeededTest.java (5)
    M apis/openstack-nova/src/test/resources/logback-test.xml (2)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/1175.patch
https://github.com/jclouds/jclouds/pull/1175.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/1175

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
Closed #1175.

-- 
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/1175#event-1455308130

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
thanks @nacx for your help and review

-- 
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/1175#issuecomment-362618718

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

753638f  fix listSecurityGroupsForNode


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1175/files/4e0b06b2510001ccd9a83f3166989d6119255f46..753638f986a376e6abd94ccc57ea35bad91a79af

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
merging 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/pull/1175#issuecomment-362617035

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
here's my live tests results:
- for `NovaSecurityGroupExtension`
```
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Java HotSpot(TM) 64-Bit Server VM warning: ignoring option MaxPermSize=256m; support was removed in 8.0
Running org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest
Configuring TestNG with: TestNG652Configurator
Starting test testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 2618ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 5762ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test testListSecurityGroups(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
INFO: Loading security groups
INFO: Found 4 security groups
INFO: Adding security group 1806862
INFO: Loading security groups
INFO: Found 5 security groups
INFO: Removing jclouds-1806862
INFO: Loading security groups
INFO: Found 4 security groups
[TestNG] Test testListSecurityGroups(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 3267ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
Starting test testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 6281ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
Starting test testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 474ms
Test suite progress: tests succeeded: 5, failed: 0, skipped: 0.
Starting test testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 2004ms
Test suite progress: tests succeeded: 6, failed: 0, skipped: 0.
Starting test testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 2080ms
Test suite progress: tests succeeded: 7, failed: 0, skipped: 0.
Starting test testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 2295ms
Test suite progress: tests succeeded: 8, failed: 0, skipped: 0.
Starting test testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) skipped.
Test suite progress: tests succeeded: 8, failed: 0, skipped: 1.
[TestNG] Test testRemoveIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) skipped.
Test suite progress: tests succeeded: 8, failed: 0, skipped: 2.
Starting test testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest)
[TestNG] Test testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest) succeeded: 718ms
Test suite progress: tests succeeded: 9, failed: 0, skipped: 2.
Tests run: 11, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 27.195 sec - in org.jclouds.openstack.nova.v2_0.compute.extensions.NovaSecurityGroupExtensionLiveTest

Results :

Tests run: 11, Failures: 0, Errors: 0, Skipped: 2
```
and for `NeutronSecurityGroupExtensions` with Neutron-context attached
```
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest
Configuring TestNG with: TestNG652Configurator
Starting test testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
Starting test testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
Starting test testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-1] Test testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 10400ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-4] Test testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 584ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-3] Test testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) failed.
Test suite progress: tests succeeded: 2, failed: 1, skipped: 0.
[pool-2-thread-5] Test testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 2758ms
Test suite progress: tests succeeded: 3, failed: 1, skipped: 0.
Starting test testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-2] Test testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 17480ms
Test suite progress: tests succeeded: 4, failed: 1, skipped: 0.
[pool-2-thread-5] Test testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 4162ms
Test suite progress: tests succeeded: 5, failed: 1, skipped: 0.
Starting test testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 2849ms
Test suite progress: tests succeeded: 6, failed: 1, skipped: 0.
Starting test testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) skipped.
Test suite progress: tests succeeded: 6, failed: 1, skipped: 1.
[pool-2-thread-5] Test testRemoveIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) skipped.
Test suite progress: tests succeeded: 6, failed: 1, skipped: 2.
Starting test testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 1334ms
Test suite progress: tests succeeded: 7, failed: 1, skipped: 2.
Tests run: 10, Failures: 1, Errors: 0, Skipped: 2, Time elapsed: 24.094 sec <<< FAILURE! - in org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest
testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)  Time elapsed: 2.308 sec  <<< FAILURE!
java.lang.IllegalStateException: command: DELETE https://networking.uk-1.cloud.global.fujitsu.com/v2.0/security-groups/341254d6-4891-4afc-ad68-20aac0e881d4 HTTP/1.1 failed with response: HTTP/1.1 409 Conflict; content: [{"NeutronError": {"message": "Operation conflicts with other operation", "type": "Conflict", "detail": ""}}]
	at org.jclouds.openstack.neutron.v2.handlers.NeutronErrorHandler.handleError(NeutronErrorHandler.java:57)
	at org.jclouds.http.handlers.DelegatingErrorHandler.handleError(DelegatingErrorHandler.java:65)
	at org.jclouds.http.internal.BaseHttpCommandExecutorService.shouldContinue(BaseHttpCommandExecutorService.java:138)
	at org.jclouds.http.internal.BaseHttpCommandExecutorService.invoke(BaseHttpCommandExecutorService.java:107)
	at org.jclouds.rest.internal.InvokeHttpMethod.invoke(InvokeHttpMethod.java:91)
	at org.jclouds.rest.internal.InvokeHttpMethod.apply(InvokeHttpMethod.java:74)
	at org.jclouds.rest.internal.InvokeHttpMethod.apply(InvokeHttpMethod.java:45)
	at org.jclouds.reflect.FunctionalReflection$FunctionalInvocationHandler.handleInvocation(FunctionalReflection.java:117)
	at com.google.common.reflect.AbstractInvocationHandler.invoke(AbstractInvocationHandler.java:87)
	at com.sun.proxy.$Proxy86.deleteSecurityGroup(Unknown Source)
	at org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtension.removeSecurityGroup(NeutronSecurityGroupExtension.java:168)
	at org.jclouds.compute.extensions.internal.BaseSecurityGroupExtensionLiveTest.deleteSecurityGroupFromAnotherView(BaseSecurityGroupExtensionLiveTest.java:441)
	at org.jclouds.compute.extensions.internal.BaseSecurityGroupExtensionLiveTest.testSecurityGroupCacheInvalidatedWhenDeletedExternally(BaseSecurityGroupExtensionLiveTest.java:415)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	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 java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: org.jclouds.http.HttpResponseException: command: DELETE https://networking.uk-1.cloud.global.fujitsu.com/v2.0/security-groups/341254d6-4891-4afc-ad68-20aac0e881d4 HTTP/1.1 failed with response: HTTP/1.1 409 Conflict; content: [{"NeutronError": {"message": "Operation conflicts with other operation", "type": "Conflict", "detail": ""}}]
	at org.jclouds.openstack.neutron.v2.handlers.NeutronErrorHandler.handleError(NeutronErrorHandler.java:40)
	at org.jclouds.http.handlers.DelegatingErrorHandler.handleError(DelegatingErrorHandler.java:65)
	at org.jclouds.http.internal.BaseHttpCommandExecutorService.shouldContinue(BaseHttpCommandExecutorService.java:138)
	at org.jclouds.http.internal.BaseHttpCommandExecutorService.invoke(BaseHttpCommandExecutorService.java:107)
	at org.jclouds.rest.internal.InvokeHttpMethod.invoke(InvokeHttpMethod.java:91)
	at org.jclouds.rest.internal.InvokeHttpMethod.apply(InvokeHttpMethod.java:74)
	at org.jclouds.rest.internal.InvokeHttpMethod.apply(InvokeHttpMethod.java:45)
	at org.jclouds.reflect.FunctionalReflection$FunctionalInvocationHandler.handleInvocation(FunctionalReflection.java:117)
	at com.google.common.reflect.AbstractInvocationHandler.invoke(AbstractInvocationHandler.java:87)
	at com.sun.proxy.$Proxy86.deleteSecurityGroup(Unknown Source)
	at org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtension.removeSecurityGroup(NeutronSecurityGroupExtension.java:168)
	at org.jclouds.compute.extensions.internal.BaseSecurityGroupExtensionLiveTest.deleteSecurityGroupFromAnotherView(BaseSecurityGroupExtensionLiveTest.java:441)
	at org.jclouds.compute.extensions.internal.BaseSecurityGroupExtensionLiveTest.testSecurityGroupCacheInvalidatedWhenDeletedExternally(BaseSecurityGroupExtensionLiveTest.java:415)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	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 java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)


Results :

Failed tests:
  NeutronSecurityGroupExtensionLiveTest>BaseSecurityGroupExtensionLiveTest.testSecurityGroupCacheInvalidatedWhenDeletedExternally:415->BaseSecurityGroupExtensionLiveTest.deleteSecurityGroupFromAnotherView:441 ยป IllegalState

Tests run: 10, Failures: 1, Errors: 0, Skipped: 2
```

I need to understand why there is that failure

-- 
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/1175#issuecomment-361651667

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
@nacx I think I've addressed most of your comments and now the duplication of `Find/Create` SecurityGroup is gone.

I still have some expected tests to fix but please let me know if the code is more readable now. Thanks


-- 
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/1175#issuecomment-359024085

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.

Just one comment. Apart from that looks good.
Could you share the results of the live tests for the Nova and Neutron SG extensions?

> +      if (region == null) {
+         return ImmutableSet.of();
+      }
+      return getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set<SecurityGroup> listSecurityGroupsForNode(String id) {
+      RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, "id"));
+      String region = regionAndId.getRegion();
+      String instanceId = regionAndId.getId();
+
+      Optional<? extends ServerWithSecurityGroupsApi> serverApi = api.getServerWithSecurityGroupsApi(region);
+      SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+      ServerWithSecurityGroups instance = serverApi.get().get(instanceId);

This is not used later. You're always returning all security groups.

-- 
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/1175#pullrequestreview-92514310

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
rebuild please

-- 
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/1175#issuecomment-359433463

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> -   @Provides
-   @Singleton
-   @Named("SECURITYGROUP_PRESENT")
-   protected final Predicate<AtomicReference<RegionAndName>> securityGroupEventualConsistencyDelay(
-            FindSecurityGroupWithNameAndReturnTrue in,
-            @Named(TIMEOUT_SECURITYGROUP_PRESENT) long msDelay) {
-      return retry(in, msDelay, 100L, MILLISECONDS);
-   }
+//   @Provides
+//   @Singleton
+//   @Named("SECURITYGROUP_PRESENT")
+//   protected final Predicate<AtomicReference<RegionAndName>> securityGroupEventualConsistencyDelay(
+//            FindSecurityGroupWithNameAndReturnTrue in,
+//            @Named(TIMEOUT_SECURITYGROUP_PRESENT) long msDelay) {
+//      return retry(in, msDelay, 100L, MILLISECONDS);
+//   }

Remove the commented code.

> +       * builder.ipPermissions(filter(transform(group.getRules(), new
+       * Function<SecurityGroupRule, IpPermission>() {
+       * 
+       * @Override public IpPermission apply(SecurityGroupRule from) { if (from.g() ==
+       * RuleDirection.EGRESS) return null; IpPermission.Builder builder =
+       * IpPermission.builder(); if (from.getIpProtocol() != null) {
+       * builder.ipProtocol(from.getIpProtocol()); } else {
+       * builder.ipProtocol(IpProtocol.TCP); } // if (from.getFromPort() != null)
+       * builder.fromPort(from.getFromPort()); // if (from.getToPort() != null)
+       * builder.toPort(from.getToPort()); if (from.getRemoteGroupId() != null) {
+       * builder.groupId(regionId + "/" + from.getGroup()); } else if
+       * (from.getRemoteIpPrefix() != null){
+       * builder.cidrBlock(from.getRemoteIpPrefix()); }
+       * 
+       * return builder.build(); } }), Predicates.notNull())); }
+       */

Remove the commented code.

> +       * the same tenant-id-and-name as that of ruleGroup then it is not possible with
+       * the Nova /v2/12345/os-security-groups API to know which group is intended, as
+       * the only information it returns about referred groups is the tenant id and
+       * name: "group": { "tenant_id": "a0ade3ca76784719845363979dc1014e", "name":
+       * "jclouds-qa-scheduler-docker-entity" }, Rather than pick one group at random
+       * and risk using the wrong group, here we fall back to the least-worst
+       * option(?) and refuse to add any rule.
+       * 
+       * See https://issues.apache.org/jira/browse/JCLOUDS-1234.
+       * 
+       * if (referredGroup.size() != 1) { logger.
+       * warn("Ambiguous group %s used in security rule, refusing to add it to %s (%s)"
+       * , ruleGroup, owningGroup.getName(), owningGroup.getId()); return null; }
+       * builder.groupId(group.getRegion() + "/" +
+       * Iterables.getOnlyElement(referredGroup).getId()); }
+       */

Remove the commented code.

>           }
-         templateOptions.securityGroups(securityGroupName);
-         tagsBuilder.add(String.format("%s-%s", JCLOUDS_SG_PREFIX, securityGroupInRegion.getSecurityGroup().getId()));
+
+         if (securityGroup == null) {

At this point this is always null. Remove the dedundant `if` and declare the variable 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/1175#pullrequestreview-93589738

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Ignasi Barrera <no...@github.com>.
nacx 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/1175#pullrequestreview-93641279

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.



> +         return ImmutableSet.of();
+      }
+      return getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set<SecurityGroup> listSecurityGroupsForNode(String id) {
+      RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, "id"));
+      String region = regionAndId.getRegion();
+
+      SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+      final FluentIterable<org.jclouds.openstack.neutron.v2.domain.SecurityGroup> allGroups = securityGroupApi.listSecurityGroups().concat();
+      Location location = locationIndex.get().get(region);
+      return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), neutronSecurityGroupToSecurityGroup.create(location)));
+   }

good point! thanks

unfortunately also `NovaSecurityGroupExtension` is not really reliable as it is built on
`Optional<? extends ServerWithSecurityGroupsApi> serverWithSecurityGroupsApi = api.getServerWithSecurityGroupsApi(region)` which is most of the time not working on recent Openstack installations (they claim the extension is there but it is not working!)

We may need to radically change the implementation but not sure how yet


-- 
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/1175#discussion_r165008461

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Ignasi Barrera <no...@github.com>.
nacx requested changes on this pull request.

I've debugged the failure and it turns to be a test concurrency issue with the two tests that verify the behavior of the SG cache. If you apply this patch to make sure both tests don't use the same security group, tests should pass:
```diff
diff --git a/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java b/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java
index 2aa7e66c87..5c39bcb783 100644
--- a/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java
+++ b/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java
@@ -406,17 +406,18 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
 
    @Test(groups = {"integration", "live"}, singleThreaded = true)
    public void testSecurityGroupCacheInvalidatedWhenDeletedExternally() throws Exception {
+      String testSecurityGroupName = secGroupNameToDelete + "-externally";
       ComputeService computeService = view.getComputeService();
       Optional<SecurityGroupExtension> securityGroupExtension = computeService.getSecurityGroupExtension();
       assertTrue(securityGroupExtension.isPresent(), "security extension was not present");
       final SecurityGroupExtension security = securityGroupExtension.get();
-      final SecurityGroup seedGroup = security.createSecurityGroup(secGroupNameToDelete, getNodeTemplate().getLocation());
+      final SecurityGroup seedGroup = security.createSecurityGroup(testSecurityGroupName, getNodeTemplate().getLocation());
 
       deleteSecurityGroupFromAnotherView(seedGroup);
 
       boolean deleted = security.removeSecurityGroup(seedGroup.getId());
       assertFalse(deleted, "SG deleted externally so should've failed deletion");
-      final SecurityGroup recreatedGroup = security.createSecurityGroup(secGroupNameToDelete, getNodeTemplate().getLocation());
+      final SecurityGroup recreatedGroup = security.createSecurityGroup(testSecurityGroupName, getNodeTemplate().getLocation());
 
       // Makes sure the security group exists and is re-created and is not just returned from cache
       security.addIpPermission(IpPermission.builder()
```

> +         return ImmutableSet.of();
+      }
+      return getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set<SecurityGroup> listSecurityGroupsForNode(String id) {
+      RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, "id"));
+      String region = regionAndId.getRegion();
+
+      SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+      final FluentIterable<org.jclouds.openstack.neutron.v2.domain.SecurityGroup> allGroups = securityGroupApi.listSecurityGroups().concat();
+      Location location = locationIndex.get().get(region);
+      return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), neutronSecurityGroupToSecurityGroup.create(location)));
+   }

This implementation is still wrong. You have to filter ONLY the security groups associated with the given node. You're returning all existing ones.

-- 
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/1175#pullrequestreview-92833226

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

0bdbdbf  addressing Nacx comments


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1175/files/7846a2ddf32ed07e6fc394a43d954fc6fe039d28..0bdbdbf29214504f4e676e0afd6fb43f83275e4e

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

ff4a725  simplify securitygroup cache


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1175/files/753638f986a376e6abd94ccc57ea35bad91a79af..ff4a7257f537a28788cfdf3d0956808a4115c4c2

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

4e0b06b  fix removal from security group cache


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1175/files/acc72a4d46f00b8cb0465d11677f32f9806a2f2f..4e0b06b2510001ccd9a83f3166989d6119255f46

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Ignasi Barrera <no...@github.com>.
nacx requested changes on this pull request.

Thanks @andreaturli! Looks like a great starting point.

Apart from the comments I've made I found it difficult to understand the duplicated "Create/FindSecurityGroup" classes. For a proper review, could you please:

* Explain what is each new class, if it is a copy of an existing one and its purpose and need?
* Explain the main design idea so we have a clear picture of how both implementations Nova/Neutron will behave together.
* Explain a bit the different execution flows and where the decisions of using Nova or Neutron are made, so we can properly understand the context around all these classes and when they are going to take place.

> +      bind(new TypeLiteral<SecurityGroupExtension>() {
+      }).toProvider(SecurityGroupExtensionProvider.class);
+
+   }
+
+   @Singleton
+   public static class SecurityGroupExtensionProvider implements Provider<SecurityGroupExtension> {
+      @Inject(optional = true)
+      @Named("openstack-neutron")
+      protected Supplier<Context> neutronApiContextSupplier;
+
+      private final NeutronSecurityGroupExtension neutronSecurityGroupExtension;
+      private final NovaSecurityGroupExtension novaSecurityGroupExtension;
+
+      @Inject
+      public SecurityGroupExtensionProvider(NeutronSecurityGroupExtension neutronSecurityGroupExtension,

Remove the public modifier

> + */
+public class NeutronSecurityGroupExtension implements SecurityGroupExtension {
+
+   @Resource
+   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
+   protected Logger logger = Logger.NULL;
+
+   private final NovaApi api;
+   private final Supplier<Set<String>> regionIds;
+   private final GroupNamingConvention.Factory namingConvention;
+   private final LoadingCache<RegionAndName, SecurityGroup> groupCreator;
+   private final Supplier<Map<String, Location>> locationIndex;
+
+   @Inject(optional = true)
+   @Named("openstack-neutron")
+   Supplier<Context> neutronContextSupplier;

Make it `private`

> +   @Resource
+   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
+   protected Logger logger = Logger.NULL;
+
+   private final NovaApi api;
+   private final Supplier<Set<String>> regionIds;
+   private final GroupNamingConvention.Factory namingConvention;
+   private final LoadingCache<RegionAndName, SecurityGroup> groupCreator;
+   private final Supplier<Map<String, Location>> locationIndex;
+
+   @Inject(optional = true)
+   @Named("openstack-neutron")
+   Supplier<Context> neutronContextSupplier;
+
+   @Inject
+   public NeutronSecurityGroupExtension(NovaApi api,

Remove the public modifier

> +
+   @Inject(optional = true)
+   @Named("openstack-neutron")
+   Supplier<Context> neutronContextSupplier;
+
+   @Inject
+   public NeutronSecurityGroupExtension(NovaApi api,
+                                        @Region Supplier<Set<String>> regionIds,
+                                        GroupNamingConvention.Factory namingConvention,
+                                        LoadingCache<RegionAndName, SecurityGroup> groupCreator,
+                                        Supplier<Map<String, Location>> locationIndex) {
+      this.api = api;
+      this.regionIds = checkNotNull(regionIds, "regionIds");
+      this.namingConvention = checkNotNull(namingConvention, "namingConvention");
+      this.groupCreator = groupCreator;
+      this.locationIndex = checkNotNull(locationIndex, "locationIndex");

Remove the null checks. They're redundant.

> +      Location location = locationIndex.get().get(region);
+      return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), toSecurityGroup(location)));
+   }
+
+   @Override
+   public SecurityGroup getSecurityGroupById(String id) {
+      RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, "id"));
+      String region = regionAndId.getRegion();
+      String groupId = regionAndId.getId();
+
+      SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+//      final FluentIterable<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup> allGroups = securityGroupApi.listSecurityGroups();
+//      SecurityGroupInRegion rawGroup = new SecurityGroupInRegion(securityGroupApi.get(groupId), region, allGroups);
+
+//      return groupConverter.apply(rawGroup);

Remove the commented code.

> +   @Override
+   public boolean supportsGroupIds() {
+      return true;
+   }
+
+   @Override
+   public boolean supportsPortRangesForGroups() {
+      return false;
+   }
+
+   @Override
+   public boolean supportsExclusionCidrBlocks() {
+      return false;
+   }
+
+   private Function<org.jclouds.openstack.neutron.v2.domain.SecurityGroup, SecurityGroup> toSecurityGroup(final Location location) {

Move this to its own type.
The private method is a bit awkward. Create a type for this function and use google assisted inject and a factory interface to populate the Location field where needed.

>  import org.jclouds.logging.Logger;
 import org.jclouds.openstack.nova.v2_0.NovaApi;
-import org.jclouds.openstack.nova.v2_0.domain.SecurityGroup;
+import org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtension;

Unused?

>        }
    }
 
-   private void authorizeGroupToItselfAndAllIPsToTCPPort(SecurityGroupApi securityGroupApi,
-                                                         SecurityGroup securityGroup, int port) {
+   private SecurityGroupApi getNeutronSecurityGroupApi(String region) {
+      if (neutronContextSupplier == null)
+         return null;
+      return ((ApiContext<NeutronApi>) neutronContextSupplier.get()).getApi().getSecurityGroupApi(region);
+   }
+
+   // TODO remove duplications

You should be able to remove this when you have the function in a new type using assisted inject.

>        }
    }
 
+   @Override
+   public void testAScriptExecutionAfterBootWithBasicTemplate() throws Exception {
+      super.testAScriptExecutionAfterBootWithBasicTemplate();
+   }

Remove this.

> +   @Resource
+   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
+   protected Logger logger = Logger.CONSOLE;
+
+   private Context neutronApiContext;
+
+   public NeutronSecurityGroupExtensionLiveTest() {
+      provider = "openstack-nova";
+
+      Properties overrides = setupProperties();
+      neutronApiContext = ContextBuilder.newBuilder("openstack-neutron")
+               .endpoint(setIfTestSystemPropertyPresent(overrides,
+               "openstack-neutron.endpoint"))
+               .credentials(setIfTestSystemPropertyPresent(overrides,
+               "openstack-neutron.identity"),
+               setIfTestSystemPropertyPresent(overrides, "openstack-neutron.credential"))

We should be able to use the same Nova properties. It is unlikely to use a neutron that is behind a different keystone instance. Let's use the same endpoint and credentials that are configured for Nova.

> +   private Context neutronApiContext;
+
+   public NeutronSecurityGroupExtensionLiveTest() {
+      provider = "openstack-nova";
+
+      Properties overrides = setupProperties();
+      neutronApiContext = ContextBuilder.newBuilder("openstack-neutron")
+               .endpoint(setIfTestSystemPropertyPresent(overrides,
+               "openstack-neutron.endpoint"))
+               .credentials(setIfTestSystemPropertyPresent(overrides,
+               "openstack-neutron.identity"),
+               setIfTestSystemPropertyPresent(overrides, "openstack-neutron.credential"))
+              .modules(ImmutableSet.<Module>of(
+                      new SshjSshClientModule(),
+                      new SLF4JLoggingModule(),
+                      new BouncyCastleCryptoModule())

Do we really need this module?

-- 
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/1175#pullrequestreview-90106960

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
@nacx here's the result of `NeutronSecurityGroupExtensionLiveTest` against a provider with keystone v3 and Neutron support
```
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest
Configuring TestNG with: TestNG652Configurator
Starting test testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
Starting test testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
Starting test testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-1] Test testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 10013ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-4] Test testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 560ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 2908ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
Starting test testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-3] Test testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 16488ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
[pool-2-thread-2] Test testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 16835ms
Test suite progress: tests succeeded: 5, failed: 0, skipped: 0.
[pool-2-thread-5] Test testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 3464ms
Test suite progress: tests succeeded: 6, failed: 0, skipped: 0.
Starting test testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 2700ms
Test suite progress: tests succeeded: 7, failed: 0, skipped: 0.
Starting test testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) skipped.
Test suite progress: tests succeeded: 7, failed: 0, skipped: 1.
[pool-2-thread-5] Test testRemoveIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) skipped.
Test suite progress: tests succeeded: 7, failed: 0, skipped: 2.
Starting test testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest) succeeded: 1202ms
Test suite progress: tests succeeded: 8, failed: 0, skipped: 2.
Tests run: 10, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 22.939 sec - in org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest

Results :

Tests run: 10, Failures: 0, Errors: 0, Skipped: 2
```

-- 
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/1175#issuecomment-361878277

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
merged at [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=09936b5)

-- 
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/1175#issuecomment-362618900

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> +         return ImmutableSet.of();
+      }
+      return getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set<SecurityGroup> listSecurityGroupsForNode(String id) {
+      RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, "id"));
+      String region = regionAndId.getRegion();
+
+      SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+      final FluentIterable<org.jclouds.openstack.neutron.v2.domain.SecurityGroup> allGroups = securityGroupApi.listSecurityGroups().concat();
+      Location location = locationIndex.get().get(region);
+      return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), neutronSecurityGroupToSecurityGroup.create(location)));
+   }

+1 to use this. I'd say we need it before merging the PR. Agree?

-- 
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/1175#discussion_r165030128

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
@nacx thanks for the review

Main reason I have 2 couples of `Create/FindSecurityGroup` now (I want to remove them before merging this PR) is because of `NovaSecurityGroupExtension` and `NeutronSecurityGroupExtension`
`NovaSecurityGroupExtension` needs to use `NovaApi` to create security groups, while `NeutronSecurityGroupExtension` uses `NeutronApi`
Each couple of `Create/FindSecurityGroup` is a reasonably simple although ugly way to force each SecurityGroupExtension to use the appropriate API

Maybe a different way to solve the problem is to decide whether the SecurityGroupExtension needs to re-use the caching mechanism, wdyt?

-- 
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/1175#issuecomment-359014622

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.



> +         return ImmutableSet.of();
+      }
+      return getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set<SecurityGroup> listSecurityGroupsForNode(String id) {
+      RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, "id"));
+      String region = regionAndId.getRegion();
+
+      SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+      final FluentIterable<org.jclouds.openstack.neutron.v2.domain.SecurityGroup> allGroups = securityGroupApi.listSecurityGroups().concat();
+      Location location = locationIndex.get().get(region);
+      return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), neutronSecurityGroupToSecurityGroup.create(location)));
+   }

I think we should use https://developer.openstack.org/api-ref/compute/#servers-security-groups-servers-os-security-groups

-- 
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/1175#discussion_r165012937