Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
Pushed to [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/efb2a3de) and [2.1.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/87ff3afe). Thanks @btrishkin! -- 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/428#issuecomment-381869359
Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
Closed #428. -- 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/428#event-1578004706
Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
@nacx small issue fixed and commits squashed. -- 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/428#issuecomment-381082477
Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
nacx commented on this pull request.
Thanks @btrishkin! Just one minor comment left. Apart from that, LGTM!
> + */
+package org.jclouds.dimensiondata.cloudcontrol.compute.functions;
+
+import com.google.common.base.Function;
+import com.google.inject.Singleton;
+import org.jclouds.compute.domain.OperatingSystem;
+
+import javax.annotation.Nullable;
+
+@Singleton
+public class OperatingSystemToOperatingSystem
+ implements
Function {
+
+ private final OperatingSystemToOsFamily operatingSystemToOsFamily;
+
+ public OperatingSystemToOperatingSystem(final OperatingSystemToOsFamily
operatingSystemToOsFamily) {
Add the `@Inject` annotation here and remove the public modifier from the
constructor.
> + */
+package org.jclouds.dimensiondata.cloudcontrol.compute.functions;
+
+import com.google.common.base.Function;
+import com.google.inject.Singleton;
+import org.jclouds.compute.domain.OperatingSystem;
+
+import javax.annotation.Nullable;
+
+@Singleton
+public class OperatingSystemToOperatingSystem
+ implements
Function {
+
+ private final OperatingSystemToOsFamily operatingSystemToOsFamily;
+
+ public OperatingSystemToOperatingSystem(final OperatingSystemToOsFamily
operatingSystemToOsFamily) {
Add the `@Inject` annotation and remove the public modifier from the
constructor to make it package private
([background](https://github.com/google/guice/wiki/KeepConstructorsHidden)).
--
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/428#pullrequestreview-111906667
Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
ServerWithNatRuleToNodeMetadata Function reimplemented without expensive image list API calls. BaseImageToImage and BaseImageToHardware are not required for this function anymore, but we have plans to use them later -- 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/428#issuecomment-380449485
Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
@btrishkin pushed 1 commit. 69cdeb3 Implement ServerWithNatRuleToNodeMetadata Function -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds-labs/pull/428/files/36ab3eb158511eb5d24bd0083068b8e7b79ef1c5..69cdeb3012acf33edaad7674eee9849a97eff1ee
Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
trevorflanagan commented on this pull request.
> + this.nodeNamingConvention = checkNotNull(namingConvention,
> "namingConvention").createWithoutPrefix();
+ this.locations = checkNotNull(locations, "locations");
+ this.baseImageToImage = checkNotNull(baseImageToImage,
"baseImageToImage");
+ this.baseImageToHardware = checkNotNull(baseImageToHardware,
"osImageToHardware");
+ this.credentialStore = checkNotNull(credentialStore, "credentialStore
cannot be null");
+ this.api = checkNotNull(api, "api cannot be null");
+ }
+
+ @Override
+ public NodeMetadata apply(final ServerWithExternalIp serverWithExternalIp) {
+ NodeMetadataBuilder builder = new NodeMetadataBuilder();
+ Server server = serverWithExternalIp.server();
+ builder.ids(server.id());
+ builder.name(server.name());
+ builder.hostname(serverWithExternalIp.server().description());
+ if (server.datacenterId() != null) {
Should not be null. Our xsd has it as a required attribute:
``
--
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/428#discussion_r158450187
Re: [jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
nacx commented on this pull request.
Thanks @btrishkin!
> + final DimensionDataCloudControlApi api) {
+ this.nodeNamingConvention = checkNotNull(namingConvention,
"namingConvention").createWithoutPrefix();
+ this.locations = checkNotNull(locations, "locations");
+ this.baseImageToImage = checkNotNull(baseImageToImage,
"baseImageToImage");
+ this.baseImageToHardware = checkNotNull(baseImageToHardware,
"osImageToHardware");
+ this.credentialStore = checkNotNull(credentialStore, "credentialStore
cannot be null");
+ this.api = checkNotNull(api, "api cannot be null");
+ }
+
+ @Override
+ public NodeMetadata apply(final ServerWithExternalIp serverWithExternalIp) {
+ NodeMetadataBuilder builder = new NodeMetadataBuilder();
+ Server server = serverWithExternalIp.server();
+ builder.ids(server.id());
+ builder.name(server.name());
+ builder.hostname(serverWithExternalIp.server().description());
Is this really the hostname? Don't put that here if not.
> + this.nodeNamingConvention = checkNotNull(namingConvention,
> "namingConvention").createWithoutPrefix();
+ this.locations = checkNotNull(locations, "locations");
+ this.baseImageToImage = checkNotNull(baseImageToImage,
"baseImageToImage");
+ this.baseImageToHardware = checkNotNull(baseImageToHardware,
"osImageToHardware");
+ this.credentialStore = checkNotNull(credentialStore, "credentialStore
cannot be null");
+ this.api = checkNotNull(api, "api cannot be null");
+ }
+
+ @Override
+ public NodeMetadata apply(final ServerWithExternalIp serverWithExternalIp) {
+ NodeMetadataBuilder builder = new NodeMetadataBuilder();
+ Server server = serverWithExternalIp.server();
+ builder.ids(server.id());
+ builder.name(server.name());
+ builder.hostname(serverWithExternalIp.server().description());
+ if (server.datacenterId() != null) {
Under which circumstances can this be null?
> + builder.imageId(image.getId());
+ builder.operatingSystem(image.getOperatingSystem());
+ }
+ if (server.state() != null) {
+ builder.status(serverStateToNodeStatus.get(server.state()));
+ }
+
+ String privateAddress = null;
+ if (server.networkInfo() != null && server.networkInfo().primaryNic() !=
null
+&& server.networkInfo().primaryNic().privateIpv4() != null) {
+ privateAddress = server.networkInfo().primaryNic().privateIpv4();
+ builder.privateAddresses(ImmutableSet.of(privateAddress));
+ }
+ if (privateAddress != null && serverWithExternalIp.externalIp() != null)
{
+
builder.publicAddresses(ImmutableSet.of(serverWithExternalIp.externalIp()));
+ }
We should consider not only the primary NIC here. Let's return all IP addresses
of the server.
> + builder.imageId(image.getId());
+ builder.operatingSystem(image.getOperatingSystem());
+ }
+ if (server.state() != null) {
+ builder.status(serverStateToNodeStatus.get(server.state()));
+ }
+
+ String privateAddress = null;
+ if (server.networkInfo() != null && server.networkInfo().primaryNic() !=
null
+&& server.networkInfo().primaryNic().privateIpv4() != null) {
+ privateAddress = server.networkInfo().primaryNic().privateIpv4();
+ builder.privateAddresses(ImmutableSet.of(privateAddress));
+ }
+ if (privateAddress != null && serverWithExternalIp.externalIp() != null)
{
+
builder.publicAddresses(ImmutableSet.of(serverWithExternalIp.externalIp()));
+ }
BTW, why read the external IP only if there is a private IP?
> + String privateAddress = null;
+ if (server.networkInfo() != null && server.networkInfo().primaryNic() !=
null
+&& server.networkInfo().primaryNic().privateIpv4() != null) {
+ privateAddress = server.networkInfo().primaryNic().privateIpv4();
+ builder.privateAddresses(ImmutableSet.of(privateAddress));
+ }
+ if (privateAddress != null && serverWithExternalIp.externalIp() != null)
{
+
builder.publicAddresses(ImmutableSet.of(serverWithExternalIp.externalIp()));
+ }
+
+ // DimensionData does not provide a way to get the credentials.
+ // Try to return them from the credential store
+ Credentials credentials = credentialStore.get("node#" + server.id());
+ if (credentials instanceof LoginCredentials) {
+ builder.credentials(LoginCredentials.class.cast(credentials));
+ }
Remove this. The credentials are [already set by
jclouds](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java#L89-L90).
> + }
+
+ // DimensionData does not provide a way to get the credentials.
+ // Try to return them from the credential store
+ Credentials credentials = credentialStore.get("node#" + server.id());
+ if (c
[jclouds/jclouds-labs] Implement ServerWithNatRuleToNodeMetadata Function (#428)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/428 -- Commit Summary -- * Implement ServerWithNatRuleToNodeMetadata Function -- File Changes -- A dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/compute/functions/ServerWithNatRuleToNodeMetadata.java (123) A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/compute/functions/ServerWithNatRuleToNodeMetadataTest.java (213) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/428.patch https://github.com/jclouds/jclouds-labs/pull/428.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/428
