Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

Closed #421.

-- 
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/421#event-1372266168

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Ignasi Barrera]

Merged to 
[master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/2b8bfcb8) 
and 
[2.0.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/8c1449bd). 
Thanks @trevorflanagan!

-- 
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/421#issuecomment-349232066

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Ignasi Barrera]

nacx approved this pull request.

Thanks, @trevorflanagan! Mind squashing the commits into a single one for a 
clean merge?



-- 
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/421#pullrequestreview-80498531

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

rebuild please

-- 
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/421#issuecomment-348116402

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

@trevorflanagan pushed 1 commit.

58366dd  Correcting the types used in ComputeServiceAdapterContextModule.


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/421/files/c3943adffaec4e251138fdcc26a834c2b322d114..58366dd0d49b5b4251b9a63e8c971c5787ca9ce0

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

@nacx I have refactored OsImageToImage in order to handle OsImage and 
CustomerImage. This means that JClouds can expose all of our image types. I 
have also improved the version and os family handling to be more robust.

-- 
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/421#issuecomment-347946487

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

@trevorflanagan pushed 1 commit.

c3943ad  Refactor OsImageToImage to instead use BaseImage. This allows for both 
OS Images and Customer Images to be used. Improved the version and os family 
logic to use the guest operating system information.


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/421/files/501b7ea52ebcf798855374da609585c7c12e4ac8..c3943adffaec4e251138fdcc26a834c2b322d114

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Ignasi Barrera]

Sure! :smile: 

-- 
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/421#issuecomment-347880602

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

Hi @nacx I would like to make some further changes for the PR. Since you are 
busy with the release I think it's safe to do so.

-- 
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/421#issuecomment-347878857

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Ignasi Barrera]

nacx commented on this pull request.



> +   OsImageToImage(@Memoized final Supplier locations,
+ Function imageDescriptionToOsFamily) {
+  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())

Perfect! Thanks for clarifying. Then, mind just changing the code to 
`ImageBuilder().ids(input.id())` (note the plural there)? This will set both 
the id and providerId fields.

-- 
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/421#discussion_r153420444

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

trevorflanagan commented on this pull request.



> +   OsImageToImage(@Memoized final Supplier locations,
+ Function imageDescriptionToOsFamily) {
+  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())

We use UUID for generating ID values and are satisfied that although a clash 
between regions is possible, the chances are very small.
Also in our API a getImages call will return images from a single region only 
and will contain images across all zones in the region.

So to answer your questions a getImage call with the ID will be enough to 
uniquely identify that Image within the region.

-- 
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/421#discussion_r153419189

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Ignasi Barrera]

nacx commented on this pull request.



> +   OsImageToImage(@Memoized final Supplier locations,
+ Function imageDescriptionToOsFamily) {
+  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())

I mean, you should think about it this way: calling 
`ComputeService.getImage(id)` with this ID will succeed? Or should the ID 
carry/encode additional info?

-- 
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/421#discussion_r153417324

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Ignasi Barrera]

nacx commented on this pull request.



> +   OsImageToImage(@Memoized final Supplier locations,
+ Function imageDescriptionToOsFamily) {
+  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())

Is it possible that that unique ID within that region collisions with a unique 
id within another region?

-- 
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/421#discussion_r153417063

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

trevorflanagan commented on this pull request.



> +  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())
+.status(Image.Status.AVAILABLE).operatingSystem(os).location(
+  
FluentIterable.from(locations.get()).firstMatch(LocationPredicates.idEquals(input.datacenterId()))

Image is only available in certain locations, so what I have here is valid.

-- 
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/421#discussion_r153416969

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

trevorflanagan commented on this pull request.



> +   OsImageToImage(@Memoized final Supplier locations,
+ Function imageDescriptionToOsFamily) {
+  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())

@nacx thanks for the background information. In DimensionData each image is 
associated with a single datacenter (zone) and will the ID will be unique 
within a Region. Therefore I will keep the `input.id()` as is.

-- 
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/421#discussion_r153416810

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Trevor Flanagan]

@nacx thanks for taking a look. I will make the required changes shortly.

-- 
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/421#issuecomment-347442442

Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)

[Ignasi Barrera]

nacx commented on this pull request.

Thanks for a small but important PR @trevorflanagan!

> +   private static final String REDHAT = "RedHat";
+   private static final String UBUNTU = "Ubuntu";
+   private static final String SUSE = "SuSE";
+   private static final String WINDOWS = "Win";
+
+   @Override
+   public OsFamily apply(final String description) {
+  if (description != null) {
+ if (description.contains(CENTOS))
+return OsFamily.CENTOS;
+ else if (description.contains(UBUNTU))
+return OsFamily.UBUNTU;
+ else if (description.contains(REDHAT))
+return OsFamily.RHEL;
+ else if (description.contains(SUSE))
+return OsFamily.RHEL;

Shouldn't it be `OsFamily.SUSE`?

> +   OsImageToImage(@Memoized final Supplier locations,
+ Function imageDescriptionToOsFamily) {
+  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())

When configuring the image IDs, think about the "id" and the "providerId" field 
this way:

* **providerId** is the *real id* of the image in the provider (I guess 
`input.id()`), so users can retrieve the image using the provider-specific APIs.
* **id** is the *portable id jclouds will use to uniquelly identify the image*. 
This is slightly different from the previous one. For example, in some 
providers, images in different regions may have the same id (centos-6, 
ami-0001, etc), but jcouds needs to provide unique IDs for each, so we encode 
the region in the id (something like: "region/id").

I don't know if it is the case in DimensionData, but please consider this when 
configuring the id and providerId fields. If the ID is unique, then just assign 
the same value to both fields :)

> +  this.locations = locations;
+  this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+   }
+
+   @Override
+   public Image apply(OsImage input) {
+
+  OsFamily osFamily = 
imageDescriptionToOsFamily.apply(input.description());
+  String osVersion = parseVersion(input.description());
+
+  OperatingSystem os = 
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+  return new 
ImageBuilder().id(input.id()).name(input.name()).description(input.description())
+.status(Image.Status.AVAILABLE).operatingSystem(os).location(
+  
FluentIterable.from(locations.get()).firstMatch(LocationPredicates.idEquals(input.datacenterId()))

Just one consideration about the meaning of the `location` field in an image:

* If the image is globally available in all locations (the same image, same id, 
etc), then the location field should be `null`.
* if the image is only available in certain locations, then the location field 
must be set accordingly.

> +
+   @BeforeMethod
+   public void setUp() throws Exception {
+  imageDescriptionToOsFamily = new ImageDescriptionToOsFamily();
+   }
+
+   @Test
+   public void apply_Centos() {
+  String description = "CentOS 6.0 (Final)";
+  assertEquals(OsFamily.CENTOS, 
imageDescriptionToOsFamily.apply(description));
+   }
+
+   @Test
+   public void apply_Suse() {
+  String description = "SuSE Linux Enterprise Server 11 SP4";
+  assertEquals(OsFamily.RHEL, 
imageDescriptionToOsFamily.apply(description));

`OsFamily.SUSE`?

> + *
+ * 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 specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.dimensiondata.cloudcontrol.compute.functions;
+
+import org.jclouds.compute.domain.OsFamily;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import static org.testng.AssertJUnit.assertEquals;
+
+public class ImageDescriptionToOsFamilyTest {

Configure the test in the "unit" group.

-- 
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/421#pullrequestreview-79411168