[WIP] Convert console_supported? to supports?
part of https://github.com/ManageIQ/manageiq/issues/21989
First:
- Added
supports :html5_consoleto providers instead of deriving it in main. (it is supported if any of the following is supported:spice_console,vnc_console,webmks_console. - Added
supports :consoleto providers with the same logic. Note: before,webmks_consolewas not in this list. -
:html5_consoleand:consoleare now basically identical. - Droped
console_supported?method. - Moved logic from
:launch_html5_console,:launch_native_console,:launch_vmrc_consoletosupports :{}_consoleblocks. -
supports :*_consolenow consistently have blocks checking vm for required attributes.
Notes:
- I had expected
supported_console?to be more prevalent. Felt like it was defined in a bunch of places but not really used that much. - Think some of the unsupported messaging changed in the
supportsfor providers changed. Feels like we could normalize these but probably best to make as few changes here to not produce too many changes the he localization strings. - vmware cloud manager states
supports :html5_console, but unlike others, doesn't have thesupports :*_console. - lenovo states
supports :console, but doesn't state the specific types. (same as above) - some enable/disable
html5_consolesupport based upon a feature flag or the state of the vm (e.g.archived?). Other providers seem to do his logic insupports :launch_html5_console -
supports :html5_consoleand:consoleare looking the same
This requires the following PRs in other providers:
- [ ] https://github.com/ManageIQ/manageiq-api/pull/1209 DONE
- [ ] https://github.com/ManageIQ/manageiq-providers-red_hat_virtualization/pull/24
- [ ] https://github.com/ManageIQ/manageiq-providers-openstack/pull/841
- [ ] https://github.com/ManageIQ/manageiq-providers-ovirt/pull/635
- [ ] https://github.com/ManageIQ/manageiq-providers-ibm_cloud/pull/446
- [ ] https://github.com/ManageIQ/manageiq-providers-vmware/pull/864 DONE
- [ ] https://github.com/ManageIQ/manageiq-ui-classic/pull/8687
all were modified to handle the supports Put DONE by the ones that also converted validate and launch_
The wording on some of the older ones may be a little different. Here are the old values:
supports_not :html5_console # { _("The web-based HTML5 Console is not supported") }
supports_not :native_console # { _("VM NATIVE Console not supported") }
supports_not :spice_console # { N_("Console not supported") }
supports_not :vmrc_console # { _("VMRC Console not supported") }
supports_not :vnc_console # { N_("Console not supported") }
supports_not :webmks_console # { N_("Console not supported") }
I would have preferred it if the feature name were not included in the definition. It feels like "not supported" is good enough for the text, but unsure of the implications in the ui.
spec/models/vm/operations_spec.rb
- [ ] :exclamation: - Line 56, Col 5 - Style/CommentAnnotation - Annotation keywords like
TODOshould be all upper case, followed by a colon, and a space, then a note describing the problem.
This pull request has been automatically closed because it has not been updated for at least 3 months.
Feel free to reopen this pull request if these changes are still valid.
Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.
If this seems like a good idea, maybe we should just add the supports into the children classes and not do a block here at all
This pull request has been automatically closed because it has not been updated for at least 3 months.
Feel free to reopen this pull request if these changes are still valid.
Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.
The consensus is we want to remove any dynamic calculations we are doing here and just add the following to the various provider implementations:
grep for console_supported spice
supports :console # ovirt has this dependent upon html5 enabled
supports :html5_console
supports :native_console # currently a thing
supports :vnc_console # ibm (currently console_supports)
update:
- old version was a POC
- removed blocks from supports :console, :html_console
- (in child providers) webmks now linked to console (in addition to html_console) - only changed vmware
If you like the direction this is headed, then we need a big cross repo test.
update:
- missed the change to vm/operations. the tests were still green. concerning but assuming (hoping) the provider tests would have been red for those instead?
Hard to tell from this PR, but if the actual supported features are changing, then I think we need to update the service UI as well. cc @DavidResende0 as he's looking at those specifically as we speak.
WIP:
- this got too big
- seems the concepts of the various supported are confusing
NEXT STEPS: Go through the providers mentioned and copy validates logic into supports blocks.
Some providers have conditional logic in the supports :vnc_console and others don't even have supports :vnc_console and only use supports_console?('vnc')
This pull request has been automatically marked as stale because it has not been updated for at least 3 months.
If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s)
Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.
This pull request has been automatically marked as stale because it has not been updated for at least 3 months.
If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s)
Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.
This pull request has been automatically marked as stale because it has not been updated for at least 3 months.
If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).