Change Prefill and Decode filters to be based on a common filter

[mayabar]

Change Prefill and Decode filters to be based on a common Filter which contains list of valid values in the role label.

These filters cannot be based on ByLabel filter since it uses kubernetes selector which does not support OR between multiple conditions (the logic is: is specific label exists - must have one of the specified values OR the specific label does not defined at all)

Currently P/D schedulers use Prefill and Decode filters, which are filtering pods based on value of the role label.
Remove the old implementation and create these two filters as instances of ByLabel filter

Fixes #16
Supersedes #161

[nirrozenbaum]

I'd be very happy if we can wait with merging this PR few days until the work on #179 is completed.
#179 includes the transition to the latest GIE version and includes a lot of changes which are inevitable.
if possible, I prefer to avoid conflicts as much as possible cause each conflict makes it harder to merge changes and keep it consistent.. I don't like having such big PRs but unfortunately in this case there is no other way to make the transition to the latest GIE version.

[elevran]

missing case of label not being defined at all. Otherwise, LGTM

[mayabar]

@elevran is it a valid situation that there are pods which are not marked with 'llm-d.ai/role' label?
The previous Prefill/Decode filters filtered out such pods too.

[elevran]

@elevran is it a valid situation that there are pods which are not marked with 'llm-d.ai/role' label? The previous Prefill/Decode filters filtered out such pods too.

Decode Pods were allowed when no label was defined. The code and comment on the function were inconsistent and the code had !defined in the set of conditions.

[mayabar]

@shmuelk @elevran - can you please review again
the implementation was changed back to be based on a common filter with list of relevant roles + boolean that defines if missing label is ok. It is not based in ByLabel filter since it cannot support all required logic

[elevran]

approve.

However, note that the filter can be generalized to a cater for the Role based use cases even if it is defined in terms of labels (i.e., not the existing by-label which uses a label-selector).
What you need:

• label name
• allowed values
• what to do when the label is missing

There's no reason to bind the implementation name to "Role". You can create specific instances of the label filter that use the "role" label name just as easily.
I don't want to hold this PR further, but I would prefer it is implemented more generically and not specific to the Role label.

[mayabar]

approve.

However, note that the filter can be generalized to a cater for the Role based use cases even if it is defined in terms of labels (i.e., not the existing by-label which uses a label-selector). What you need:

• label name
• allowed values
• what to do when the label is missing

There's no reason to bind the implementation name to "Role". You can create specific instances of the label filter that use the "role" label name just as easily. I don't want to hold this PR further, but I would prefer it is implemented more generically and not specific to the Role label.

@elevran Agree, current implementation has the label name hard-coded, I'll add it as a parameter and rename to ByLabel
@elevran what do you think about renaming the current ByLabel filter to ByLabelSelector

[elevran]

Suggested change

	// validValuesApp - list of valid values
	func NewByLabel(name string, labelName string, allowsNoLabel bool, validValuesApp ...string) *ByLabel {
	// validValues - list of valid values
	func NewByLabel(name string, labelName string, allowsNoLabel bool, validValues ...string) *ByLabel {

[elevran]

you can change to ByLabelSelector
Also change the doc to use the new name