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

Filter VMs in vm list views #764

Closed
4 tasks
yaacov opened this issue Oct 25, 2023 · 9 comments
Closed
4 tasks

Filter VMs in vm list views #764

yaacov opened this issue Oct 25, 2023 · 9 comments
Assignees
Labels
enhancement Categorizes issue or PR as related to a new feature.
Milestone

Comments

@yaacov
Copy link
Member

yaacov commented Oct 25, 2023

Enhancement:
Filter VMs in VM list views, and match functionality ( * ) with the current filters we have in the create plan wizard.

(*) need to get an "ack" from back-end the filters are still relevant for 2.6 version, maybe we need to update the filters to match new functionalities.

Currently
a. We filter VMs in two stages using a tree view.
b. Filtering is using "@migtools/lib-ui"

Changes needed:
Deprecate "@migtools/lib-ui" implementation in favor of an in-list one-step list filters

Sub tasks:

  • map current filters used for each provider in the create plan wizard
    get an "ack" form back-end to assert this are actually good filters fro this version
  • map the fields that are needed for list filter implementation ( columns we have and that are missing )
  • map gaps. e.g fields needed for filtering but missing in inventory API
    raise missing fields issues to back-end and find workarounds if needed
  • implement filters for each provider type
@yaacov yaacov added the enhancement Categorizes issue or PR as related to a new feature. label Oct 25, 2023
@yaacov yaacov added this to the 2.6.0 milestone Oct 25, 2023
@ahadas
Copy link
Member

ahadas commented Nov 8, 2023

Enhancement: Filter VMs in VM list views, and match functionality ( * ) with the current filters we have in the create plan wizard.

ok, that part explains why I had the filtering in the VM list view when starting to comment on #770 :)

further to what I wrote there about filtering by-feature, I have no strong objection to filter by feature but there can be quite a duplication if this is introduced together with filtering by concern. a representative example is TPM - we would want to display a warning about migration of VMs that are configured with TPM, and if we enable filtering VMs with such warning then filtering them by feature=Persistent TPM seems redundant.

out validations are definitely incomplete and some of them should be fixed - the question is whether it would be a better use of our time to fix them and introduce filter by-concern instead of by-feature

@yaacov
Copy link
Member Author

yaacov commented Nov 8, 2023

out validations are definitely incomplete and some of them should be fixed - the question is whether it would be a better use of our time to fix them and introduce filter by-concern instead of by-feature

sounds good to me,
we will need a way to do finer filtering by warnings/errors , currently we only filter by existing of warning/errors, we will need to build a way to filter by content/message of the warning

@ahadas
Copy link
Member

ahadas commented Nov 8, 2023

out validations are definitely incomplete and some of them should be fixed - the question is whether it would be a better use of our time to fix them and introduce filter by-concern instead of by-feature

sounds good to me, we will need a way to do finer filtering by warnings/errors , currently we only filter by existing of warning/errors, we will need to build a way to filter by content/message of the warning

yep, I don't think it would be complicated to add a type that we agree on to the reported concerns

@ahadas
Copy link
Member

ahadas commented Nov 9, 2023

@yaacov looking at #778, I want to follow up on the error code-like (/machine-readable) representation of the concerns

I see your point in saying that discovering the labels from the queried VMs may be problematic with large number of VMs, but let's say that the concern for TPM will be set with a numeric code (e.g., 50) or some string (e.g., TPM) - how would you determine the description to display for this concern without getting it from a VM? (I don't think you can get the set to possible concerns, right?)

also, I'm not sure what would actually be a better user experience, to see and be able to select any concern (including concerns that don't match any of the reported VMs) or show only concerns that match at least one VM

@yaacov
Copy link
Member Author

yaacov commented Nov 9, 2023

also, I'm not sure what would actually be a better user experience, to see and be able to select any concern (including concerns that don't match any of the reported VMs) or show only concerns that match at least one VM

currently we have label, category and assessment

https://github.com/kubev2v/forklift/blob/0849112a069461d5d0f85074c96e3a62dd93b759/pkg/controller/provider/model/base/model.go#L47

IMHO we need a new field that is more expresive then category , we already filter by category ( it's a known 3 values ) , it should be a known pre defined short list of string labels:

a. i prefer a short string and not an error code, so it will be human readable and the developer writing a new warning will easily know what value match the new warning, e.g. category='warning' group='disk warnings' label='bla, bla ...'
b. i prefer one short string for a group of errors, for example all errors related to disks can be labeled group='snapshot error'
c. i prefer the list of known short strings to be small and well known, each new error/warning should get one of this label strings that match the error/warning group, the UI will have this list hardcoded, no need to fetch it dynamiclly

@ahadas
Copy link
Member

ahadas commented Nov 9, 2023

IMHO we need a new field that is more expresive then category , we already filter by category ( it's a known 3 values ) , it should be a known pre defined short list of string labels:

yes, the category was not really meant for filtering - it's more about what to do when a concern appears (block the migration or show warning and let the user to proceed), but it's a good idea in my opinion to allow filtering by that as well

a. i prefer a short string and not an error code, so it will be human readable and the developer writing a new warning will easily know what value match the new warning, e.g. category='warning' group='disk warnings' label='bla, bla ...' b. i prefer one short string for a group of errors, for example all errors related to disks can be labeled group='snapshot error' c. i prefer the list of known short strings to be small and well known, each new error/warning should get one of this label strings that match the error/warning group, the UI will have this list hardcoded, no need to fetch it dynamiclly

there's a bit of an overlap between these items so tell me if I understand it properly by saying that it concretely means to:

  1. add a "group" field to a concern
  2. map each concern to a group
  3. the frontend will store these groups and we need to make sure they align with what the backend sets on concerns
  4. the frontend will allow to filter by catagory and group (not by label)

is that correct?

@yaacov
Copy link
Member Author

yaacov commented Nov 9, 2023

  1. add a "group" field to a concern
  2. map each concern to a group
  3. the frontend will store these groups and we need to make sure they align with what the backend sets on concerns
  4. the frontend will allow to filter by catagory and group (not by label)

is that correct?

exectly 👍

@ahadas
Copy link
Member

ahadas commented Nov 9, 2023

ok, just checked the amount of validations we have for each provider:

$ ls ovirt/ | grep -v test | wc -l
37
$ ls openstack/ | grep -v test | wc -l
18
$ ls vmware/ | grep -v test | wc -l
21

need to take this as an exercise to see how many groups we end up with for each provider

@yaacov
Copy link
Member Author

yaacov commented Jan 11, 2024

closed by #778

@yaacov yaacov closed this as completed Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants