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

Add VM set_description action #1120

Merged
merged 1 commit into from
May 24, 2022

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Jan 19, 2022

Alternative to #1112

ManageIQ/manageiq#21561

POST /api/vms/:id -d {"action": "set_description", "new_description": "test"}
{
  "success": true,
  "message": "VM id:2 name:'VM_NAME' setting description to test",
  "task_id": "82",
  "task_href": "http://localhost:3000/api/tasks/82",
  "href": "http://localhost:3000/api/vms/2"
}

@agrare sorry. Just had to tweak the way you wrote set_resource_description

spec/requests/vms_spec.rb Outdated Show resolved Hide resolved
@kbrock
Copy link
Member Author

kbrock commented Feb 1, 2022

@agrare agreed
As we are standardizing the infrastructure, we are going towards 404.

While I prefer 403, we can not go that direction. Our rbac is on the query so nothing comes back - we can't tell the difference between 403/404.

I have a number of outstanding branches/PRs consolidating code and those move us to towards this 404 consistency

@kbrock kbrock changed the title Add support vm set description Add VM set_description action Feb 8, 2022
raise BadRequestError, "Setting description requires a new_description" if new_description.blank?

task_id = vm.set_description_queue(User.current_user.userid, new_description)
action_result(true, "Setting description for #{model_ident(vm, type)} to #{new_description}", :task_id => task_id)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to tell the user what they passed in, or can we just use the default description of "Setting description for #{model_ident(vm, type)}" (which will get automatically added for us).

@kbrock kbrock force-pushed the add_support_vm_set_description branch from 1372260 to 66647c4 Compare March 5, 2022 18:49
@kbrock kbrock force-pushed the add_support_vm_set_description branch from 66647c4 to 7752b27 Compare May 24, 2022 17:39
@kbrock
Copy link
Member Author

kbrock commented May 24, 2022

I just rewrote this and realized that this PR already existed

update:

  • rebased
  • added a few spec lines

config/api.yml Outdated
@@ -4534,6 +4534,8 @@
:identifier: vm_guest_shutdown
- :name: reboot_guest
:identifier: vm_guest_restart
- :name: set_description
:identifier: vm_rename
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this identifier...I'm wondering if vm_edit is better.

@kbrock kbrock force-pushed the add_support_vm_set_description branch from 7752b27 to 37fb0f1 Compare May 24, 2022 19:55
@kbrock
Copy link
Member Author

kbrock commented May 24, 2022

update:

  • change privs from vm_rename to vm_edit

@kbrock kbrock force-pushed the add_support_vm_set_description branch from 37fb0f1 to f9d6b0c Compare May 24, 2022 20:45
@miq-bot
Copy link
Member

miq-bot commented May 24, 2022

Checked commit kbrock@f9d6b0c with ruby 2.6.9, rubocop 1.19.1, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 👍

@Fryguy Fryguy merged commit 587a75f into ManageIQ:master May 24, 2022
@Fryguy Fryguy assigned Fryguy and unassigned agrare May 24, 2022
@kbrock kbrock deleted the add_support_vm_set_description branch May 25, 2022 13:10
@kbrock kbrock mentioned this pull request Jun 10, 2022
1 task
@Fryguy Fryguy mentioned this pull request Dec 22, 2022
18 tasks
@jeffibm
Copy link
Member

jeffibm commented Nov 3, 2023

Hey @kbrock, I was trying to use the API like this -

http://localhost:3000/api/vms/2983?action=set_description&new_description=test description

is this the right way?

Also, this is how the component is being rendered.

= react('VmEditForm', {:recordId    => @record.id&.to_s,
                         :emsId       => @record.ems_id&.to_s,
                         :showTitle   => !@edit[:explorer],
                         :isTemplate  => @record.kind_of?(::MiqTemplate),
                         :displayName => model_for_vm(@record).display_name})

we have 2 kinds of id in here,

  1. @record.id (ManageIQ::Providers::redhat-new::InfraManager::Vm) and
  2. @record.ems_id

I am guessing we need to use the @record.id in the API post request.

@kbrock
Copy link
Member Author

kbrock commented Nov 3, 2023

Hey Jeffrey,

Use @record.id.
The rest looks good to me.

From here, the actual description will not change but rather a message will get sent to the provider to change it at the source.

Think you'll get a task I'd, and when the task completes, it will tell you success but in fact the success is the queuing to the provider.

If you have a provider setup, then it will flow back, otherwise you won't be able to test it on your machine.

@kbrock
Copy link
Member Author

kbrock commented Nov 13, 2023

@jeffibm Think the issue is this needs to be a POST and not a GET

http://localhost:3000/api/vms/2983?action=set_description&new_description=test description
(where 2983 is @record.id)

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.

5 participants