You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Trevor Flanagan <no...@github.com> on 2017/11/27 16:42:16 UTC
[jclouds/jclouds-labs] Implement OsImageToImage Function (#421)
This PR shows the approach we will use for the next steps involved in creating the JCLouds abstraction now that the feature APIs are complete.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/421
-- Commit Summary --
* Implement OsImageToImage Function
-- File Changes --
A dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/compute/functions/ImageDescriptionToOsFamily.java (49)
A dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/compute/functions/OsImageToImage.java (87)
A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/compute/functions/ImageDescriptionToOsFamilyTest.java (70)
A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/compute/functions/OsImageToImageTest.java (117)
A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/config/DimensionDataCloudControlComputeServiceContextModule.java (44)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/421.patch
https://github.com/jclouds/jclouds-labs/pull/421.diff
--
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
Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)
Posted by Ignasi Barrera <no...@github.com>.
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)
Posted by Ignasi Barrera <no...@github.com>.
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)
Posted by Trevor Flanagan <no...@github.com>.
@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)
Posted by Trevor Flanagan <no...@github.com>.
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)
Posted by Trevor Flanagan <no...@github.com>.
trevorflanagan commented on this pull request.
> + OsImageToImage(@Memoized final Supplier<Set<Location>> locations,
+ Function<String, OsFamily> 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)
Posted by Trevor Flanagan <no...@github.com>.
@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)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> + OsImageToImage(@Memoized final Supplier<Set<Location>> locations,
+ Function<String, OsFamily> 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)
Posted by Trevor Flanagan <no...@github.com>.
trevorflanagan commented on this pull request.
> + OsImageToImage(@Memoized final Supplier<Set<Location>> locations,
+ Function<String, OsFamily> 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)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> + OsImageToImage(@Memoized final Supplier<Set<Location>> locations,
+ Function<String, OsFamily> 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)
Posted by Trevor Flanagan <no...@github.com>.
@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)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> + OsImageToImage(@Memoized final Supplier<Set<Location>> locations,
+ Function<String, OsFamily> 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)
Posted by Trevor Flanagan <no...@github.com>.
@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)
Posted by Trevor Flanagan <no...@github.com>.
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)
Posted by Ignasi Barrera <no...@github.com>.
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)
Posted by Ignasi Barrera <no...@github.com>.
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<Set<Location>> locations,
+ Function<String, OsFamily> 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
Re: [jclouds/jclouds-labs] Implement OsImageToImage Function (#421)
Posted by Trevor Flanagan <no...@github.com>.
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)
Posted by Trevor Flanagan <no...@github.com>.
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