Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

Closed #431.

-- 
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/431#event-2059949881

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 2 commits.

c7b21e6  minor build fixes
d17319e  minor changes


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/60147d505ecf776574f4259ff00efe7cd9a0e472..d17319e16d03dc8d6b43097fda82adc4e4a9

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 8 commits.

202b03e  Baremetal update
dd664e0  updates
f5b715d  Baremetal update
a4dfa41  Add baremetal logic to ComputeServiceAdapter
ba010cc  test fix
5a027df  Update code for review
9f41453  JCLOUDS-1386 1&1 Baremetal servers
e4dd572  Merge branch 'oneandone-baremetal-update' of 
https://github.com/StackPointCloud/jclouds-labs into oneandone-baremetal-update


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/1903c2ccac1f0b22a9e73a32806a44339c9748a3..e4dd5723b3113eecca52b60403fbe0cd2b5f413c

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -78,26 +81,43 @@ public ServerToNodeMetadata(Function 
> fnVolume,
@Override
public NodeMetadata apply(final Server server) {
   checkNotNull(server, "Null server");
+  String hardwareId = null;
 
   DataCenter dataCenter = 
api.dataCenterApi().get(server.datacenter().id());
   Location location = find(locations.get(), idEquals(dataCenter.id()));

changed

-- 
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/431#discussion_r212283174

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



>   }
   }
 
-  // provision server
-  Server server = null;
-  Double cores = ComputeServiceUtils.getCores(hardware);
-  Double ram = (double) hardware.getRam();
-  if (ram < 1024) {
- ram = 0.5;
-  } else {
- ram = ram / 1024;
+  //configuring Firewall rules
+  Map portsRange = getPortRangesFromList(inboundPorts);

I suggest we open a separate PR addressing any backwards concern you might have 

-- 
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/431#discussion_r212277861

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> -   SingleServerAppliance appliance = 
> api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  Hardware hardwareModel = 
generateHardwareRequest.isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);
+  ServerAppliance workingImage;
+
+  //choose the correct image based on the server type baremetal or cloud

no need as the PR will get even more complicated to 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-labs/pull/431#discussion_r212277695

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -179,9 +179,46 @@
   return new NodeAndInitialCredentials(updateServer, 
updateServer.id(), serverCredentials);
}
 
+   private ServerAppliance findWorkingImage(String imageId, 
Types.ServerTypeCompatibility serverType) {

changed

-- 
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/431#discussion_r212277619

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -253,7 +253,7 @@ public static ApplianceType fromValue(String v) {
}
 
public enum OSImageType {
-  Standard, Minimal, Personal, ISO_OS, ISO_TOOL, NULL, UNRECOGNIZED;
+ STANDARD, MINIMAL, Personal, ISO_OS, ISO_TOOL, NULL, UNRECOGNIZED;

done

-- 
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/431#discussion_r212277586

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -283,4 +283,22 @@ public String toString() {
  return value;
   }
}
+
+   public enum ServerType {
+  cloud, baremetal, UNRECOGNIZED;

done

-- 
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/431#discussion_r212277605

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -283,4 +283,22 @@ public String toString() {
  return value;
   }
}
+
+   public enum ServerType {
+  cloud, baremetal, UNRECOGNIZED;
+
+  public static ServerType fromValue(String v) {
+ return Enums.getIfPresent(ServerType.class, v).or(UNRECOGNIZED);
+  }
+
+   }
+
+   public enum ServerTypeCompatibility {
+  vps, cloud, baremetal, UNRECOGNIZED;

done

-- 
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/431#discussion_r212277592

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli a few of your comments do make sense about this PR, i will address 
those issues so we can move on.

I don't see why i would separate Recovery Images it is a very small feature and 
i am not sure that will really affect reviewing the PR. In my opinion i think 
that most of the things that you are asking me to change will make reviewing 
this PR difficult, like changing firewall policies and moving few lines of code 
into a separate method, changes like that made this PR look messy and increase 
the diff.

I agree with you about being committed to jClouds best practices, but from my 
experience this is not the right PR to go back and review things that were 
already accepted by jClouds.

I have addressed all the small changes that make sense, i can post the live 
test results yet again.I hope you agree with me about closing this PR before 
its gets even worst

-- 
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/431#issuecomment-415386348

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> }
 
@Override
public void suspendNode(String id) {
   waitServerUntilAvailable.apply(getNode(id));
-  api.serverApi().updateStatus(id, 
Server.UpdateStatus.create(Types.ServerAction.POWER_OFF, 
Types.ServerActionMethod.HARDWARE));
+  api.serverApi().updateStatus(id, 
Server.UpdateStatus.create(Types.ServerAction.POWER_OFF, 
Types.ServerActionMethod.HARDWARE, false, null));

it will remain powered off


-- 
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/431#discussion_r212275678

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> }
 
@Override
public void resumeNode(String id) {
-  api.serverApi().updateStatus(id, 
Server.UpdateStatus.create(Types.ServerAction.POWER_ON, 
Types.ServerActionMethod.HARDWARE));
+  api.serverApi().updateStatus(id, 
Server.UpdateStatus.create(Types.ServerAction.POWER_ON, 
Types.ServerActionMethod.HARDWARE, false, null));

nothing happens


-- 
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/431#discussion_r212275634

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

andreaturli requested changes on this pull request.

@alibazlamit I've added some more comments but I could not finish up the review 
as the code is becoming too complicated to be reviewed. As I told you other 
times it would be much easier to add small chunks of code in small PRs. 
For example you could focus on a PR that adds `RecoveryImageApi` with Mock and 
Live Tests only (+ domain objects needed)

Then we can focus on the baremetal feature itself as it requires a huge number 
of changes! Does it sound like a plan?

Thanks again

> @@ -253,7 +253,7 @@ public static ApplianceType fromValue(String v) {
}
 
public enum OSImageType {
-  Standard, Minimal, Personal, ISO_OS, ISO_TOOL, NULL, UNRECOGNIZED;
+ STANDARD, MINIMAL, Personal, ISO_OS, ISO_TOOL, NULL, UNRECOGNIZED;

please make it all capital

> @@ -283,4 +283,22 @@ public String toString() {
  return value;
   }
}
+
+   public enum ServerType {
+  cloud, baremetal, UNRECOGNIZED;
+
+  public static ServerType fromValue(String v) {
+ return Enums.getIfPresent(ServerType.class, v).or(UNRECOGNIZED);
+  }
+
+   }
+
+   public enum ServerTypeCompatibility {
+  vps, cloud, baremetal, UNRECOGNIZED;

upper case?

> @@ -283,4 +283,22 @@ public String toString() {
  return value;
   }
}
+
+   public enum ServerType {
+  cloud, baremetal, UNRECOGNIZED;

why not upper case?

> @@ -179,9 +179,46 @@
   return new NodeAndInitialCredentials(updateServer, 
updateServer.id(), serverCredentials);
}
 
+   private ServerAppliance findWorkingImage(String imageId, 
Types.ServerTypeCompatibility serverType) {

why this name? it seems that it tries to find a `ServerAppliance`, doesn't it?

> -//check if the bootable device has enough size to run the 
> appliance(image).
-float minHddSize = volume.getSize();
-if (volume.isBootDevice()) {
-   SingleServerAppliance appliance = 
api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  Hardware hardwareModel = 
generateHardwareRequest.isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);

wouldn't be better to introduce a custom `TemplateOptions` where you can pass 
in `bareMetal` boolean parameter? see 
https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/main/java/org/jclouds/digitalocean2/compute/options/DigitalOcean2TemplateOptions.java#L38
 for an example

> -   SingleServerAppliance appliance = 
> api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  Hardware hardwareModel = 
generateHardwareRequest.isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);
+  ServerAppliance workingImage;
+
+  //choose the correct image based on the server type baremetal or cloud

move this business logic into its own method

>   }
   }
 
-  // provision server
-  Server server = null;
-  Double cores = ComputeServiceUtils.getCores(hardware);
-  Double ram = (double) hardware.getRam();
-  if (ram < 1024) {
- ram = 0.5;
-  } else {
- ram = ram / 1024;
+  //configuring Firewall rules
+  Map portsRange = getPortRangesFromList(inboundPorts);

all the reasonings for the firewalls / inbound ports shouldn't probably happen 
in the adapter: firewalls are created once and attached to all the nodes of a 
group. See `CreateResourcesThenCreateNodes` as a reference at 
https://github.com/jclouds/jclouds/blob/master/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/strategy/CreateResourcesThenCreateNodes.java#L138

> - ram = ram / 1024;
+  //configuring Firewall rules
+  Map portsRange = getPortRangesFromList(inboundPorts);
+  List rules = new 
ArrayList();
+
+  for (Map.Entry range : portsRange.entrySet()) {
+ FirewallPolicy.Rule.CreatePayload rule = 
FirewallPolicy.Rule.CreatePayload.builder()
+ .portFrom(range.getKey())
+ .portTo(range.getValue())
+ 

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

andreaturli commented on this pull request.



>  
@Inject
OneandoneComputeServiceAdapter(OneAndOneApi api, CleanupResources 
cleanupResources,
+   GenerateHardwareRequest generateHardwareRequest,
@Named(POLL_PREDICATE_SERVER) Predicate 
waitServerUntilAvailable,
PasswordGenerator.Config passwordGenerator) {

`PasswordGenerator.Config passwordGenerator` is not used, remove 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/431#discussion_r212021489

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli  I have addressed the issues you mentioned, could you now please 
take a look?

-- 
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/431#issuecomment-414971479

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -125,4 +126,28 @@ public boolean apply(Server server) {
  || server.status().percent() != 0);
   }
}
+
+   @Provides
+   @Singleton
+   @Named(POLL_PREDICATE_SERVER_SUSPENDED)

I have changed it to `TIMEOUT_NODE_SUSPENDED`

-- 
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/431#discussion_r206911564

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 1 commit.

1903c2c  Change POLL_PREDICATE_SERVER_SUSPENDED to TIMEOUT_NODE_SUSPENDED


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/7fbe8d9331964a1d2b5c75918eb95abcd05cab44..1903c2ccac1f0b22a9e73a32806a44339c9748a3

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

@alibazlamit I've added a question to help the discussion. Can we start from 
there?

-- 
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/431#issuecomment-409595895

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

andreaturli commented on this pull request.



> @@ -125,4 +126,28 @@ public boolean apply(Server server) {
  || server.status().percent() != 0);
   }
}
+
+   @Provides
+   @Singleton
+   @Named(POLL_PREDICATE_SERVER_SUSPENDED)

@alibazlamit why do we need this `POLL_PREDICATE_SERVER_SUSPENDED` when jclouds 
has `TIMEOUT_NODE_SUSPENDED` ?

-- 
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/431#discussion_r206903040

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli Could you please let me know which issues are not addressed, i 
replied to every you comment you had, some changes are applied the others are 
explained as you can see.

-- 
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/431#issuecomment-409531204

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -19,6 +19,7 @@
 public final class OneAndOneProperties {
 
public static final String POLL_PREDICATE_SERVER = 
"jclouds.oneandone.rest.predicate.server";
+   public static final String POLL_PREDICATE_SERVER_SUSPENDED = 
"jclouds.oneandone.rest.predicate.serverdelete";

I don't quite get what is required here could you please explain more?

-- 
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/431#discussion_r206828912

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



>try {
- org.apache.jclouds.oneandone.rest.domain.Hardware.CreateHardware 
hardwareRequest
- = 
org.apache.jclouds.oneandone.rest.domain.Hardware.CreateHardware.create(cores, 
1, ram, hdds);
+ if (hardwareModel != null) {

done 
[here](https://github.com/StackPointCloud/jclouds-labs/blob/oneandone-baremetal-update/oneandone/src/main/java/org/apache/jclouds/oneandone/rest/compute/strategy/GenerateHardwareRequest.java)

-- 
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/431#discussion_r206828741

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -65,6 +66,7 @@
private final OneAndOneApi api;
private final Predicate waitServerUntilAvailable;
private final PasswordGenerator.Config passwordGenerator;

removed

-- 
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/431#discussion_r206828100

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 1 commit.

7fbe8d9  minor commit


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/4b4172f6e6bc50fb3b4b023b05cb7822f9ca53d6..7fbe8d9331964a1d2b5c75918eb95abcd05cab44

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

@alibazlamit I think there are still unaddressed issues. Can you please address 
them first?

-- 
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/431#issuecomment-409300125

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 1 commit.

861321b  Merge branch 'master' of https://github.com/jclouds/jclouds-labs into 
oneandone-baremetal-update


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/6b289e8cc80ce0e8e1740953ecb250bebd321633..861321bcc2ff80d6ebb61c408145a0fc1879db5b

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli Can you please take a look at this PR and let me know when can we 
move on with 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/431#issuecomment-403967229

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli @nacx just a friendly reminder, i have addresses all of the 
comments either by applying the appropriate changes or be replying to them 
here, could we please move on with this PR?

-- 
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/431#issuecomment-392109541

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli I have addressed all the issues you have reviewd, [here 
](https://gist.github.com/alibazlamit/c7bd2044589d8a77a34a7d451cddc57a) is the 
live test results.
Let me know if there is anything else i can do to get this PR moving forward.

-- 
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/431#issuecomment-390322300

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 1 commit.

1be7bec  remove test file


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/b6531dcab36e7c9dfb7405737e53eb8ef24faae8..1be7bec42ee88c4335c5c72b4f54606272acaa87

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 2 commits.

6238700  Merge remote-tracking branch 'upstream/master' into 
oneandone-baremetal-update
b6531dc  Addressed review comments


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/8a97c6e2bc03de788bc9ea5bed4fa5d3d428e51d..b6531dcab36e7c9dfb7405737e53eb8ef24faae8

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

> andreaturli commented on Feb 14  
Not familiar with the oneandone API but looks like baremetal is another 
servertype for server creation.
> 
> If this is correct, then we could either introduce a OneAndOneTemplateOption 
> for serverType or try to infer it from hardwareProfiles if this additional 
> flag is available. Makes sense?

@andreaturli I am looking into that option just in case, could you please point 
me to an example where one of the current providers actually use and 
TemplateOption, i would like to investigate that and see if it would fit the 
case 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/431#issuecomment-387256499

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

Reopened #431.

-- 
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/431#event-1613377500

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

Closed #431.

-- 
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/431#event-1613377382

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



>   }
   }
 
-  // provision server
-  Server server = null;
-  Double cores = ComputeServiceUtils.getCores(hardware);
-  Double ram = (double) hardware.getRam();
-  if (ram < 1024) {
- ram = 0.5;
-  } else {
- ram = ram / 1024;
+  //configuring Firewall rules
+  Map portsRange = getPortRangesFromList(inboundPorts);

This has just been moved to the GenerateHardwareRequest.java to organize code 
just as you request to separate the creation of the hardware 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/431#discussion_r186477609

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> - ram = ram / 1024;
+  //configuring Firewall rules
+  Map portsRange = getPortRangesFromList(inboundPorts);
+  List rules = new 
ArrayList();
+
+  for (Map.Entry range : portsRange.entrySet()) {
+ FirewallPolicy.Rule.CreatePayload rule = 
FirewallPolicy.Rule.CreatePayload.builder()
+ .portFrom(range.getKey())
+ .portTo(range.getValue())
+ .protocol(Types.RuleProtocol.TCP)
+ .build();
+ rules.add(rule);
+  }
+  String firewallPolicyId = "";
+  if (inboundPorts.length > 0) {
+ FirewallPolicy firewallPolicy = 
api.firewallPolicyApi().create(FirewallPolicy.CreateFirewallPolicy.create(name 
+ " firewall policy", "desc", rules));

It does handle the creation correctly as all the live tests passes, if you 
think there is a necessary change here, we can do that in another PR that 
addresses improvements.

This bulk of code was moved up just to organize the process more. it has been 
merged before since the first PR

-- 
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/431#discussion_r186477243

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -143,23 +160,6 @@
 
  updateServer = api.serverApi().get(server.id());
 
- Map portsRange = 
getPortRangesFromList(inboundPorts);

This bulk of code was moved up

-- 
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/431#pullrequestreview-118062400

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> }
 
@Override
public NodeAndInitialCredentials 
createNodeWithGroupEncodedIntoName(String group, String name, Template 
template) {
 
-  final String dataCenterId = template.getLocation().getId();
+  final String dataCenterId ="908DC2072407C94C8054610AD5A53B8C";// 
template.getLocation().getId();
   Hardware hardware = template.getHardware();
   TemplateOptions options = template.getOptions();
   Server updateServer = null;

removed

-- 
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/431#discussion_r186476480

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> -float minHddSize = volume.getSize();
-if (volume.isBootDevice()) {
-   SingleServerAppliance appliance = 
api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  Hardware hardwareModel = 
generateHardwareRequest.isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);
+  ServerAppliance workingImage = null;

removed

-- 
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/431#discussion_r186476536

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



>  
@Inject
OneandoneComputeServiceAdapter(OneAndOneApi api, CleanupResources 
cleanupResources,
+   GenerateHardwareRequest generateHardwareRequest,
@Named(POLL_PREDICATE_SERVER) Predicate 
waitServerUntilAvailable,
PasswordGenerator.Config passwordGenerator) {

It is still in the constructor as i need to use 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/431#discussion_r186475898

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> }
 
@Override
public NodeAndInitialCredentials 
createNodeWithGroupEncodedIntoName(String group, String name, Template 
template) {
 
-  final String dataCenterId = template.getLocation().getId();
+  final String dataCenterId ="908DC2072407C94C8054610AD5A53B8C";// 
template.getLocation().getId();

Mistake, fixed.

> }
 
@Override
public NodeAndInitialCredentials 
createNodeWithGroupEncodedIntoName(String group, String name, Template 
template) {
 
-  final String dataCenterId = template.getLocation().getId();
+  final String dataCenterId ="908DC2072407C94C8054610AD5A53B8C";// 
template.getLocation().getId();

Mistake, fixed.

-- 
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/431#discussion_r186474466

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 1 commit.

8a97c6e  removed id


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/f69b3d2ac50234c0b71e7fd8e3357d4c83440659..8a97c6e2bc03de788bc9ea5bed4fa5d3d428e51d

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> -   SingleServerAppliance appliance = 
> api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  Hardware hardwareModel = 
generateHardwareRequest.isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);
+  ServerAppliance workingImage = null;
+
+  //choose the correct image based on the server type baremetal or cloud

Some of the images are of Type ISO and type Image, the ones that are of type 
ISO are basically an installation of the Linux system and not needed here, this 
function makes sure to choose the correct boot-able image based on the image 
from the template at hand

-- 
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/431#discussion_r186468957

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ignasi Barrera]

>I am not sure why is it trying to ssh into the server with loginUser=web, 
>there is not such user at the created server and it is not going to be able to 
>ssh, any ideas?

There is a test that installs a Jetty Server and creates an admin ("web") user 
for that service to test end-to-end a typical use case. It is using that "web" 
user to try access your node. If the login fails, it may be due to the previous 
script failing or because the user could not be properly configured in the OS. 
You could check that.
Anyway, let's first address Andrea's comments and once everything is clean we 
can investigate this 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-labs/pull/431#issuecomment-380002653

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

andreaturli requested changes on this pull request.

Hi @alibazlamit thanks for the effort!

There are still some comments unaddressed but more importantly looks like bare 
metal and VM have a too different work path so worth considering 2 different 
jclouds APIs, one for 1&1 VM and one for 1&1 bare metal. Code path would be 
easier to follow maybe?

This seems an idea worth investigating especially if image formats for VMs and 
for bare metals have a completely different syntax/semantic (ie: imageId 
structurally different and so on)

--- 
Finally the liveTests results are extremely important so we need to figure out 
where this `web` user is coming from and fix it

---

Thanks for your help!

> }
 
@Override
public NodeAndInitialCredentials 
createNodeWithGroupEncodedIntoName(String group, String name, Template 
template) {
 
-  final String dataCenterId = template.getLocation().getId();
+  final String dataCenterId ="908DC2072407C94C8054610AD5A53B8C";// 
template.getLocation().getId();

why this hardcoded value?

>  
@Inject
OneandoneComputeServiceAdapter(OneAndOneApi api, CleanupResources 
cleanupResources,
+   GenerateHardwareRequest generateHardwareRequest,
@Named(POLL_PREDICATE_SERVER) Predicate 
waitServerUntilAvailable,
PasswordGenerator.Config passwordGenerator) {

as you have removed it from the constructor, why do you still need to inject it?

> }
 
@Override
public NodeAndInitialCredentials 
createNodeWithGroupEncodedIntoName(String group, String name, Template 
template) {
 
-  final String dataCenterId = template.getLocation().getId();
+  final String dataCenterId ="908DC2072407C94C8054610AD5A53B8C";// 
template.getLocation().getId();
   Hardware hardware = template.getHardware();
   TemplateOptions options = template.getOptions();
   Server updateServer = null;

null not needed, maybe remove it?

> -float minHddSize = volume.getSize();
-if (volume.isBootDevice()) {
-   SingleServerAppliance appliance = 
api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  Hardware hardwareModel = 
generateHardwareRequest.isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);
+  ServerAppliance workingImage = null;

null not needed, maybe remove it?

> - ram = ram / 1024;
+  //configuring Firewall rules
+  Map portsRange = getPortRangesFromList(inboundPorts);
+  List rules = new 
ArrayList();
+
+  for (Map.Entry range : portsRange.entrySet()) {
+ FirewallPolicy.Rule.CreatePayload rule = 
FirewallPolicy.Rule.CreatePayload.builder()
+ .portFrom(range.getKey())
+ .portTo(range.getValue())
+ .protocol(Types.RuleProtocol.TCP)
+ .build();
+ rules.add(rule);
+  }
+  String firewallPolicyId = "";
+  if (inboundPorts.length > 0) {
+ FirewallPolicy firewallPolicy = 
api.firewallPolicyApi().create(FirewallPolicy.CreateFirewallPolicy.create(name 
+ " firewall policy", "desc", rules));

what happens when you ask to create multiple nodes? does `firewallPolicyApi` 
handles the concurrency correctly?

jclouds usually takes care of fw rules for a group of node extending 
`CreateNodesWithGroupEncodedIntoNameThenAddToSet` -- see 
`ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet` in 
openstack-nova api for an example

> -   SingleServerAppliance appliance = 
> api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  Hardware hardwareModel = 
generateHardwareRequest.isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);
+  ServerAppliance workingImage = null;
+
+  //choose the correct image based on the server type baremetal or cloud

sorry can't understand this.

Can you elaborate a bit more the rationale of this `findWorkingImage` method?

>   }
   }
 
-  // provision server
-  Server server = null;
-  Double cores = 

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli Thanks for the review, i have applied most of the changes you 
requested.
I wanted to run the live tests, but four of them fail on when trying to ssh 
into the server, for example `testCreateAndRunAService` fails with this error 
```
(web:rsa[fingerprint(37:18:5e:f4:ce:d4:95:c4:02:10:8c:01:b9:b9:d3:4b),sha1(c0:81:47:7b:a1:88:47:2d:e0:1c:e4:6a:28:d6:fc:ab:c3:6b:44:16)]@50.21.186.61:22)
 
(web:rsa[fingerprint(37:18:5e:f4:ce:d4:95:c4:02:10:8c:01:b9:b9:d3:4b),sha1(c0:81:47:7b:a1:88:47:2d:e0:1c:e4:6a:28:d6:fc:ab:c3:6b:44:16)]@50.21.186.61:22)
 error acquiring {hostAndPort=50.21.186.61:22, loginUser=web, ssh=null, 
connectTimeout=30, sessionTimeout=30} (out of retries - max 7): 
Exhausted available authentication methods
org.jclouds.rest.AuthorizationException
at org.jclouds.sshj.SshjSshClient.propagate(SshjSshClient.java:394)
```

I am not sure why is it trying to ssh into the server with loginUser=web, there 
is not such user at the created server and it is not going to be able to ssh, 
any ideas?

-- 
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/431#issuecomment-377948259

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@alibazlamit pushed 1 commit.

f69b3d2  Update code for review


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/431/files/3ebda7edf3007ba25dfa23c3cf926cbdc2573b71..f69b3d2ac50234c0b71e7fd8e3357d4c83440659

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> -//check if the bootable device has enough size to run the 
> appliance(image).
-float minHddSize = volume.getSize();
-if (volume.isBootDevice()) {
-   SingleServerAppliance appliance = 
api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  HardwareFlavour hardwareModel = isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);

My intention was to allow the users to just pass in the flavor name i.e. 
`BMC_XL` or `XL` as a `hardwareId` to choose between baremetal and cloud 
servers.

Maybe the `OneandoneTemplateOptions` would be an overkill since the only thing 
users will need to provide is the different hardware flavor name which can be 
achieved using the `public TemplateBuilder hardwareId(String string);` from the 
`TeamplateBuilder` class

-- 
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/431#discussion_r178092281

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @@ -53,7 +53,7 @@ public Hardware apply(HardwareFlavour from) {
   }
   final HardwareBuilder builder;
   builder = new HardwareBuilder()
-  .ids(from.id())
+  .ids(from.name())

My first approach was what you have suggested, for some reason the API has 
similiar Id's between the baremetal flavors and the cloud flavors, the only 
thing to make them unique is the name so far, this might be an issue on the API 
side that might get changed in the future, if that ever happens we can go with 
what you suggested

-- 
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/431#discussion_r178075870

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

alibazlamit commented on this pull request.



> @Override
public List listHardwareProfiles() {
-  return api.serverApi().listHardwareFlavours();
+  List result = new ArrayList();
+  List cloudModels = 
api.serverApi().listHardwareFlavours();
+  for (HardwareFlavour flavor : cloudModels) {
+ result.add(flavor);
+  }
+  List baremetalModels = 
api.serverApi().listBaremetalModels();
+  for (BareMetalModel model : baremetalModels) {

I need to combine `BareMetalModel` with `HardwareFlavour` as one list, to mix 
the baremetal flavors with the cloud flavors, do you think that could be 
achieved using the function you suggested? 

-- 
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/431#discussion_r178074709

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

andreaturli requested changes on this pull request.

great start @alibazlamit! I've only some comments, feel free to ask for 
clarification!

As I don't have access to oneandone it would be nice if you could attach the 
results of live tests before merging this. Thanks

> @@ -65,6 +66,7 @@
private final OneAndOneApi api;
private final Predicate waitServerUntilAvailable;
private final PasswordGenerator.Config passwordGenerator;

it seems not used

> -//check if the bootable device has enough size to run the 
> appliance(image).
-float minHddSize = volume.getSize();
-if (volume.isBootDevice()) {
-   SingleServerAppliance appliance = 
api.serverApplianceApi().get(image.getId());
-   if (appliance.minHddSize() > volume.getSize()) {
-  minHddSize = appliance.minHddSize();
-   }
-}
-Hdd.CreateHdd hdd = Hdd.CreateHdd.create(minHddSize, 
volume.isBootDevice());
-hdds.add(hdd);
- } catch (Exception ex) {
-throw Throwables.propagate(ex);
-
+  String imageId = image.getId();
+  HardwareFlavour hardwareModel = isFlavor(hardware.getId());
+  boolean isBaremetal = hardware.getName() == null ? false : 
hardware.getName().contains(baremetalModelsKey);

I think worth introducing a `OneandoneTemplateOptions extends TemplateOptions` 
to deal with specific requirements a user may specify like bare metal or even 
`hardwareModel` above. wdyt?

>try {
- org.apache.jclouds.oneandone.rest.domain.Hardware.CreateHardware 
hardwareRequest
- = 
org.apache.jclouds.oneandone.rest.domain.Hardware.CreateHardware.create(cores, 
1, ram, hdds);
+ if (hardwareModel != null) {

this logic to build the hardwareRequest is quite complex. would it make sense 
to extract it in its own method, and add some unit tests, ideally

> @Override
public List listHardwareProfiles() {
-  return api.serverApi().listHardwareFlavours();
+  List result = new ArrayList();
+  List cloudModels = 
api.serverApi().listHardwareFlavours();
+  for (HardwareFlavour flavor : cloudModels) {
+ result.add(flavor);
+  }
+  List baremetalModels = 
api.serverApi().listBaremetalModels();
+  for (BareMetalModel model : baremetalModels) {

maybe you can consider a `Function` for better 
readability and testability?

> @@ -125,4 +126,28 @@ public boolean apply(Server server) {
  || server.status().percent() != 0);
   }
}
+
+   @Provides
+   @Singleton
+   @Named(POLL_PREDICATE_SERVER_SUSPENDED)

why a not using `ComputeServiceProperties.TIMEOUT_NODE_SUSPENDED` so that you 
can use a quite adopted pattern like in openstack,digitalocean,packet
```
   @Provides
   @com.google.inject.name.Named(TIMEOUT_NODE_TERMINATED)
   protected Predicate provideServerTerminatedPredicate(final 
NovaApi api, ComputeServiceConstants.Timeouts timeouts,

ComputeServiceConstants.PollPeriod pollPeriod) {
  return retry(new ServerTerminatedPredicate(api), timeouts.nodeTerminated, 
pollPeriod.pollInitialPeriod,
  pollPeriod.pollMaxPeriod);
   }
```
?

> @@ -53,7 +53,7 @@ public Hardware apply(HardwareFlavour from) {
   }
   final HardwareBuilder builder;
   builder = new HardwareBuilder()
-  .ids(from.id())
+  .ids(from.name())

maybe better to use 
```
.id(from.id())
.name(from.name())
```
if `.ids()` is not what you want here

> @@ -19,6 +19,7 @@
 public final class OneAndOneProperties {
 
public static final String POLL_PREDICATE_SERVER = 
"jclouds.oneandone.rest.predicate.server";
+   public static final String POLL_PREDICATE_SERVER_SUSPENDED = 
"jclouds.oneandone.rest.predicate.serverdelete";

oh now I see, it's a predicate to wait for server deletion not really 
suspension. ok maybe my comment above is still valid for other predicates though

> +   public static BareMetalModel create(String id, String name, Hardware 
> hardware) {
+  return new AutoValue_BareMetalModel(id, name, hardware);
+   }
+
+   @AutoValue
+   public abstract static class Hardware {
+
+  public abstract double core();
+
+  public abstract double coresPerProcessor();
+
+  public abstract double ram();
+
+  public abstract String unit();
+
+  public abstract Hdd hdds();

out of curiosity, why it's `hdds` if it is not a collection?

-- 
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/431#pullrequestreview-105791803

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Andrea Turli]

Thanks @alibazlamit, we'll have asap. 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-labs/pull/431#issuecomment-374544641

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

@andreaturli a friendly reminder, this is a review ready PR

-- 
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/431#issuecomment-374534274

Re: [jclouds/jclouds-labs] JCLOUDS-1386 1&1 Baremetal servers (#431)

[Ali Bazlamit]

I have implemented the baremetal creation in the compute service adapter, based 
on the hardware flavor chosen we can determine the server type.

Here is the [JIRA ](https://issues.apache.org/jira/browse/JCLOUDS-1386)ticket.

-- 
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/431#issuecomment-366979292