You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by swaqos <no...@github.com> on 2017/11/17 09:35:33 UTC
[jclouds/jclouds] This is the first commit of Softlayer Network API
(#1158)
Jira link: https://issues.apache.org/jira/browse/JCLOUDS-1357
JCLOUDS-1357
The update focuses on adding SoftLayer Network API to the features list of SoftLayer provider.
Domain type files:
- Added Network.java
- Added Subnet.java
Features:
- Added NetworkApi.java
- Added NetworkApiLiveTest.java
- Added NetworkApiMockTest.java
- Added resource files for NetworkApiMockTest.java
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/1158
-- Commit Summary --
* This is the first commit of for SoftLayer Network API set
* minor code improvement
-- File Changes --
M providers/softlayer/src/main/java/org/jclouds/softlayer/SoftLayerApi.java (8)
A providers/softlayer/src/main/java/org/jclouds/softlayer/domain/Network.java (163)
A providers/softlayer/src/main/java/org/jclouds/softlayer/domain/Subnet.java (166)
A providers/softlayer/src/main/java/org/jclouds/softlayer/features/NetworkApi.java (175)
A providers/softlayer/src/test/java/org/jclouds/softlayer/features/NetworkApiLiveTest.java (207)
A providers/softlayer/src/test/java/org/jclouds/softlayer/features/NetworkApiMockTest.java (230)
A providers/softlayer/src/test/resources/network_createObject.json (7)
A providers/softlayer/src/test/resources/network_createSubnet.json (18)
A providers/softlayer/src/test/resources/network_get_153001.json (8)
A providers/softlayer/src/test/resources/network_list.json (41)
A providers/softlayer/src/test/resources/network_subnet_get_1592631.json (26)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/1158.patch
https://github.com/jclouds/jclouds/pull/1158.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/pull/1158
Re: [jclouds/jclouds] [JCLOUDS-1357] This is the first commit of
Softlayer Network API (#1158)
Posted by Andrea Turli <no...@github.com>.
andreaturli approved this pull 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/pull/1158#pullrequestreview-78443503
Re: [jclouds/jclouds] [JCLOUDS-1357] This is the first commit of
Softlayer Network API (#1158)
Posted by swaqos <no...@github.com>.
swaqos commented on this pull request.
> +import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import java.util.List;
+import java.util.ArrayList;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.assertNull;
+
+/**
+ * Tests behavior of {@code NetworkApi}
+ */
+@Test(groups = "live")
+public class NetworkApiLiveTest extends BaseSoftLayerApiLiveTest {
Hi Andrea, I believe that only certain type of accounts have access to the new SL+ features.
The following is a mvn test report results:
```
-------------------------------------------------------
T E S T S
-------------------------------------------------------
Java HotSpot(TM) 64-Bit Server VM warning: ignoring option MaxPermSize=256m; support was removed in 8.0
Running org.jclouds.softlayer.features.NetworkApiLiveTest
Configuring TestNG with: TestNG652Configurator
Starting test testCreateNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testCreateNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 1710ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testCreateSubnet(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testCreateSubnet(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 3401ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test testGetName(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testGetName(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 1206ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
Starting test testGetNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testGetNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 1325ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
Starting test testGetNotes(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testGetNotes(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 1147ms
Test suite progress: tests succeeded: 5, failed: 0, skipped: 0.
Starting test testListNetworks(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testListNetworks(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 7443ms
Test suite progress: tests succeeded: 6, failed: 0, skipped: 0.
Starting test testEditNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testEditNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 2570ms
Test suite progress: tests succeeded: 7, failed: 0, skipped: 0.
Starting test testGetSubnets(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testGetSubnets(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 1108ms
Test suite progress: tests succeeded: 8, failed: 0, skipped: 0.
Starting test testDeleteSubnet(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testDeleteSubnet(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 2658ms
Test suite progress: tests succeeded: 9, failed: 0, skipped: 0.
Starting test testDeleteNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest)
[TestNG] Test testDeleteNetwork(org.jclouds.softlayer.features.NetworkApiLiveTest) succeeded: 11568ms
Test suite progress: tests succeeded: 10, failed: 0, skipped: 0.
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 53.068 sec - in org.jclouds.softlayer.features.NetworkApiLiveTest
Results :
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:05 min
[INFO] Finished at: 2017-11-21T18:07:17+08:00
[INFO] Final Memory: 27M/330M
[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/pull/1158#discussion_r152228511
Re: [jclouds/jclouds] This is the first commit of Softlayer Network
API (#1158)
Posted by Andrea Turli <no...@github.com>.
hi @swaqos thanks for your contribution!
Looks like the failures are related to this PR: some checkstyle violations -- see https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/2778/org.apache.jclouds.provider$softlayer/console for more details
Please have a look at `Coding standard` notes at https://cwiki.apache.org/confluence/display/JCLOUDS/Coding+Standards on how to fix those kind problems.
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/pull/1158#issuecomment-345198949
Re: [jclouds/jclouds] [JCLOUDS-1357] This is the first commit of
Softlayer Network API (#1158)
Posted by Andrea Turli <no...@github.com>.
thanks @swaqos for the great work!
One last thing before merging, can you please make sure your branch is rebased on master and could you please squash your 4 commits into 1 -- fyi `git rebase -i HEAD~4` should be a good starting point. 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/pull/1158#issuecomment-346286631
Re: [jclouds/jclouds] This is the first commit of Softlayer Network
API (#1158)
Posted by Andrea Turli <no...@github.com>.
andreaturli requested changes on this pull request.
Excellent work, @swaqos! great AutoValue classes and good start with tests, mock and live.
Apart some very minor comments , I think you need to improve the `NetworkApiMockTest` to test also bad cases. Thanks for the great effort!
> @@ -56,4 +57,11 @@
*/
@Delegate
AccountApi getAccountApi();
+
[minor] remove extra un-needed line
> + * 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.softlayer.domain;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import org.jclouds.javax.annotation.Nullable;
+import org.jclouds.json.SerializedNames;
+
+import java.util.List;
+
+// packets for auto value
[minor] remove this comment, as it doesn't seem relevant ;)
> +
+@AutoValue
+public abstract class Network {
+
+ public abstract long accountId();
+ public abstract long id();
+ public abstract int cidr();
+ public abstract String networkIdentifier();
+ @Nullable
+ public abstract String name();
+ @Nullable
+ public abstract String notes();
+ @Nullable
+ public abstract List<Subnet> subnets();
+
+
[minor] remove extra empty line
> + *
+ * 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.softlayer.domain;
+
+import com.google.auto.value.AutoValue;
+import org.jclouds.javax.annotation.Nullable;
+import org.jclouds.json.SerializedNames;
+
+import java.util.Date;
+
+// packets for auto value
[minor] remove this comment
> +
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.assertNull;
+
+/**
+ * Tests behavior of {@code NetworkApi}
+ */
+@Test(groups = "live")
+public class NetworkApiLiveTest extends BaseSoftLayerApiLiveTest {
+
+ private NetworkApi networkApi;
+
+ private Network network = null;
+ private List<Network> networks = null;
doesn't seem to be used, remove?
> +import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.DELETE;
+
+import javax.ws.rs.core.MediaType;
+import java.io.Closeable;
+import java.util.List;
+
+/**
+ * Provides access to Network via their REST API.
+ * <p/>
+ * @see <a href="http://sldn.softlayer.com/article/REST" />
+ */
+@RequestFilters(BasicAuthentication.class)
+@Path("/v3")
I think we should use `@Path("/v{jclouds.api-version}")`
we may need to bump the `api-version` in `org.jclouds.softlayer.SoftLayerApiMetadata` -- in particular from `3` to `3.1` in https://github.com/jclouds/jclouds/blob/master/providers/softlayer/src/main/java/org/jclouds/softlayer/SoftLayerApiMetadata.java#L65, I think
> +
+ private Network network = null;
+ private List<Network> networks = null;
+ private Subnet subnet = null;
+
+ private Datacenter datacenter = null;
+
+ @BeforeClass(groups = {"integration", "live"})
+ @Override
+ public void setup() {
+ super.setup();
+
+ networkApi = api.getNetworkApi();
+
+ for (Datacenter dc : api.getDatacenterApi().listDatacenters()) {
+ if (dc.getName().equals("dal13")) {
why `dal013`? is the only datacenter that supports this network feature so far?
maybe consider making it a constant
> + }
+
+ assertTrue(found, "List Networks didn't return created network.");
+ networks = response;
+ }
+
+
+ @Test(dependsOnMethods = "testCreateNetwork")
+ public void testGetNetwork() throws Exception {
+ Network found = networkApi.getNetwork(network.id());
+ assertEquals(found, network);
+ checkNetwork(found);
+ assertEquals(found.networkIdentifier(), "192.168.253.0");
+ assertEquals(found.name(), "NetworkApiLiveTestNetwork");
+ assertEquals(found.cidr(), 24);
+ assertEquals(found.notes(), "this is a test network from JClouds NetworkApiLiveTest.");
[minor] it's `jclouds`
> +import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import java.util.List;
+import java.util.ArrayList;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.assertNull;
+
+/**
+ * Tests behavior of {@code NetworkApi}
+ */
+@Test(groups = "live")
+public class NetworkApiLiveTest extends BaseSoftLayerApiLiveTest {
Unfortunately my account is not allowed to create networks so I couldn't run the `NetworkApiLiveTest` -- `org.jclouds.http.HttpResponseException: command: POST https://api.softlayer.com/rest/v3.1/SoftLayer_Network/createObject HTTP/1.1 failed with response: HTTP/1.1 500 Internal Server Error; content: [{"error":"Cloudsoft Corporation (#xxxxx) is not allowed to create networks.","code":"SoftLayer_Exception_Account_Limit"}]` so could you maybe paste here the results from your account, maybe? thanks!
> +import org.jclouds.softlayer.domain.Network;
+import org.jclouds.softlayer.domain.Subnet;
+import org.jclouds.softlayer.internal.BaseSoftLayerMockTest;
+import org.testng.annotations.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static com.google.common.collect.Iterables.size;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+/**
+ * Mock tests for the {@link NetworkApi} class.
+ */
+@Test(groups = "unit", testName = "SecurityGroupApiMockTest")
+public class NetworkApiMockTest extends BaseSoftLayerMockTest {
great start @swaqos!
Similarly to other provider, jclouds also tests fallbacks strategies when the operations are not successful.
For example, `listNetworks` is annotated with `@Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)` so a test similar to https://github.com/jclouds/jclouds/blob/master/providers/packet/src/test/java/org/jclouds/packet/features/FacilityApiMockTest.java#L45 is required.
Please add the missing "bad case" tests for the operations in this MockTest class to improve the coverage
--
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/pull/1158#pullrequestreview-77610657
Re: [jclouds/jclouds] This is the first commit of Softlayer Network
API (#1158)
Posted by Andrea Turli <no...@github.com>.
Thanks @swaqos I'll have a proper review shortly! 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/pull/1158#issuecomment-345257284
Re: [jclouds/jclouds] [JCLOUDS-1357] This is the first commit of
Softlayer Network API (#1158)
Posted by Andrea Turli <no...@github.com>.
Merged at [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/a540daf3)
Thanks again @swaqos -- looking forward to seeing more of your work!
--
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/pull/1158#issuecomment-346351227
Re: [jclouds/jclouds] This is the first commit of Softlayer Network
API (#1158)
Posted by Andrea Turli <no...@github.com>.
@swaqos if you name the PR with the jira issue, JCLOUDS-1357 it will be automatically linked to the jira issue.
--
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/pull/1158#issuecomment-345428393
Re: [jclouds/jclouds] This is the first commit of Softlayer Network
API (#1158)
Posted by swaqos <no...@github.com>.
@swaqos pushed 1 commit.
96e3c0c Syntax fix with coding standards
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1158/files/d77041387df5a3837f0f68e9f7fd0a273f8770ea..96e3c0c6895845a91e0683f45c8ba2ac1b0862b9
Re: [jclouds/jclouds] [JCLOUDS-1357] This is the first commit of
Softlayer Network API (#1158)
Posted by swaqos <no...@github.com>.
@swaqos pushed 1 commit.
9a49e0d - added more test coverage to NetworkApiMockTest class
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1158/files/96e3c0c6895845a91e0683f45c8ba2ac1b0862b9..9a49e0de4000fc7d294f794333efce2cabc4645a
Re: [jclouds/jclouds] [JCLOUDS-1357] This is the first commit of
Softlayer Network API (#1158)
Posted by Andrea Turli <no...@github.com>.
Closed #1158.
--
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/pull/1158#event-1354070232
Re: [jclouds/jclouds] This is the first commit of Softlayer Network
API (#1158)
Posted by swaqos <no...@github.com>.
Hi @andreaturli , thanks for the information. I updated the code and the build passed.
--
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/pull/1158#issuecomment-345225778