-
Notifications
You must be signed in to change notification settings - Fork 982
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
Fixes #13259 - Add create_additional_volume_api method #3088
Conversation
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
It looks like you're adding new commits with the fixed message. If you can squash your commits into a single one, then push that then it should fix the issue - see http://mattsnider.com/amending-and-squashing-commits-in-git/. |
af837ec
to
4ac312b
Compare
[test] |
@dLobatog would you mind reviewing this? I'm afraid I don't know anything about the subject. @plumcraft please note there are some rubocop style errors, run |
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
116cedc
to
602e2b0
Compare
|
||
# Iterate into volumes attributes | ||
args[:volumes_attributes].each do |key, value| | ||
boot_index = key.to_i + 1 |
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.
This sets a 'hidden contract' where the order of the volumes you specify determines the boot order. Can you make this use boot_index from the arguments? If it's not available, then fine, default to this. Leave a comment mentioning it, as we would have to add that to the documentation.
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.
Will do, thanks
@plumcraft Thanks! I left a couple of comments inline to use the arguments instead of defaulting always to something. Also notice this won't work with the v1 Openstack API. This will only work through hammer, which is not great. Would you mind adding some UI for this? We have some examples for other Compute resources under I had to test this with #3051 as currently Openstack provisioning is broken in develop. Also testing this one made me realize this bug http://projects.theforeman.org/issues/13356 , so thanks for that 😄 |
Thank you for your contribution, @plumcraft! This PR has been inactive for 6 months, closing for now. |
Similar to Feature #11577, when creating a new vm using Openstack with Hammer API, additional volumes won't be created. I have implemented and tested this feature in our development environment.
I put only volume attributes via hammer .
hammer :
--volume="capacity=14"
--volume="capacity=15"