Added: vm filtering #458
Added: vm filtering #458
Conversation
This adds a way to create Filter objects from a given string. It also adds the filtering mechanism itself that will be used in the `ignite ps` command
This adds the filter flag to the `ignite ps` command
chanwit
requested review from
chanwit and
stealthybox
and removed request for
twelho
chanwit
added
area/UX
pkg/filter/meta.go
Outdated
| } | ||
| metaFilterList := make([]metaFilter, 0, len(filtersInfos)) | ||
| for _, fInfo := range filtersInfos { | ||
| metaFilterList = append(metaFilterList, metaFilter{identifier: fInfo["key"], expectedValue: fInfo["value"]}) |
There was a problem hiding this comment.
This conversion is not needed.
extractMultipleKeyValueFiltering() can return a []metaFilter directly.
Did you mean to return a map[string]string of filterKey => value and convert that?
(I wouldn't do that if you want to support negation operators)
pkg/filter/meta.go
Outdated
| return match[1], match[2], nil | ||
| } | ||
|
|
||
| // extractMultipleKeyValueFiltering extracts all he keys and values to filter |
pkg/filter/meta.go
Outdated
| for _, filter := range filterList { | ||
| key, value, err := extractKeyValueFiltering(filter) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
it would be helpful to include the failing filterList here for the user
pkg/filter/meta.go
Outdated
| const ( | ||
| filterSeparator = "," | ||
| filterApplyFailed = "failed to apply filtering" | ||
| regexString = `^(?P<key>{{(?:\.|[a-zA-Z]+)+}})=(?P<value>[a-zA-Z0-9-_]+)$` |
There was a problem hiding this comment.
Can we capture the operator here and support = == != =~ !~?
The operator can then be stored in the metaFilter.
The filter can be executed with a case statement on the op
= == Equal
!= Not Equal
=~ Contains
!~ Not Contains
Also, note that this regex does not support all valid go identifiers for the key:
# the filter can fail to compile with valid identifiers
sudo ~/Repos/ignite/bin/ignite vm ls -f '{{.Name2}}=my-test-vm'
FATA[0000] failed to generate filter
pkg/filter/meta.go
Outdated
| } | ||
| matches := reg.FindAllStringSubmatch(str, -1) | ||
| if len(matches) != 1 { | ||
| return "", "", fmt.Errorf("failed to generate filter") |
There was a problem hiding this comment.
Update error to indicate filter contained more than one key + op + value
pkg/filter/meta.go
Outdated
| } | ||
| match := matches[0] | ||
| if len(match) != 3 { | ||
| return "", "", fmt.Errorf("failed to generate filter") |
There was a problem hiding this comment.
Update error to indicate filter was missing a key, operator, or value
|
Further U/X could be done in a follow-up:
This is also related to the U/X of #229 |
Filters now handles operators and other field than Metadata
|
I think I'm ready for the next round |
|
Thanks so much for the persistence to get this through. This is a very high quality addition to ignite. One tweak we can follow up with is to perform an We can also document This is great to merge as is 👍 |
This adds vm filtering to the
ignite pscommand with the-for--filterflag.We can now apply filtering on VM metadata
We can use just one filter or multiple.
ex one filter
sudo ignite ps --filter "{{.Name}}=my-vm"ex multi filters (separating them with a
,):sudo ignite ps --filter "{{.Name}}=my-vm",{{.UID}}=123sudo ignite ps --filter "{{.Name}}=my-vm",{{.Labels.mylabel}}=123Fixes #228