You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by jmourelos <no...@github.com> on 2014/03/27 23:44:17 UTC
[jclouds] JCLOUDS-317: Add missing states to Nova v2 Server (#334)
- Remove state: STOPPED
- Add states: MIGRATING, SHUTOFF, RESCUE, SOFT_DELETED,
SHELVED, SHELVED_OFFLOADED
- Related blocker issue: https://issues.apache.org/jira/browse/JCLOUDS-317
- It would very nice if, in case of being merged, the pull request could be backported to the 1.7.x branch.
You can merge this Pull Request by running:
git pull https://github.com/jmourelos/jclouds JCLOUDS-317
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/334
-- Commit Summary --
* JCLOUDS-317: Add missing states to Nova v2 Server
-- File Changes --
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/Server.java (3)
M apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/features/ServerApiExpectTest.java (37)
A apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/parse/ParseServerDetailsStatesTest.java (264)
A apis/openstack-nova/src/test/resources/server_list_details_states.json (342)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/334.patch
https://github.com/jclouds/jclouds/pull/334.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1202](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1202/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39501469
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #718](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/718/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-38874883
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #993](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/993/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39505774
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Andrew Phillips <no...@github.com>.
> + .links(
> + Link.create(
> + Relation.SELF,
> + URI.create("http://openstack:8774/v2/4e1900cf21924a098709c23480e157c0/servers/cad76945-8851-489a-99e1-f1049e02c769")),
> + Link.create(
> + Relation.BOOKMARK,
> + URI.create("http://openstack:8774/4e1900cf21924a098709c23480e157c0/servers/cad76945-8851-489a-99e1-f1049e02c769")))
> + .image(
> + Resource.builder()
> + .id("e3f84189-964e-4dc3-8ac6-832c2b7553d4")
> + .links(
> + Link.create(
> + Relation.BOOKMARK,
> + URI.create("http://openstack:8774/4e1900cf21924a098709c23480e157c0/images/e3f84189-964e-4dc3-8ac6-832c2b7553d4"))).build())
> + .flavor(
> + Resource.builder()
If we're already reformatting, could you change things like
```
.flavor(
Resource.builder()
```
to
```
.flavor(Resource.builder()
...
```
? A lot of review scrolling required now ;-) Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334/files#r11156181
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #732](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/732/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39501372
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by jmourelos <no...@github.com>.
I have seen in the JIRA issue that the 1.6.x version is marked as affected, so I guess that either the pull request should be also backported to the 1.6.x version or the "Affects version/s" field in the JIRA ticket should be changed accordingly (but I do not have permissions to do so).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-38871151
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Everett Toews <no...@github.com>.
Squashed and merged to master and 1.7.x
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39516000
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1188](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1188/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-38875451
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Everett Toews <no...@github.com>.
Closed #334.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Everett Toews <no...@github.com>.
Can you please put the STOPPED state back?
I realize the OpenStack doesn't return this but that state has been in jclouds for a long time and some code is bound to depend on it. @Deprecated STOPPED and add a comment telling users to change it to SHUTOFF.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39144844
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #975](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/975/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-38874695
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Andrew Phillips <no...@github.com>.
> @@ -59,7 +59,15 @@
> */
> public static enum Status {
>
> - ACTIVE, BUILD, REBUILD, SUSPENDED, PAUSED, RESIZE, VERIFY_RESIZE, REVERT_RESIZE, PASSWORD, REBOOT, HARD_REBOOT, DELETED, UNKNOWN, ERROR, STOPPED, UNRECOGNIZED;
> + ACTIVE, BUILD, REBUILD, SUSPENDED, PAUSED, RESIZE, VERIFY_RESIZE, REVERT_RESIZE, PASSWORD, REBOOT, HARD_REBOOT,
[minor] Formatting - this is now a [4-space indent](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=blob;f=apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/Server.java;h=143131c0c4cd68baab11d87fe8ae47bd08a426c3;hb=222779bbc4052c41a6db0e484cee44b58251abdd), should be 3.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334/files#r11323011
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Andrew Phillips <no...@github.com>.
> @@ -82,6 +83,42 @@ public void testListServersWhenReponseIs404IsEmpty() throws Exception {
> assertTrue(apiWhenNoServersExist.getServerApiForZone("az-1.region-a.geo-1").list().concat().isEmpty());
> }
>
> + public void testListInDetailServersWhenResponseIs2xx() throws Exception {
> + HttpRequest listServers = HttpRequest
[minor] Indents - jclouds uses 3 spaces, this code uses 4. Could you reformat?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334/files#r11156145
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Everett Toews <no...@github.com>.
SGTM. No translation logic is necessary. AFAICT, the actual string "STOPPED" has never been returned by OpenStack.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39207787
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by jmourelos <no...@github.com>.
I think I have addressed the issues pointed out during review. I must say that the lines in the serialization test are still quite long, the only solution I see is to change the real UUIDs to fake UUIDs (like 1234) as it is done in other tests. However I thought that the fact of using real payloads from an OpenStack instance would be nice.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39496638
Re: [jclouds] JCLOUDS-317: Add missing states to Nova v2 Server
(#334)
Posted by Andrew Phillips <no...@github.com>.
> Can you please put the STOPPED state back?
+1. Good catch, @everett-toews! What would your thoughts be on deprecating it in 1.7.x and removing it on master? Do we need any kind of translation logic to handle responses from older provider versions?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/334#issuecomment-39195397