You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Limor Bortman-Stotland <no...@github.com> on 2015/02/28 14:11:04 UTC
[jclouds] adding attache detach interface to nova api (#697)
adding attache detach interface to nova api
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/697
-- Commit Summary --
* adding attache detach interface to nova api
-- File Changes --
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/InterfaceAttachment.java (169)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/NovaApi.java (8)
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/FixedIp.java (94)
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/features/InterfaceApi.java (52)
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/options/AttachInterfaceOptions.java (191)
A apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/features/InterfaceApiExpectTest.java (38)
A apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/features/InterfaceApiLiveTest.java (64)
A apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/parse/ParseInterfaceAttachmentTest.java (48)
A apis/openstack-nova/src/test/resources/interface_attachment.json (14)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/697.patch
https://github.com/jclouds/jclouds/pull/697.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
Interesting. I think the test maybe expects that these networks are already created on the openstack instance. I am guessing this will fail on a pristine devstack instance, for example.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78156454
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> + InterfaceApi api = requestsSendResponses(
> + keystoneAuthWithUsernameAndPasswordAndTenantName,
> + responseWithKeystoneAccess,
> + authenticatedGET().endpoint(endpoint)
> + .method("POST")
> + .payload(payloadFromStringWithContentType(
> + "{\"interfaceAttachment\":{\"net_id\":\"1017d1c5-963b-4ae3-b40f-2e8266287249\"}}", "application/json")).
> + build(),
> + HttpResponse.builder().statusCode(200).message("HTTP/1.1 200").payload(payloadFromResource("/interface_attachment.json")).build()
> + ).getInterfaceApiForZone("az-1.region-a.geo-1").get();
> +
> + InterfaceAttachment i = api.attachInterface("a01ea7e8107f48a3b9f04e12b01771b6", AttachInterfaceOptions.Builder.netId("1017d1c5-963b-4ae3-b40f-2e8266287249"));
> + assertEquals(api.attachInterface("a01ea7e8107f48a3b9f04e12b01771b6", AttachInterfaceOptions.Builder.netId("1017d1c5-963b-4ae3-b40f-2e8266287249")).toString(),
> + new ParseInterfaceAttachmentTest().expected().toString());
> + }
> +
Add the missing test for the detach interface method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602159
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
> @RequestFilters(AuthenticateRequest.class)
> @Consumes(MediaType.APPLICATION_JSON)
> @Path("/servers")
> -public interface AttachInterfaceApi {
> +public interface InterfaceApi {
Will this be backported? It seems like a breaking change for existing code.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r28434298
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + @RequestFilters(AuthenticateRequest.class)
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId, AttachInterfaceOptions... options);
> +
> + @DELETE
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/{server_id}/os-interface/{port_id}")
> + @RequestFilters(AuthenticateRequest.class)
> + void detachInterface(@PathParam("server_id") String serverId, @PathParam("port_id") String portId);
Add the corresponding `@Named` annotation to the method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602097
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
I am not sure, but have you checked if these are available to list/delete using the neutron api?
________________________________
From: Limor Bortman-Stotland <no...@github.com>
Sent: Tuesday, March 10, 2015 12:27 PM
To: jclouds/jclouds
Subject: Re: [jclouds] adding attache detach interface to nova api (#697)
Hi @nacx<https://github.com/nacx> .
Thanks for the quick response.
I find it's a bit difficult to remove all the hard code UUIds of the networks and the port because I don't have any way to get them using the Nova Api.
I also tried to get an example from the ServerApiLiveTest and I saw that they are also using hard coded UUIds (see testCreateWithNetworkOptions() ).
So do you have any suggestion how can i get the UUId of the networks, ports and subnets?
Thanks
-
Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/697#issuecomment-78103968>.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78108248
Re: [jclouds/jclouds] adding attache detach interface to nova api
(#697)
Posted by Andrew Gaul <no...@github.com>.
Closed due to inactivity.
--
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/697#issuecomment-332911390
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
@limorbortman Apologies for the lack of feedback. We're about releasing 1.9.0 and the master branches are frozen. We expect to have it out by the end of the week, and we'll get back to the pending PRs.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-85413637
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
> @RequestFilters(AuthenticateRequest.class)
> @Consumes(MediaType.APPLICATION_JSON)
> @Path("/servers")
> -public interface AttachInterfaceApi {
> +public interface InterfaceApi {
I hope so.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r28621879
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @limorbortman! I've done a quick review, and apart from the inline comments, there are a couple things to address:
* Add a unit test for the `AttachInterfaceOptions` class that verifies the `bindToRequest` method.
* Format the code according to the [jclouds style guide ](https://cwiki.apache.org/confluence/display/JCLOUDS/Coding+Standards): 3 space indend and 120 line wrap.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-76722997
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
Hi @nacx and @everett-toews
I think I fix everything... can you please take a look?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78311905
Re: [jclouds/jclouds] adding attache detach interface to nova api
(#697)
Posted by Andrew Gaul <no...@github.com>.
@limorbortman do we have a path forward on 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/697#issuecomment-331354917
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
Hi @nacx, @everett-toews and @zack-shoylev
I think I fix everything... can you please take a look?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78418664
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
> @RequestFilters(AuthenticateRequest.class)
> @Consumes(MediaType.APPLICATION_JSON)
> @Path("/servers")
> -public interface AttachInterfaceApi {
> +public interface InterfaceApi {
So did we decide if we want to depart the code or do we want to give up on the backported?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r28754403
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
> +import org.jclouds.openstack.nova.v2_0.extensions.InterfaceApi;
> +import org.jclouds.openstack.nova.v2_0.internal.BaseNovaApiExpectTest;
> +import org.jclouds.openstack.nova.v2_0.options.AttachInterfaceOptions;
> +import org.jclouds.openstack.nova.v2_0.parse.ParseInterfaceAttachmentListTest;
> +import org.jclouds.openstack.nova.v2_0.parse.ParseInterfaceAttachmentTest;
> +import org.testng.annotations.Test;
> +
> +import java.net.URI;
> +
> +import static org.testng.Assert.assertEquals;
> +
> +/**
> + *
> + */
> +@Test(groups = "unit", testName = "InterfaceApiExpectTest")
> +public class InterfaceApiExpectTest extends BaseNovaApiExpectTest {
Does not seem to have fail tests, i.e. ...ResponseIs4xx() tests. Intended?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26591353
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + @RequestFilters(AuthenticateRequest.class)
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId, AttachInterfaceOptions... options);
> +
> + @DELETE
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/{server_id}/os-interface/{port_id}")
> + @RequestFilters(AuthenticateRequest.class)
Remove this. It is already declared at class level.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602080
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
LGTm. Just one comment i didn't notice in the previous review.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78014110
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> + String netId = "727ce2ef-6983-49c2-a404-e11a92e8fd28";
> + FixedIp fixedIp = FixedIp.builder().subnetId("cf18362d-f117-4598-b32d-edb316f26c3f").ipAddress("11.40.7.22").build();
> + List<FixedIp> fixIpsLis = new LinkedList<FixedIp>();
> + fixIpsLis.add(fixedIp);
> + AttachInterfaceOptions attachInterfaceOptions = AttachInterfaceOptions.Builder.netId(netId).fixedIps(fixIpsLis);
> + InterfaceAttachment inListenableFuture = interfaceApi.attachInterface("532eff71-c523-4380-aa24-e39695c983a2", attachInterfaceOptions);
> + assertNotNull(inListenableFuture);
> + assertNotNull(inListenableFuture.getMacAddr());
> + assertEquals(inListenableFuture.getFixedIp(), fixedIp);
> + assertNotNull(inListenableFuture.getPortId());
> + assertNotNull(inListenableFuture.getPortState());
> + assertEquals(inListenableFuture.getNetId(), netId);
> + }
> + }
> +
> + @Test(description = "DELETE /v${apiVersion}/{tenantId}/servers/{server_id}/os-interface/{port_id}\"")
The test dependency is still needed here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25880757
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> + assertNotNull(inListenableFuture);
> + assertNotNull(inListenableFuture.getMacAddr());
> + assertEquals(inListenableFuture.getFixedIp(), fixedIp);
> + assertNotNull(inListenableFuture.getPortId());
> + assertNotNull(inListenableFuture.getPortState());
> + assertEquals(inListenableFuture.getNetId(), netId);
> + }
> + }
> +
> + @Test(description = "DELETE /v${apiVersion}/{tenantId}/servers/{server_id}/os-interface/{port_id}\"", dependsOnMethods = {"testAttachedInterfaces"})
> + public void testDetachIInterfaces() throws Exception {
> + for (String regionId : regions) {
> + InterfaceApi interfaceApi = api.getInterfaceApiForZone(regionId).get();
> + interfaceApi.detachInterface("532eff71-c523-4380-aa24-e39695c983a2", "2839d4cd-0f99-4742-98ce-0585605d0222");
> + }
> + }
Where do all these uuids come from? Will the live test execution succeed in **any** OpenStack environment?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26105184
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> + * interface api for nova
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + @RequestFilters(AuthenticateRequest.class)
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId, AttachInterfaceOptions... options);
> +
> + @DELETE
> + @Consumes(MediaType.APPLICATION_JSON)
Remove this. It is already declared at class level.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602074
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Everett Toews <no...@github.com>.
> + public Set<org.jclouds.openstack.nova.v2_0.domain.FixedIp> getFixedIps() {
> + return fixedIps;
> + }
> +
> + public String getPortId() {
> + return portId;
> + }
> +
> + public String getNetId() {
> + return netId;
> + }
> +
> + public static class Builder {
> +
> + /**
> + * @see AttachInterfaceOptions#getPortState()()
Double `()()` here
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26171721
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
Hi @nacx .
Thanks for the quick response.
I find it's a bit difficult to remove all the hard code UUIds of the networks and the port because I don't have any way to get them using the Nova Api.
I also tried to get an example from the ServerApiLiveTest and I saw that they are also using hard coded UUIds (see testCreateWithNetworkOptions() ).
So do you have any suggestion how can i get the UUId of the networks, ports and subnets?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78103968
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
Hi @nacx, @everett-toews and @zack-shoylev
can you please take a look?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-85372414
Re: [jclouds/jclouds] adding attache detach interface to nova api
(#697)
Posted by Andrea Turli <no...@github.com>.
hi @limorbortman, can you address the last comment from @limorbortman so that we can finally merge this one?
---
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/697#issuecomment-238022635
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> + String netId = "727ce2ef-6983-49c2-a404-e11a92e8fd28";
> + FixedIp fixedIp = FixedIp.builder().subnetId("cf18362d-f117-4598-b32d-edb316f26c3f").ipAddress("11.40.7.22").build();
> + List<FixedIp> fixIpsLis = new LinkedList<FixedIp>();
> + fixIpsLis.add(fixedIp);
> + AttachInterfaceOptions attachInterfaceOptions = AttachInterfaceOptions.Builder.netId(netId).fixedIps(fixIpsLis);
> + InterfaceAttachment inListenableFuture = interfaceApi.attachInterface("532eff71-c523-4380-aa24-e39695c983a2", attachInterfaceOptions);
> + assertNotNull(inListenableFuture);
> + assertNotNull(inListenableFuture.getMacAddr());
> + assertEquals(inListenableFuture.getFixedIp(), fixedIp);
> + assertNotNull(inListenableFuture.getPortId());
> + assertNotNull(inListenableFuture.getPortState());
> + assertEquals(inListenableFuture.getNetId(), netId);
> + }
> + }
> +
> + @Test(description = "DELETE /v${apiVersion}/{tenantId}/servers/{server_id}/os-interface/{port_id}\"")
Test execution order is not guaranteed. Configure the test dependency to make sure this is always executed after the attach tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602244
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
I dont know why the comment is still here but i fixed it
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-84405263
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
HI @limorbortman! Apologies for the delay. jclouds 1.9.0 is out and we're back to reviewing PRs. Mind rebasing the PR to the latest version of master? Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-88235566
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Everett Toews <no...@github.com>.
> +
> +import javax.inject.Named;
> +import javax.ws.rs.Consumes;
> +import javax.ws.rs.DELETE;
> +import javax.ws.rs.POST;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * interface api for nova
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
This also need to implement the _List interfaces_ and _Show attached interface information_ methods from [os-interface](http://developer.openstack.org/api-ref-compute-v2-ext.html#ext-os-interface). If you have those methods you shouldn't have to depend on particular UUIDs in the tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26171229
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
@zack-shoylev @everett-toews Can you have a final look and merge this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-93421112
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
Hi @nacx, @everett-toews and @zack-shoylev
can you please take a look?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-82255621
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
@limorbortman : Will this work if you have the original code @Deprecated instead of removed?
Then we would also be able to backport, and remove the @Deprecated AttachInterfaceApi in a few releases.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-99110323
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
Hi @zack-shoylev and @nacx
If I will use the nuteronApi i will have to create a dependency between the openstack-nova model and the openstack-neutron model which is under the jclouds-labs-openstack....
I cant think of a way to remove the hardcoded uuid without creating a depend with the neutron api.
I'm pretty sure this live test can't run by anyone without changing the uuid manually, but on the other side the ServerLiveTest can't run as well...
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78140559
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> +import javax.ws.rs.DELETE;
> +import javax.ws.rs.POST;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * interface api for nova
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
Remove this. It is already declared at class level.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602071
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
HI @nacx,
I merge my commit to the master branch
can you please take a look?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-92773621
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
> @RequestFilters(AuthenticateRequest.class)
> @Consumes(MediaType.APPLICATION_JSON)
> @Path("/servers")
> -public interface AttachInterfaceApi {
> +public interface InterfaceApi {
But the problem might be that existing code will break. We might want to consider deprecating the original code instead, so it still works.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r28636359
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * interface api for nova
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + @RequestFilters(AuthenticateRequest.class)
remove this. It is already declared at class level.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602045
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
There seems to be a failure in ParseInterfaceAttachmentListTest?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78329975
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
I'm not the OpenStack expert, and @zack-shoylev, @everett-toews and @jdaggett may be able to help.
These are the live tests for the Nova API, and they should not depend on other APIs. Is there a way all those hardcoded values (in this test and in the mentioned ones that already existed) could be removed? If not, are we sure these live tests can be run by *anyone*?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78139052
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
Hm. I think there is a way to list the available nova networks using the nova api. It can be done through the networks extension (yep):
http://developer.openstack.org/api-ref-compute-v2-ext.html
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78158125
Re: [jclouds/jclouds] adding attache detach interface to nova api
(#697)
Posted by Andrew Gaul <no...@github.com>.
Closed #697.
--
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/697#event-1270278027
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
HI @nacx and @zack-shoylev
can we merge this change or should i do anything else?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-99104677
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Everett Toews <no...@github.com>.
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * 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.openstack.nova.v2_0.features;
This should be in `package org.jclouds.openstack.nova.v2_0.extensions` instead.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26171299
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Zack Shoylev <no...@github.com>.
However, I don't think we support that particular extension?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78158182
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Everett Toews <no...@github.com>.
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @Named("interface:attach")
> + @POST
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId);
> +
> + @Named("interface:attach")
> + @POST
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId, AttachInterfaceOptions options);
Needs some Javadoc. You can just copy the info out of [os-interface](http://developer.openstack.org/api-ref-compute-v2-ext.html#ext-os-interface).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26172099
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Everett Toews <no...@github.com>.
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * interface api for nova
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @Named("interface:attach")
> + @POST
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId);
Needs some Javadoc. You can just copy the info out of [os-interface](http://developer.openstack.org/api-ref-compute-v2-ext.html#ext-os-interface).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26172087
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Everett Toews <no...@github.com>.
> + @Named("interface:attach")
> + @POST
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId);
> +
> + @Named("interface:attach")
> + @POST
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId, AttachInterfaceOptions options);
> +
> + @Named("interface:detach")
> + @DELETE
> + @Path("/{server_id}/os-interface/{port_id}")
> + void detachInterface(@PathParam("server_id") String serverId, @PathParam("port_id") String portId);
Needs some Javadoc. You can just copy the info out of [os-interface](http://developer.openstack.org/api-ref-compute-v2-ext.html#ext-os-interface).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26172103
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Everett Toews <no...@github.com>.
> @@ -457,4 +457,12 @@ ImageApi getImageApiForZone(
> @Delegate
> Optional<? extends ConsolesApi> getConsolesExtensionForZone(
> @EndpointParam(parser = RegionToEndpoint.class) String zone);
> +
> + /**
> + * Provides access to interface Type features.
> + */
> + @Delegate
> + Optional<? extends InterfaceApi> getInterfaceApiForZone(
> + @EndpointParam(parser = RegionToEndpoint.class) String zone);
Should be `Optional<InterfaceApi> getInterfaceApi(@EndpointParam(parser = RegionToEndpoint.class) String region);`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r26171650
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
HI @nacx
Thanks For the quick response.
I fixed all the comments.
can you recheck?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-77317719
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Limor Bortman-Stotland <no...@github.com>.
Hi @nacx
Sorry, I missed the comment .
I now really fixed everything. Can you please take a look?
And I have a very small commit of only one file can you please take a look at that one as well (698)?
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697#issuecomment-78006941
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.openstack.nova.v2_0.domain.InterfaceAttachment;
> +import org.jclouds.openstack.nova.v2_0.internal.BaseNovaApiExpectTest;
> +import org.jclouds.openstack.nova.v2_0.options.AttachInterfaceOptions;
> +import org.jclouds.openstack.nova.v2_0.parse.ParseInterfaceAttachmentTest;
> +import org.testng.annotations.Test;
> +
> +import java.net.URI;
> +
> +import static org.testng.Assert.assertEquals;
> +
> +/**
> + *
> + */
> +@Test(groups = "unit", testName = "InterfaceApiTest")
Change name to "InterfaceApiExpectTest".
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602166
Re: [jclouds] adding attache detach interface to nova api (#697)
Posted by Ignasi Barrera <no...@github.com>.
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * interface api for nova
> + */
> +@RequestFilters(AuthenticateRequest.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/servers")
> +public interface InterfaceApi {
> +
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @SelectJson("interfaceAttachment")
> + @Path("/{server_id}/os-interface")
> + @RequestFilters(AuthenticateRequest.class)
> + InterfaceAttachment attachInterface(@PathParam("server_id") String serverId, AttachInterfaceOptions... options);
Add the corresponding `@Named` annotation to this method.
Don't use a varargs parameter to express that a parameter is Optional. Overload the method to have two: one without the parameter and one with the parameter (since you don't want more than one options object being passed).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/697/files#r25602019