Skip to content

add IncludesFilter FilterFn#24

Merged
tallaxes merged 3 commits intoAzure:mainfrom
jackfrancis:sku-includes-filter
May 14, 2025
Merged

add IncludesFilter FilterFn#24
tallaxes merged 3 commits intoAzure:mainfrom
jackfrancis:sku-includes-filter

Conversation

@jackfrancis
Copy link
Member

@jackfrancis jackfrancis commented May 2, 2025

This PR adds an Includes() filter to the built-in SKU type (thin wrapper around Azure's SDK track 1 compute.ResourceSku type).

This is an expensive filter as it may have to enumerate through an entire list (unbound in size) of SKUs. Only intended for use in cache-enabled data stores.

We also expose a IncludesFilter FilterFn so that karpenter-provider-azure can call it conveniently from the cache List flow.

Signed-off-by: Jack Francis <jackfrancis@gmail.com>
// Includes returns true if the SKU's name is in the list of SKUs.
func (s *SKU) Includes(skuList []SKU) bool {
for _, sku := range skuList {
if s.GetName() == sku.GetName() {
Copy link

Choose a reason for hiding this comment

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

EqualFold might be safer here, depending on where the list is coming from?

Copy link
Member Author

Choose a reason for hiding this comment

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

The list would be populated thusly:

https://github.com/Azure/skewer/pull/25/files#diff-3cdaf65cd294be89999f768c30e5dedd31813ba12b2f5f05c82cf7ba2eaf90a4R72-R78

And then moved over to the karpenter provider repo for manual curation. I think Name is sufficient if we're going to want to have an easily manageable data structure to maintain.

Signed-off-by: Jack Francis <jackfrancis@gmail.com>
@jackfrancis jackfrancis changed the title add Includes filter to SKU type add IncludesFilter FilterFn May 6, 2025
Signed-off-by: Jack Francis <jackfrancis@gmail.com>
@tallaxes tallaxes merged commit 2121114 into Azure:main May 14, 2025
2 checks passed
@jackfrancis jackfrancis deleted the sku-includes-filter branch May 15, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants