karmada-search: support field selector for corev1 resources #5801

SataQiu · 2024-11-11T06:18:43Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
karmada-search: support field selector for corev1 resources.

This supports most scenarios that use fieldSelector, such as finding a list of pods by node name.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
without this patch:

 # kubectl get pod --kubeconfig karmada-search.config --field-selector="spec.nodeName=member1-worker"
Error from server (BadRequest): Unable to find "/v1, Resource=pods" that match label selector "", field selector "spec.nodeName=member1-worker": "spec.nodeName" is not a known field selector: only "metadata.name", "metadata.namespace"

with this patch:

 # kubectl get pod --kubeconfig karmada-search.config --field-selector="spec.nodeName=member1-worker"
example-app-6c847589df-xmjm5                2024-10-29T03:15:25Z
...

Does this PR introduce a user-facing change?:

`karmada-search`: Support field selector for corev1 resources.

karmada-bot · 2024-11-11T06:18:54Z

Welcome @SataQiu! It looks like this is your first PR to karmada-io/karmada 🎉

codecov-commenter · 2024-11-11T06:42:44Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 26.74419% with 63 lines in your changes missing coverage. Please review.

Project coverage is 43.16%. Comparing base (d8b58c4) to head (c9e6ce5).
Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
...framework/plugins/cache/apis/core/v1/conversion.go	22.22%	50 Missing and 13 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5801      +/-   ##
==========================================
+ Coverage   43.00%   43.16%   +0.15%     
==========================================
  Files         656      658       +2     
  Lines       55888    56006     +118     
==========================================
+ Hits        24034    24173     +139     
+ Misses      30308    30263      -45     
- Partials     1546     1570      +24

Flag	Coverage Δ
unittests	`43.16% <26.74%> (+0.15%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

XiShanYongYe-Chang · 2024-11-11T07:03:46Z

Cool! Thanks~
/assign

XiShanYongYe-Chang · 2024-11-11T08:11:11Z

pkg/search/proxy/framework/plugins/cache/scheme.go

+var cacheScheme = runtime.NewScheme()
+
+func init() {
+	// This is to ensure that the cache plugin can handle the field selectors for these resources.


Resources that are not listed here can't be processed, can they? So how do you determine which resources can be listed here? In other words, are there any plans to add resources that are not listed?

Thanks @XiShanYongYe-Chang I checked the resource kinds of Kubernetes and the corev1 group is the most important and the most common used. Most definitions of fieldSelector are in the corev1 group. IMO, we should support the corev1 resource kinds first, and the other resource types can be gradually added based on community feedback.

In other words, are there any plans to add resources that are not listed?

There are two ways to handle this:

According to users feedback, support more resource types, such as apps, these can be added in subsequent PRs

Waiting for the Kubernetes runtime scheme package to support automatic ConversionFuncs registration, the community is trying to remove the legacyscheme

@SataQiu, thanks for your quick reply!

Where does the Kubernetes community maintain Corev1 resources?

Waiting for the Kubernetes runtime scheme package to support automatic ConversionFuncs registration, the community is trying to remove the legacyscheme

Is the Kuberenetes community working on this plan? Do we have to rely on this plan to add resources in the future?

Where does the Kubernetes community maintain Corev1 resources?

https://github.com/kubernetes/kubernetes/blob/8fe10dc378b7cc3b077b83aef86622e1019302d5/pkg/apis/core/v1/conversion.go#L34-L104

Is the Kuberenetes community working on this plan? Do we have to rely on this plan to add resources in the future?

We don't have to rely on community plan.

Let me understand method two again, which means that when the upstream community implements this, other resources will automatically support it, and we don't need to register additional conversion functions. Am I right?

Yes, you understand correctly.

Thanks~

According to users feedback, support more resource types, such as apps, these can be added in subsequent PRs

I think we can do what you say. In addition, can you help create an issue to track this task, and if we don't get feedback on other resources for a long time, we may be able to proactively plan in the future.

Would it be better if we added a comment to the file stating that the implementation is referenced from xxx?

Good idea, updated.

XiShanYongYe-Chang

Thanks a lot~

XiShanYongYe-Chang · 2024-11-12T02:00:29Z

pkg/search/proxy/framework/plugins/cache/scheme.go

+	_ = cacheScheme.AddFieldLabelConversionFunc(corev1.SchemeGroupVersion.WithKind("Event"),
+		func(label, value string) (string, string, error) {
+			switch label {
+			case "involvedObject.kind",
+				"involvedObject.namespace",
+				"involvedObject.name",
+				"involvedObject.uid",
+				"involvedObject.apiVersion",
+				"involvedObject.resourceVersion",
+				"involvedObject.fieldPath",
+				"reason",
+				"reportingComponent",
+				"source",
+				"type",
+				"metadata.namespace",
+				"metadata.name":
+				return label, value, nil
+			default:
+				return "", "", fmt.Errorf("field label not supported: %s", label)
+			}
+		},
+	)
+	_ = cacheScheme.AddFieldLabelConversionFunc(corev1.SchemeGroupVersion.WithKind("Namespace"),
+		func(label, value string) (string, string, error) {
+			switch label {
+			case "status.phase",
+				"metadata.name":
+				return label, value, nil
+			default:
+				return "", "", fmt.Errorf("field label not supported: %s", label)
+			}
+		},
+	)
+	_ = cacheScheme.AddFieldLabelConversionFunc(corev1.SchemeGroupVersion.WithKind("Secret"),
+		func(label, value string) (string, string, error) {
+			switch label {
+			case "type",
+				"metadata.namespace",
+				"metadata.name":
+				return label, value, nil
+			default:
+				return "", "", fmt.Errorf("field label not supported: %s", label)
+			}
+		},
+	)


For these methods, can we directly import Kubernetes open functions?

Emm... I guess we can't, because these functions are defined in the k/k repository, but we can't import it directly, which is not recommended.

Okay, I get it. It doesn't exit in the https://github.com/kubernetes/api repo.

XiShanYongYe-Chang · 2024-11-12T02:02:17Z

pkg/search/proxy/framework/plugins/cache/scheme.go

+	_ = cacheScheme.AddFieldLabelConversionFunc(corev1.SchemeGroupVersion.WithKind("Service"),
+		func(label, value string) (string, string, error) {
+			switch label {
+			case "metadata.namespace",
+				"metadata.name",
+				"spec.clusterIP",
+				"spec.type":
+				return label, value, nil
+			default:
+				return "", "", fmt.Errorf("field label not supported: %s", label)
+			}
+		},
+	)


I don't seem to find any service code in the Kubernetes repository. Can you point it out?

https://github.com/kubernetes/kubernetes/blob/6b031e50b2abfe15434beef6c7e01056091a26ea/pkg/apis/core/v1/conversion.go#L494-L507

Although the AddFieldLabelConversionsForService function is exported, we can't use it because we can't import k8s.io/kubernetes/... directly.

https://github.com/kubernetes/kubernetes/tree/master?tab=readme-ov-file#to-start-using-k8s

To use Kubernetes code as a library in other applications, see the list of published components. Use of the k8s.io/kubernetes module or k8s.io/kubernetes/... packages as libraries is not supported.

Oh, I can see why I can't find it, it hasn't existed since version 1.31 and my code is still in v1.28.

XiShanYongYe-Chang · 2024-11-12T07:40:28Z

Thanks~
/lgtm
/cc @yanfeng1992 @RainbowMango

yanfeng1992 · 2024-11-12T08:13:13Z

lgtm

SataQiu · 2024-11-13T02:44:58Z

@XiShanYongYe-Chang need approve, PTAL!

XiShanYongYe-Chang · 2024-11-13T02:56:16Z

@XiShanYongYe-Chang need approve, PTAL!

Let's wait for a review from @RainbowMango

Hi @SataQiu, are you using Karmada now?

RainbowMango · 2024-11-13T11:01:00Z

/assign
But hoping the author @ikaven1024 could take a look.

ikaven1024 · 2024-11-13T11:55:18Z

Just for an advice, we can organize code in kubernetes style:

cache
├── apis
│   └── core
│       └── v1
│           ├── conversion.go
│           └── register.go
├── cache.go
├── cache_test.go
└── scheme.go

scheme.go:

package cache

import (
	corev1 "github.com/karmada-io/karmada/pkg/search/proxy/framework/plugins/cache/apis/core/v1"
	"k8s.io/apimachinery/pkg/runtime"
	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
)

var cacheScheme = runtime.NewScheme()

func init() {
	AddToScheme(cacheScheme)
}

func AddToScheme(scheme *runtime.Scheme) {
	utilruntime.Must(corev1.AddToScheme(scheme))
}

apis/core/v1/register.go:

package v1

import (
	corev1 "k8s.io/api/core/v1"
	"k8s.io/apimachinery/pkg/runtime"
)

var SchemeGroupVersion = corev1.SchemeGroupVersion

var (
	SchemeBuilder = runtime.NewSchemeBuilder(addConversionFuncs)
	AddToScheme   = SchemeBuilder.AddToScheme
)

conversion.go:

// copy it from kubernetes
func addConversionFuncs(scheme *runtime.Scheme) error {
   ...
}

Signed-off-by: SataQiu <[email protected]>

SataQiu · 2024-11-13T14:04:32Z

ping @ikaven1024

ikaven1024

/lgtm

RainbowMango

/approve

karmada-bot · 2024-11-14T01:43:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango, yanfeng1992

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/search/OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 11, 2024

karmada-bot requested review from XiShanYongYe-Chang and yanfeng1992 November 11, 2024 06:18

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 11, 2024

SataQiu force-pushed the fieldSelector branch 2 times, most recently from 12f1f88 to 774d133 Compare November 11, 2024 06:28

karmada-bot assigned XiShanYongYe-Chang Nov 11, 2024

XiShanYongYe-Chang reviewed Nov 11, 2024

View reviewed changes

XiShanYongYe-Chang reviewed Nov 12, 2024

View reviewed changes

SataQiu force-pushed the fieldSelector branch from 774d133 to 3876d97 Compare November 12, 2024 06:19

karmada-bot requested a review from RainbowMango November 12, 2024 07:40

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 12, 2024

yanfeng1992 approved these changes Nov 12, 2024

View reviewed changes

karmada-bot assigned RainbowMango Nov 13, 2024

SataQiu force-pushed the fieldSelector branch from 3876d97 to 056daa5 Compare November 13, 2024 13:37

karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2024

SataQiu force-pushed the fieldSelector branch from 056daa5 to 84816c8 Compare November 13, 2024 13:47

karmada-search: support field selector for corev1 resources

c9e6ce5

Signed-off-by: SataQiu <[email protected]>

SataQiu force-pushed the fieldSelector branch from 84816c8 to c9e6ce5 Compare November 13, 2024 13:48

ikaven1024 reviewed Nov 13, 2024

View reviewed changes

karmada-bot assigned ikaven1024 Nov 13, 2024

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2024

RainbowMango approved these changes Nov 14, 2024

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 14, 2024

karmada-bot merged commit 6f138cf into karmada-io:master Nov 14, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

karmada-search: support field selector for corev1 resources #5801

karmada-search: support field selector for corev1 resources #5801

SataQiu commented Nov 11, 2024 •

edited by RainbowMango

Loading

karmada-bot commented Nov 11, 2024

codecov-commenter commented Nov 11, 2024 •

edited

Loading

XiShanYongYe-Chang commented Nov 11, 2024

XiShanYongYe-Chang Nov 11, 2024

SataQiu Nov 11, 2024 •

edited

Loading

SataQiu Nov 11, 2024

XiShanYongYe-Chang Nov 11, 2024

SataQiu Nov 11, 2024

XiShanYongYe-Chang Nov 12, 2024

SataQiu Nov 12, 2024

XiShanYongYe-Chang Nov 12, 2024

XiShanYongYe-Chang Nov 12, 2024

SataQiu Nov 12, 2024

XiShanYongYe-Chang left a comment

XiShanYongYe-Chang Nov 12, 2024

SataQiu Nov 12, 2024

XiShanYongYe-Chang Nov 12, 2024

XiShanYongYe-Chang Nov 12, 2024

SataQiu Nov 12, 2024

SataQiu Nov 12, 2024 •

edited

Loading

XiShanYongYe-Chang Nov 12, 2024

XiShanYongYe-Chang commented Nov 12, 2024

yanfeng1992 commented Nov 12, 2024

SataQiu commented Nov 13, 2024

XiShanYongYe-Chang commented Nov 13, 2024

RainbowMango commented Nov 13, 2024

ikaven1024 commented Nov 13, 2024

SataQiu commented Nov 13, 2024

ikaven1024 left a comment

RainbowMango left a comment

karmada-bot commented Nov 14, 2024

karmada-search: support field selector for corev1 resources #5801

karmada-search: support field selector for corev1 resources #5801

Conversation

SataQiu commented Nov 11, 2024 • edited by RainbowMango Loading

karmada-bot commented Nov 11, 2024

codecov-commenter commented Nov 11, 2024 • edited Loading

Codecov Report

XiShanYongYe-Chang commented Nov 11, 2024

Choose a reason for hiding this comment

SataQiu Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SataQiu Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiShanYongYe-Chang commented Nov 12, 2024

yanfeng1992 commented Nov 12, 2024

SataQiu commented Nov 13, 2024

XiShanYongYe-Chang commented Nov 13, 2024

RainbowMango commented Nov 13, 2024

ikaven1024 commented Nov 13, 2024

SataQiu commented Nov 13, 2024

ikaven1024 left a comment

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Nov 14, 2024

SataQiu commented Nov 11, 2024 •

edited by RainbowMango

Loading

codecov-commenter commented Nov 11, 2024 •

edited

Loading

SataQiu Nov 11, 2024 •

edited

Loading

SataQiu Nov 12, 2024 •

edited

Loading