Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Tacker Vnf - Attributes & TackerError Propagation.. #814

Merged
merged 4 commits into from
Sep 20, 2016

Conversation

vishvesh
Copy link
Contributor

@vishvesh vishvesh commented Sep 15, 2016

Added Tacker Vnf - Attributes - Static Create method..
  • Added missing Tacker Vnf static create() method for setting Vnf Attributes..
  • This can then be set in VnfBuilder attributes(VnfAttributes attributes);
Added TackerError Propagation..
  • TackerError is now propagated..
  • Currently propagating for register - vim, create - vnf and create vnfd calls on status 500..
  • Currently there is no way to propagate 404 status as well.. Can this be enhanced?
    • create method (with param PropagateOnStatus) only propagates on the provided status code..

vdeshmukh added 2 commits September 15, 2016 15:47
* Added missing `Tacker Vnf` `static create()` method for setting `Vnf
Attributes`..
* `TackerError` is now propagated..
* Currently `propagating` for `register - vim` call on status `500`..
* Currently there is no way to propagate `404` status as well..
@vishvesh vishvesh changed the title Added Tacker Vnf - Attributes - Static Create method.. Added Tacker Vnf - Attributes & TackerError Propagation.. Sep 16, 2016
* Added `500` status error propagation for `Vnf` and `Vnfd` Create
call..
@auhlig
Copy link
Member

auhlig commented Sep 16, 2016

@vishvesh Is that the PR you want to continuously work on? Maybe worth adding a WIP, do not merge in the beginning for the time being.

@vishvesh
Copy link
Contributor Author

@auhlig : All the changes (except for VIM Update) have been contributed for Tacker.. So I believe these can be merged..

I'll probably have a WIP for VPN implementation.. Is that fine or should this be the WIP where I contribute VPN as well?

@auhlig
Copy link
Member

auhlig commented Sep 16, 2016

Ah okay. Cool. Code looks good so far 👍 Could you also add some short tests?

Sounds good. I think it makes sense to get this one done and create a new PR for VPN.

@vishvesh
Copy link
Contributor Author

@auhlig : I tried adding tests for the same, but the only output I get is something like below:

Sep 16, 2016 2:14:58 PM org.glassfish.jersey.filter.LoggingFilter log
INFO: 1 * Sending client request on thread http-nio-80-exec-3
1 > POST http://10.10.1.132:9890/v1.0/vims
1 > Accept: application/json
1 > Content-Type: application/json
1 > User-Agent: OpenStack4j / OpenStack Client
1 > X-Auth-Token: 52d880f348f74a0fac9c402648870e1e
{
  "vim" : {
    "name" : "test",
    "description" : "test-desc",
    "type" : "openstack",
    "is_default" : true,
    "auth_cred" : {
      "username" : "admin",
      "password" : "password",
      "user_domain_name" : ""
    },
    "auth_url" : "",
    "vim_project" : {
      "name" : "",
      "project_domain_name" : ""
    }
  }
}
TackerError
{"TackerError": {"message": "'project_domain_name' is missing", "type": "VimProjectDomainNameMissingException", "detail": ""}}

The Mock server complains about JsonMappingException, so I don't really want to spend more time on this.. The error is propagated on any 500 status message.. So this shouldn't really break anything..

@auhlig
Copy link
Member

auhlig commented Sep 17, 2016

I'd really appreciate tested code.
JsonMappingException should be thrown. Not sure I get the error you described in the last sentence. What about the empty strings in the request - especially the project domain id it complains about?

@vishvesh
Copy link
Contributor Author

@auhlig : The empty string that is being passed, is just a simulation for triggering the exception.. The project _domain_name is required, and that's why is throws an error..

* Added `testRegisterVimWithTackerError` method for `simulating` an
`TackerError`..
@vishvesh
Copy link
Contributor Author

vishvesh commented Sep 19, 2016

@auhlig : Test added.. Please Review !!

1 > POST http://127.0.0.1:9890/v1.0/vims
1 > Accept: application/json
1 > Content-Type: application/json
1 > User-Agent: OpenStack4j / OpenStack Client
1 > X-Auth-Token: 123456789
{
  "vim" : {
    "name" : "test-vim",
    "description" : "test-vim-description",
    "type" : "openstack",
    "is_default" : true,
    "auth_cred" : {
      "username" : "admin",
      "password" : "password",
      "user_domain_name" : "default"
    },
    "auth_url" : "http://openstack.os4j.com:35357/v3",
    "vim_project" : {
      "name" : "admin"
    }
  }
}

Sep 19, 2016 4:07:42 PM okhttp3.mockwebserver.MockWebServer$4 processOneRequest
INFO: MockWebServer[9890] received request: POST /v1.0/vims HTTP/1.1 and responded: HTTP/1.1 500 Server Error
Sep 19, 2016 4:07:42 PM org.glassfish.jersey.filter.LoggingFilter log
INFO: 1 * Client response received on thread main
1 < 500
1 < Content-Length: 78
1 < Content-Type: application/json
{"TackerError": {"message": "'project_domain_name' is missing.", "code": 500}}

@auhlig
Copy link
Member

auhlig commented Sep 20, 2016

Thanks for the work. LGTM

@auhlig auhlig added this to the 3.0.3 Release milestone Sep 20, 2016
@auhlig
Copy link
Member

auhlig commented Sep 20, 2016

What do think on this @vinodborole ?

@vinodborole
Copy link
Contributor

LGTM @auhlig

@auhlig auhlig merged commit c71dc82 into ContainX:master Sep 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants