Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SecurityGroupEditor- Allow for different jclouds prefixes on different clouds #549

Merged
merged 1 commit into from
Feb 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import java.util.Set;
import java.util.concurrent.Callable;
import java.util.regex.Pattern;

import static com.google.common.base.Preconditions.checkNotNull;

Expand All @@ -43,7 +44,7 @@
public class SecurityGroupEditor {

private static final Logger LOG = LoggerFactory.getLogger(SecurityGroupEditor.class);
public static final java.lang.String JCLOUDS_PREFIX = "jclouds#";
public static final java.lang.String JCLOUDS_PREFIX_REGEX = "^jclouds[#-]";

private final Location location;
private final SecurityGroupExtension securityApi;
Expand Down Expand Up @@ -103,6 +104,11 @@ public SecurityGroup createSecurityGroup(final String name) {
public SecurityGroup call() throws Exception {
return securityApi.createSecurityGroup(name, location);
}

@Override
public String toString() {
return "Create security group " + name;
}
};
return runOperationWithRetry(callable);
}
Expand All @@ -120,7 +126,7 @@ public boolean removeSecurityGroup(final SecurityGroup group) {
}
/**
* Removes a security group and its permissions.
* @param The jclouds id (provider id) of the group
* @param groupId The jclouds id (provider id) of the group (including region code)
* @return true if the group was found and removed.
*/
public boolean removeSecurityGroup(final String groupId) {
Expand Down Expand Up @@ -156,11 +162,11 @@ public AmbiguousGroupName(String s) {
* @throws AmbiguousGroupName in the unexpected case that the cloud returns more than one matching group.
*/
public Optional<SecurityGroup> findSecurityGroupByName(final String name) {
final String query = name.startsWith(JCLOUDS_PREFIX) ? name : JCLOUDS_PREFIX + name;
final Iterable<SecurityGroup> groupsMatching = findSecurityGroupsMatching(new Predicate<SecurityGroup>() {
final String rawName = name.replaceAll(JCLOUDS_PREFIX_REGEX, "");
@Override
public boolean apply(final SecurityGroup input) {
return input.getName().equals(query);
return input.getName().replaceAll(JCLOUDS_PREFIX_REGEX, "").equals(rawName);
}
});
final ImmutableList<SecurityGroup> matches = ImmutableList.copyOf(groupsMatching);
Expand Down Expand Up @@ -211,6 +217,11 @@ public SecurityGroup call() throws Exception {
throw Exceptions.propagate(e);
}
}

@Override
public String toString() {
return "Add permission " + permission + " to security group " + group;
}
};
return runOperationWithRetry(callable);
}
Expand Down Expand Up @@ -240,6 +251,11 @@ public SecurityGroup removePermission(final SecurityGroup group, final IpPermiss
public SecurityGroup call() throws Exception {
return securityApi.removeIpPermission(permission, group);
}

@Override
public String toString() {
return "Remove permission " + permission + " from security group " + group;
}
};
return runOperationWithRetry(callable);
}
Expand All @@ -258,13 +274,14 @@ public SecurityGroup removePermissions(SecurityGroup group, final Iterable<IpPer
protected <T> T runOperationWithRetry(Callable<T> operation) {
int backoff = 64;
Exception lastException = null;
LOG.debug("Running operation {}", operation);
for (int retries = 0; retries < 12; retries++) { // 12 = keep trying for about 5 minutes
try {
return operation.call();
} catch (Exception e) {
lastException = e;
if (isExceptionRetryable.apply(e)) {
LOG.debug("Attempt #{} failed to add security group: {}", retries + 1, e.getMessage());
LOG.debug("Attempt #{} failed to run operation, due to: {}", retries + 1, e.getMessage());
try {
Thread.sleep(backoff);
} catch (InterruptedException e1) {
Expand All @@ -277,7 +294,8 @@ protected <T> T runOperationWithRetry(Callable<T> operation) {
}
}

throw new RuntimeException("Unable to add security group rule; repeated errors from provider", lastException);
throw new RuntimeException("Unable to run operation '" + operation + "'; repeated errors from provider",
lastException);
}

@Override
Expand All @@ -298,5 +316,10 @@ public RemoveSecurityGroup(final String groupId) {
public Boolean call() throws Exception {
return securityApi.removeSecurityGroup(groupId);
}

@Override
public String toString() {
return "Remove security group " + groupId;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package org.apache.brooklyn.location.jclouds.networking;

import static org.apache.brooklyn.location.jclouds.networking.SecurityGroupEditor.JCLOUDS_PREFIX;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
Expand Down Expand Up @@ -66,6 +65,7 @@

public class JcloudsLocationSecurityGroupCustomizerTest {

private static final String JCLOUDS_PREFIX_AWS = "jclouds#";
JcloudsLocationSecurityGroupCustomizer customizer;
@Mock(answer = Answers.RETURNS_DEEP_STUBS) ComputeService computeService;
@Mock(answer = Answers.RETURNS_SMART_NULLS) Location location;
Expand Down Expand Up @@ -363,8 +363,8 @@ private SecurityGroup newGroup(String id) {

private SecurityGroup newGroup(String name, Set<IpPermission> ipPermissions) {
String id = name;
if (!name.startsWith(JCLOUDS_PREFIX)) {
id = JCLOUDS_PREFIX + name;
if (!name.startsWith(JCLOUDS_PREFIX_AWS)) {
id = JCLOUDS_PREFIX_AWS + name;
}
URI uri = null;
String ownerId = null;
Expand Down