Re: [jclouds/jclouds-labs] [JCLOUDS-1430] Aliyun ECS (#443)
nacx requested changes on this pull request.
> +annotations-java5
+RELEASE
+compile
+
+
+org.jetbrains
+annotations-java5
+RELEASE
+compile
+
+
+org.jetbrains
+annotations-java5
+RELEASE
+compile
+
Remove all this
>.instanceName(name)
.keyPairName(keyPairName)
.tagOptions(tagOptions)
);
- String regionAndInstanceId =
RegionAndId.slashEncodeRegionAndId(regionId, instanceRequest.getInstanceId());
- instanceSuspendedPredicate.apply(regionAndInstanceId);
+ String regionAndInstanceId = slashEncodeRegionAndId(regionId,
instanceRequest.getInstanceId());
+ if (!instanceSuspendedPredicate.apply(regionAndInstanceId)) {
Add some log here explaining what went wrong.
> }
@Override
- public Image getImage(final String id) {
- Optional firstInterestingImage = Iterables.tryFind(listImages(),
new Predicate() {
- public boolean apply(Image input) {
-return input.id().equals(id);
- }
- });
- if (!firstInterestingImage.isPresent()) {
- throw new IllegalStateException("Cannot find image with the required
slug " + id);
- }
- return firstInterestingImage.get();
+ public ImageInRegion getImage(final String id) {
+ RegionAndId regionAndId = fromSlashEncoded(id);
+ return ImageInRegion.create(regionAndId.regionId(),
Iterables.getFirst(api.imageApi().list(regionAndId.regionId(),
This returns an object even if the image does not exist. That-s not correct.
Just return `null`.
> @@ -54,6 +46,7 @@ protected Properties setupProperties() {
Properties properties = super.setupProperties();
vpcId = setIfTestSystemPropertyPresent(properties, provider + ".vpcId");
vSwitchId = setIfTestSystemPropertyPresent(properties, provider +
".vSwitchId");
+ group = "jclouds"; // otherwise jclouds will use provider name as group
but `aliyun` is a forbidden prefix for tags
Then we should probably preconfigure the `prefix` property in the provider
metadata too, so users don't have to do it manually. We must provide defaults
that work.
> +// expect(serverApi.getServer(serverId)).andReturn(server);
+// }
+//
+// private void applyWithExpectedErrorMessage(String expectedErrorMessage) {
+// try {
+// cleanupServer.apply(serverId);
+// } catch (IllegalStateException e) {
+// assertEquals(expectedErrorMessage, e.getMessage());
+// }
+// }
+//
+// private void networkApiExpectations() {
+// expect(api.getNetworkApi()).andReturn(networkApi);
+// }
+//
+//}
Why is all this commented?
> +
+ final Image image =
imageInRegionToImage.apply(ImageInRegion.create(Regions.EU_CENTRAL_1.getName(),
ecsImage));
+ assertEquals(ecsImage.id(), image.getProviderId());
+ assertEquals(ecsImage.name(), image.getName());
+ assertEquals(Image.Status.AVAILABLE, image.getStatus());
+ final org.jclouds.compute.domain.OperatingSystem operatingSystem =
image.getOperatingSystem();
+
+ assertEquals(ecsImage.osName(), operatingSystem.getName());
+ assertEquals(ecsImage.description(), operatingSystem.getDescription());
+ assertTrue(operatingSystem.is64Bit());
+ assertEquals(region, image.getLocation());
+ }
+
+ Date parseDate(final String dateString) {
+ return DatatypeConverter.parseDateTime(dateString).getTime();
+ }
Add tests that check the special cases such as the `OTHER_OS_MAP`.
> + .resourceGroupId("resourceGroupId")
+ .osType("osType")
+ .osName("osName")
+ .instanceNetworkType("instanceNetworkType")
+ .hostname("hostname")
+ .creationTime(new
SimpleDateFormatDateService().iso8601DateParse("2014-03-22T05:16:45.784120972Z"))
+ .status(Instance.Status.RUNNING)
+ .clusterId("clusterId")
+ .recyclable(false)
+ .gpuSpec("")
+ .dedicatedHostAttribute(DedicatedHostAttribute.create("id",
"name"))
+ .instanceChargeType("instanceChargeType")
+ .gpuAmount(1)
+ .expiredTime(new
SimpleDateFormatDateService().iso8601DateParse("2014-03-22T09:16:45.784120972Z"))
+ .innerIpAddress(ImmutableMap.>of("IpAddress", ImmutableList.of("192.168.0.1")))
+ .publicIpAddress(ImmutableMap.>of("IpAddress", ImmutableList.of("47.254.152.220")))
Add more values to the addresses to make sure to test we them all.
> + }
+
+ private void assertNodeMetadata(NodeMetadata result,
org.jclouds.compute.domain.OperatingSystem os, String imageId,
+ NodeMetadata.Status status, List
privateIpAddresses, List publicIpAddresses) {
+ assertNotNull(result);
+ assertEquals(resu
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] Aliyun ECS (#443)
andreaturli commented on this pull request. > +annotations-java5 +RELEASE +compile + + +org.jetbrains +annotations-java5 +RELEASE +compile + + +org.jetbrains +annotations-java5 +RELEASE +compile + where this crap come from? :( -- 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#discussion_r207828506
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] Aliyun ECS (#443)
andreaturli commented on this pull request.
> @@ -54,6 +46,7 @@ protected Properties setupProperties() {
Properties properties = super.setupProperties();
vpcId = setIfTestSystemPropertyPresent(properties, provider + ".vpcId");
vSwitchId = setIfTestSystemPropertyPresent(properties, provider +
".vSwitchId");
+ group = "jclouds"; // otherwise jclouds will use provider name as group
but `aliyun` is a forbidden prefix for tags
good point, Ill look into 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/443#discussion_r207828745
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] Aliyun ECS (#443)
andreaturli commented on this pull request.
> +// expect(serverApi.getServer(serverId)).andReturn(server);
+// }
+//
+// private void applyWithExpectedErrorMessage(String expectedErrorMessage) {
+// try {
+// cleanupServer.apply(serverId);
+// } catch (IllegalStateException e) {
+// assertEquals(expectedErrorMessage, e.getMessage());
+// }
+// }
+//
+// private void networkApiExpectations() {
+// expect(api.getNetworkApi()).andReturn(networkApi);
+// }
+//
+//}
still working on this, forget to exclude from commit, sorry
--
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#discussion_r207828836
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] Aliyun ECS (#443)
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 internetChargeType) {
+ return new AutoValue_EipAddress(ipAddress, allocationId,
internetChargeType);
+ }
+
+ public abstract String ipAddress();
+
+ public abstract String allocationId();
+
+ public abstract String internetChargeType();
don't know, they are not well documented
--
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#discussion_r207833707
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] Aliyun ECS (#443)
andreaturli commented on this pull request.
> @@ -54,6 +46,7 @@ protected Properties setupProperties() {
Properties properties = super.setupProperties();
vpcId = setIfTestSystemPropertyPresent(properties, provider + ".vpcId");
vSwitchId = setIfTestSystemPropertyPresent(properties, provider +
".vSwitchId");
+ group = "jclouds"; // otherwise jclouds will use provider name as group
but `aliyun` is a forbidden prefix for tags
@nacx as it can't start with `aliyun` shall I use `alibaba-ecs` as id for the
provider in the `ECSComputeServiceProviderMetadata` ?
--
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#discussion_r207836467
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] Aliyun ECS (#443)
nacx commented on this pull request.
> @@ -54,6 +46,7 @@ protected Properties setupProperties() {
Properties properties = super.setupProperties();
vpcId = setIfTestSystemPropertyPresent(properties, provider + ".vpcId");
vSwitchId = setIfTestSystemPropertyPresent(properties, provider +
".vSwitchId");
+ group = "jclouds"; // otherwise jclouds will use provider name as group
but `aliyun` is a forbidden prefix for tags
Sounds good. Preconfigure that and remove this 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-labs/pull/443#discussion_r207868271
