Skip to content

Conversation

chitra607
Copy link

No description provided.

@chitra607 chitra607 requested a review from a team as a code owner July 17, 2025 11:04
@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2025

Checked commits chitra607/manageiq-ui-classic@9297892~...d1f700d with ruby 3.1.7, rubocop 1.56.3, haml-lint 0.64.0, and yamllint
9 files checked, 10 offenses detected

app/controllers/vm_common.rb

app/controllers/vm_infra_controller.rb

app/helpers/application_helper/button/vm_attach_volume.rb

app/helpers/application_helper/button/vm_detach_volume.rb

app/views/vm_common/_add_volume.html.haml

  • 💣 💥 🔥 🚒 - Line 0 - app/views/vm_common/_add_volume.html.haml - Invalid filter name ":recordid => @record.id.to_s,".

app/views/vm_common/_remove_volume.html.haml

  • 💣 💥 🔥 🚒 - Line 0 - app/views/vm_common/_remove_volume.html.haml - Invalid filter name ":recordid => @record.id.to_s,".

@agrare
Copy link
Member

agrare commented Jul 23, 2025

@chitra607 I think we want to use VM Reconfigure for this so it would fit in to existing UI / API flows and not need anything extra here

def storage_class_list
vm = find_record_with_rbac(VmOrTemplate, params[:id])
ems = vm.ext_management_system
storage_class = vm.storage_classes
Copy link
Member

Choose a reason for hiding this comment

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

NOTE storage_classes isn't something that we currently have. We would need to collect them in inventory refresh as part of the provider and persist them in the database for this to work. We do currently have a StorageProfile model which we could use to store these without having to create a new table/model.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @agrare. Just to clarify, storage_classes here is a function I added that fetches the list directly from the OpenShift/KubeVirt API at runtime, it is not a table/model. Would it make sense to use the existing StorageProfile model to persist storage classes list in OpenShift/KubeVirt as you suggested?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's not going to work for a couple of reasons, first this is generic VM code so we can't have provider specific methods in here, also since this is running from the UI role we can't run anything that would hit the live provider since that can only be guaranteed to be accessed via ems_operations role in the proper zone.

StorageProfiles sound like they could be a good match, what data do you need for them other than uid/name?

Copy link
Author

Choose a reason for hiding this comment

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

Screenshot_3-9-2025_104658_192 168 210 200

In the infrastructure VM volume attach workflow we have two options:

  • Select an existing PVC
  • Create a new PVC and attach it to the VM
    For the create new PVC case, I need to list the available OpenShift storage class names so the user can choose one (since storageClassName is required when creating a PVC)

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.

4 participants