-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CLOUDSTACK-8897: baremetal:addHost:make host tag info mandtory in bar… #874
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
Conversation
|
cloudstack-pull-rats #700 SUCCESS |
|
cloudstack-pull-analysis #650 SUCCESS |
|
We won't be able to test this on simulator. |
| } | ||
|
|
||
| if ((hypervisorType.equalsIgnoreCase(HypervisorType.BareMetal.toString()))) { | ||
| if (hostTags == null || hostTags.size() == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @harikrishna-patnala just a little tip on line 684. You could use Apache Commons' CollectionUtils to make such checks, it does exactly the same thing but is a little more clean. But that's just a little adjustment. http://commons.apache.org/proper/commons-collections/javadocs/api-release/org/apache/commons/collections4/CollectionUtils.html#isEmpty(java.util.Collection)
|
@harikrishna-patnala please rebase against latest master and push -f, update on status of your PR LGTM |
ACS CI BVT RunSumarry: Link to logs Folder (search by build_no): https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0 Failed tests:
Skipped tests: Passed test suits: |
aa3d020 to
7f63dca
Compare
…emetal addhost Api call addhost api is successful with out providing the host tag info and we recommend host tag is mandatory for bare-metal. In the current implementation host tag check is happening at vm deployment stage but it will be good to have host tag field as mandatory field during adding of the host it self.
7f63dca to
1e256cc
Compare
|
I have updated the PR based on the comments. Please review. |
ACS CI BVT RunSumarry: Link to logs Folder (search by build_no): https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0 Failed tests:
Skipped tests: Passed test suits: |
|
There are 2 failed test cases which are failing on all other PRs |
|
LGTM |
|
tag:mergeready |
* Add button to enable/disable storage pool * Disable only if pool state is Up Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
CLOUDSTACK-8897: baremetal:addHost:make host tag info mandtory in baremetal addhost Api call
addhost api is successful with out providing the host tag info and we recommend host tag is mandatory for bare-metal.
In the current implementation host tag check is happening at vm deployment stage but it will be good to have host tag field as mandatory field during adding of the host