Closed #443.
--
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/443#event-1840127835
merger at
[master](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=a5dbf0065d8fa8cabcaf020b7c10fe2f7ccf8d6a)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
thanks @nacx, merging it now
--
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/443#issuecomment-420501158
Thanks, @andreaturli! I think we should merge this now and keep iterating on
it. Thanks for all the effort!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
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-labs/pull/443#pullrequestreview-153710254
@andreaturli pushed 1 commit.
0673eb8 fix NPE in InstanceToNodeMetadata
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Re-run the live tests, slightly better
Tests run: 55, Failures: 9, Errors: 0, Skipped: 6
`SshKeyPairApiLiveTest.testImport` - I'm afraid the API is broken
`ECSComputeServiceLiveTest.testCorrectAuthException` returns
`ResourceNotFoundException` instead of `AuthorizationException` not entirely
here's the live tests result
```
Results :
Failed tests:
SshKeyPairApiLiveTest.testImport:58 » IllegalArgument
{"RequestId":"4084B85E-D...
ECSComputeServiceLiveTest.testCorrectAuthException » Test
Method BaseComputeS...
sorry again as this PR is becoming huge. @nacx I think I've addressed all of
your comments now and I've also simplified the RetryHandler for 4xx errors
I'm running the live tests to see the result, some of them are expected to fail
though - particularly from `ECSComputeServiceLiveTest`. I'll
andreaturli commented on this pull request.
> +import org.jclouds.http.HttpRetryHandler;
+import org.jclouds.http.HttpUtils;
+import org.jclouds.http.annotation.ClientError;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
+import org.jclouds.json.Json;
+
+import
andreaturli commented on this pull request.
> + this.instanceSuspendedPredicate = instanceSuspendedPredicate;
+ this.regionIds = regionIds;
+ this.cleanupResources = cleanupResources;
+ }
+
+ @Override
+ public NodeAndInitialCredentials
andreaturli commented on this pull request.
> +
+ public void testList() {
+ final AtomicInteger found = new AtomicInteger(0);
+
assertTrue(Iterables.all(api().list(Regions.EU_CENTRAL_1.getName()).concat(),
new Predicate() {
+ @Override
+ public boolean
andreaturli commented on this pull request.
> +
+ public void testList() {
+ final AtomicInteger found = new AtomicInteger(0);
+
assertTrue(Iterables.all(api().list(Regions.EU_CENTRAL_1.getName()).concat(),
new Predicate() {
+ @Override
+ public boolean apply(VPC
andreaturli commented on this pull request.
> + * 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
andreaturli commented on this pull request.
> +
+ final Image image =
imageInRegionToImage.apply(ImageInRegion.create(Regions.EU_CENTRAL_1.getName(),
ecsImage));
+ assertEquals(ecsImage.id(), image.getProviderId());
+ assertEquals(ecsImage.name(), image.getName());
+
@nacx it is a wip, so please don't bother to review it yet. I'll ping when I
think it's ready for your review, ok?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@andreaturli pushed 2 commits.
2d991e1 wip
88d83f3 more comments addressed
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
andreaturli commented on this pull request.
> +// public String apply(Map.Entry input) {
+//return String.format(PORT_RANGE_FORMAT, input.getKey(),
input.getValue());
+// }
+// }));
+//
+// for (SecurityGroup securityGroup :
andreaturli commented on this pull request.
> public class BaseECSComputeServiceApiLiveTest extends
> BaseApiLiveTest {
+ protected static final String TEST_REGION = Regions.EU_CENTRAL_1.getName();
ok done
--
You are receiving this because you are subscribed to this thread.
Reply to
andreaturli commented on this pull request.
> @@ -23,6 +23,10 @@
@AutoValue
public abstract class Tag {
+ public static final String DEFAULT_OWNER_KEY = "owner";
+ public static final String DEFAULT_OWNER_VALUE = "jclouds";
+ public static final String GROUP = "group";
not sure we
nacx commented on this pull request.
> @@ -50,7 +51,11 @@ public void handleError(HttpCommand command, HttpResponse
> response) {
break;
case 401:
case 403:
-exception = new AuthorizationException(message, exception);
+if
nacx commented on this pull request.
> +// public String apply(Map.Entry input) {
+//return String.format(PORT_RANGE_FORMAT, input.getKey(),
input.getValue());
+// }
+// }));
+//
+// for (SecurityGroup securityGroup :
nacx commented on this pull request.
> +
+ public DependencyViolationException() {
+ super();
+ }
+
+ public DependencyViolationException(String arg0, Throwable arg1) {
+ super(arg0, arg1);
+ }
+
+ public DependencyViolationException(String arg0) {
+ super(arg0);
+ }
andreaturli commented on this pull request.
> +
+ public abstract String description();
+
+ public abstract String regionId();
+
+ public abstract String status();
+
+ public abstract Map> userCidrs();
+
+ public abstract String vRouterId();
+
+ public abstract Map> vSwitchIds();
+
andreaturli commented on this pull request.
> + checkTagsInNodeEquals(node, tags);
+
+ getAnonymousLogger().info(
+ format("<< available node(%s) os(%s) in %ss", node.getId(),
node.getOperatingSystem(), createSeconds));
+
+ watch.reset().start();
+
+
andreaturli commented on this pull request.
> +
+ public abstract Date creationTime();
+
+ public abstract String description();
+
+ public abstract String zoneId();
+
+ public abstract String status();
+
+ public abstract int availableIpAddressCount();
+
+ public abstract String
andreaturli 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,
andreaturli commented on this pull request.
> +import org.jclouds.rest.annotations.RequestFilters;
+import org.jclouds.rest.annotations.ResponseParser;
+import org.jclouds.rest.annotations.Transform;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
@andreaturli pushed 1 commit.
801266b addressing more comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/443/files/3205bbfc559eeeda59f2f7994f1aa179d16057e0..801266bac92ae8d6c54cdec540b1860b95f8cb94
@andreaturli pushed 1 commit.
b2ce3b5 address some comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/443/files/b79a887eb460662edb19201977ae961f208dff88..b2ce3b5056c0330d747f04e85dfa815685422e4d
andreaturli commented on this pull request.
> + checkTagsInNodeEquals(node, tags);
+
+ getAnonymousLogger().info(
+ format("<< available node(%s) os(%s) in %ss", node.getId(),
node.getOperatingSystem(), createSeconds));
+
+ watch.reset().start();
+
+
nacx requested changes on this pull request.
> @@ -104,11 +117,39 @@ protected
> CreateResourcesThenCreateNodes(CreateNodeWithGroupEncodedIntoName addN
String regionId = template.getLocation().getId();
ECSServiceTemplateOptions options =
andreaturli commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
nacx commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
deadNode.getGroup());
danielestevez commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
andreaturli commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
danielestevez commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
andreaturli commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
andreaturli commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
danielestevez commented on this pull request.
> +import org.jclouds.rest.annotations.RequestFilters;
+import org.jclouds.rest.annotations.ResponseParser;
+import org.jclouds.rest.annotations.Transform;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
danielestevez commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
andreaturli commented on this pull request.
> } catch (Exception ex) {
logger.warn(ex, "Error cleaning up resources for node %s",
deadNode);
}
+
+ List securityGroups =
cleanupResources.findOrphanedSecurityGroups(regionAndId.regionId(),
@andreaturli pushed 4 commits.
701f36e add pagination to instanceStatus api
9dc04fb add network apis
ebd1642 improve CreateResourcesThenCreateNodes
b79a887 wip
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
nacx commented on this pull request.
> @@ -54,6 +46,7 @@ protected Properties setupProperties() {
Properties properties = super.setupProperties();
vpcId = setIfTestSystemPropertyPresent(properties, provider + ".vpcId");
vSwitchId =
andreaturli commented on this pull request.
> @@ -54,6 +46,7 @@ protected Properties setupProperties() {
Properties properties = super.setupProperties();
vpcId = setIfTestSystemPropertyPresent(properties, provider + ".vpcId");
vSwitchId =
andreaturli commented on this pull request.
> +@AutoValue
+public abstract class EipAddress {
+
+ EipAddress() {
+ }
+
+ @SerializedNames({ "IpAddress", "AllocationId", "InternetChargeType" })
+ public static EipAddress create(String ipAddress, String allocationId,
String
andreaturli commented on this pull request.
> +// expect(serverApi.getServer(serverId)).andReturn(server);
+// }
+//
+// private void applyWithExpectedErrorMessage(String expectedErrorMessage) {
+// try {
+// cleanupServer.apply(serverId);
+// } catch
andreaturli commented on this pull request.
> @@ -54,6 +46,7 @@ protected Properties setupProperties() {
Properties properties = super.setupProperties();
vpcId = setIfTestSystemPropertyPresent(properties, provider + ".vpcId");
vSwitchId =
andreaturli commented on this pull request.
> +annotations-java5
+RELEASE
+compile
+
+
+org.jetbrains
+annotations-java5
+RELEASE
+compile
+
+
+org.jetbrains
+
nacx requested changes on this pull request.
> +annotations-java5
+RELEASE
+compile
+
+
+org.jetbrains
+annotations-java5
+RELEASE
+compile
+
+
+org.jetbrains
+
@andreaturli pushed 1 commit.
88d0852 address comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/443/files/39fef5305d6cdfb660ecee32244eba7e7f5f68b5..88d085280a99fcc7bb799721b873661fbb3feaeb
nacx commented on this pull request.
> + vSwitchId = setIfTestSystemPropertyPresent(properties, provider +
> ".vSwitchId");
+ return properties;
+ }
+
+ @Override
+ protected TemplateBuilder templateBuilder() {
+ return super.templateBuilder()
+
nacx commented on this pull request.
> + this.instanceSuspendedPredicate = instanceSuspendedPredicate;
+ this.regionIds = regionIds;
+ this.cleanupResources = cleanupResources;
+ }
+
+ @Override
+ public NodeAndInitialCredentials
createNodeWithGroupEncodedIntoName(String
andreaturli commented on this pull request.
> + vSwitchId = setIfTestSystemPropertyPresent(properties, provider +
> ".vSwitchId");
+ return properties;
+ }
+
+ @Override
+ protected TemplateBuilder templateBuilder() {
+ return super.templateBuilder()
+
andreaturli commented on this pull request.
> @@ -92,4 +94,14 @@ public boolean apply(@Nullable String input) {
}), "Cannot starts with " + Iterables.toString(FORBIDDEN_PREFIX));
}
+ /**
+* This is strictly not needed but apparently tags with `-` can create a
problem when
andreaturli commented on this pull request.
> +
+ String securityGroupId = null;
+ if (!options.getGroups().isEmpty()) {
+ Iterable securityGroupNames =
api.securityGroupApi().list(location.getId()).concat().transform(new
Function() {
+@Override
+
andreaturli commented on this pull request.
> + } else {
+logger.warn(">> security group: (%s) has not been deleted.",
securityGroupId);
+ }
+ }
+
+ return instanceDeleted;
+ }
+
+ private List getSecurityGroupIdsUsedByNode(RegionAndId regionAndId)
{
andreaturli commented on this pull request.
> + if (hardware.isPresent()) {
+ builder.hardware(hardware.get());
+ } else {
+ logger.info(">> hardware with id %s for instance %s was not found. "
+ + "This might be because the image that was used
andreaturli commented on this pull request.
> +
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+import org.jclouds.compute.options.TemplateOptions;
+
+import static com.google.common.base.Objects.equal;
+
+/**
+ * Custom options for the Alibaba Elastic
andreaturli commented on this pull request.
> + private final Supplier> hardwares;
+ private final Supplier> locations;
+ private final Function
toPortableStatus;
+ private final GroupNamingConvention groupNamingConvention;
+ @Resource @Named(ComputeServiceConstants.COMPUTE_LOGGER)
andreaturli commented on this pull request.
> + @Override
+ public boolean apply(@Nullable InstanceType input) {
+return contains(instanceTypeIds.build(),
input.instanceTypeId());
+ }
+ }).toList();
+
+
andreaturli commented on this pull request.
> + transform(listLocations(), new Function() {
+ @Override
+ public String apply(Region location) {
+return location.id();
+ }
+ }));
+
+ for (String
andreaturli commented on this pull request.
> + for (String regionId : availableLocationNames) {
+ instanceTypeIds.addAll(getInstanceTypeId(regionId));
+ }
+
+ List instanceTypes =
FluentIterable.from(api.instanceApi().listTypes())
+ .filter(new Predicate()
andreaturli commented on this pull request.
> +
+ for (String regionId : availableLocationNames) {
+ images.addAll(api.imageApi().list(regionId).concat());
+ }
+ return images.build();
+ }
+
+ @Override
+ public Image getImage(final String id) {
+ Optional
andreaturli commented on this pull request.
> + transform(listLocations(), new Function() {
+ @Override
+ public String apply(Region location) {
+return location.id();
+ }
+ }));
+
+ for (String
andreaturli commented on this pull request.
> +}
+ }
+ }
+ return instanceTypeIds;
+ }
+
+ @Override
+ public Iterable listImages() {
+ final ImmutableList.Builder images = ImmutableList.builder();
+ final List availableLocationNames = newArrayList(
andreaturli commented on this pull request.
> +
+ InstanceRequest instanceRequest = api.instanceApi().create(regionId,
imageId, securityGroupId, name, instanceType,
+ CreateInstanceOptions.Builder
+ .vSwitchId(vSwitchId)
+
andreaturli commented on this pull request.
> + this.instanceSuspendedPredicate = instanceSuspendedPredicate;
+ this.regionIds = regionIds;
+ this.cleanupResources = cleanupResources;
+ }
+
+ @Override
+ public NodeAndInitialCredentials
nacx commented on this pull request.
Thanks, @andreaturli!
There are many comments but in general, it looks pretty good.
Apart from the comments, we need proper tests for most of the things.
Basically, every class in the `compute/functions` package needs to have a
proper unit test.
Also, add
- add instance API
- add compute abstraction
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/443
-- Commit Summary --
* [JCLOUDS-1430] Aliyun ECS
-- File Changes --
A aliyun-ecs/README.md (47)
M
70 matches
Mail list logo
