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

Reply via email to