Support offline-operations of data surface resources by dcdc831 · Pull Request #5901 · kubeedge/kubeedge

dcdc831 · 2024-09-29T11:11:01Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

In edge computing scenarios, edge nodes are usually under different networks from the cloud and are often offline due to network fluctuations and other issues. If the edge node is offline, users can only wait for the edge node to come online to perform operations from the cloud if they find any abnormality in the pod or device.

However, the data related to pods and devices are stored locally on the edge nodes, and due to the existence of edged, it is feasible in principle to obtain the data and operate the resources on the edge nodes. The support of this feature will reduce the network resource consumption of the edge node for debugging and resource adjustment, and avoid the situation that some functions of the KubeEdge platform are not available when the edge node is offline.

Which issue(s) this PR fixes:

Fixes #5587

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

wbc6080

Need to make verify CI check pass. You can refer to https://kubeedge.io/docs/developer/make_verify

wbc6080 · 2024-10-14T12:57:57Z

+	"github.com/kubeedge/kubeedge/edge/pkg/edged/config"
+	"github.com/kubeedge/kubeedge/edge/pkg/metamanager/metaserver/common"


Sort the import. Packages starting with github.com/kubeedge should be placed in the third part

wbc6080 · 2024-10-14T13:00:05Z

+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/common"
+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/ctl/client"
+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/util"
+	"github.com/spf13/cobra"


remove github.com/spf13/cobra to the second part

wbc6080 · 2024-10-14T13:01:03Z

+			if device.Spec.NodeName == nodeName {
+				deviceListFilter.Items = append(deviceListFilter.Items, *device)
+			} else {
+				fmt.Printf("can't query device: \"%s\" for node: \"%s\"\n", device.Name, device.Spec.NodeName)


Use klog instead of fmt. Please check the rest of the code as well

wbc6080 · 2024-10-14T13:03:05Z

+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/common"
+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/ctl/client"
+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/util"
+	"github.com/spf13/cobra"


wbc6080 · 2024-10-14T13:03:22Z

+	"github.com/moby/term"
+	"github.com/spf13/cobra"


wbc6080 · 2024-10-14T13:03:40Z

+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/ctl/client"
+	"github.com/spf13/cobra"


luomengY · 2024-10-15T01:10:57Z

These interfaces need to add tests

dcdc831 · 2024-10-15T02:42:56Z

Need to make verify CI check pass. You can refer to https://kubeedge.io/docs/developer/make_verify

Thank you for your suggestion. I will sort the import and run make verify before commit.

wbc6080

Now that #5894 has been merged, it should be able to get the correct DeviceList via MetaServer's API. You can rebase the latest code for testing.
@dcdc831

dcdc831 · 2024-10-25T02:37:18Z

I pushed the latest commit with the following major changes:

Added tests for the MetaServer interfaces of exec and logs;
Added a test for MetaManager send messages to DeviceTwin.
Sort non-standard imports.
Change fmt to klog in the CLI tool.
Rebase the latest code.
run make lint and make verify before push.

luomengY · 2024-10-12T03:45:42Z

 				ls.Factory.Delete().ServeHTTP(w, req)
 			case reqInfo.Verb == "update":
-				ls.Factory.Update(reqInfo).ServeHTTP(w, req)
+				if reqInfo.Resource == "devices" {


Device is a type of CRD, and K8S APIServer has a unified API interface. It is not appropriate to exhaustively list specific CRD types in this way.

luomengY · 2024-10-25T03:37:15Z

+			}
+
+			if err := scanner.Err(); err != nil {
+				http.Error(w, fmt.Sprintf("Error reading logs: %v", err), http.StatusInternalServerError)


Here err != nil, don't it need a return?

luomengY · 2024-10-28T01:13:38Z

+	Stdin     bool
+	Stdout    bool
+	Stderr    bool
+	TTY       bool


What is TTY? Please add code comments.

luomengY · 2024-10-28T01:16:03Z

+
+		exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL())
+		if err != nil {
+			return nil, fmt.Errorf("fail to create executor: %v", err)


Suggested change

return nil, fmt.Errorf("fail to create executor: %v", err)

return nil, fmt.Errorf("failed to create executor: %v", err)

luomengY · 2024-10-28T01:17:00Z

+		if cases.isGetLogsFail {
+			return &types.LogsResponse{
+				LogMessages: []string{},
+				ErrMessages: []string{"fail to get logs for container"},


Suggested change

ErrMessages: []string{"fail to get logs for container"},

ErrMessages: []string{"failed to get logs for container"},

luomengY · 2024-10-28T01:20:32Z

+	return req
+}
+
+func disableEcho(fd uintptr) (func(), error) {


Please add function comments

luomengY · 2024-10-28T01:24:05Z

+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/util"
+)
+
+var edgeDescribeDeviceShortDescription = `Describe devices in edge node`


Suggested change

var edgeDescribeDeviceShortDescription = `Describe devices in edge node`

var edgeDescribeDeviceShortDescription = `Describe device in edge node`

luomengY · 2024-10-28T01:25:00Z

+	"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/util"
+)
+
+var edgeDescribePodShortDescription = `Describe pods in edge node`


Suggested change

var edgeDescribePodShortDescription = `Describe pods in edge node`

var edgeDescribePodShortDescription = `Describe pod in edge node`

luomengY · 2024-10-28T01:30:32Z

+			}
+			pod, err := podRequest.GetPod(ctx)
+			if err != nil {
+				klog.Info(err.Error())


Why klog.Info() ,not klog.Error() ?

luomengY · 2024-10-28T01:57:07Z

+				err := json.Unmarshal(body, &logsResponse)
+				assert.NoError(t, err)
+				expectedResponse := types.LogsResponse{}
+				json.Unmarshal([]byte(tt.expectedBody), &expectedResponse)


Suggested change

json.Unmarshal([]byte(tt.expectedBody), &expectedResponse)

err := json.Unmarshal([]byte(tt.expectedBody), &expectedResponse)

....

Please add error handling.

luomengY · 2024-10-28T07:01:23Z

+		stderrStr := queryparams.Get("stderr")
+		ttyStr := queryparams.Get("tty")
+
+		stdin := (stdinStr == trueStr)


Suggested change

stdin := (stdinStr == trueStr)

stdin := stdinStr == trueStr

luomengY · 2024-10-28T07:01:36Z

+		ttyStr := queryparams.Get("tty")
+
+		stdin := (stdinStr == trueStr)
+		stdout := (stdoutStr == trueStr)


Suggested change

stdout := (stdoutStr == trueStr)

stdout := stdoutStr == trueStr

luomengY · 2024-10-28T07:01:50Z

+
+		stdin := (stdinStr == trueStr)
+		stdout := (stdoutStr == trueStr)
+		stderr := (stderrStr == trueStr)


Suggested change

stderr := (stderrStr == trueStr)

stderr := stderrStr == trueStr

luomengY · 2024-10-28T07:02:03Z

+		stdin := (stdinStr == trueStr)
+		stdout := (stdoutStr == trueStr)
+		stderr := (stderrStr == trueStr)
+		tty := (ttyStr == trueStr)


Suggested change

tty := (ttyStr == trueStr)

tty := ttyStr == trueStr

luomengY · 2024-10-28T07:06:09Z

+	CoreAPIPrefix       = "api"
+	CoreAPIGroupVersion = schema.GroupVersion{Group: "", Version: "v1"}
+	Prefix              = "apis"
+)


Delete these unused constants.

luomengY · 2024-10-28T07:17:58Z

+package restful
+
+const (
+	CONTAINERLOGS string = "/containerLogs"


Suggest deleting this file:
edge/pkg/metamanager/metaserver/kubernetes/storage/restful/constants.go，Put CONTAINERLOGS string = "/containerLogs" into edge/pkg/metamanager/metaserver/kubernetes/storage/restful/request.go.

Shelley-BaoYue · 2024-10-29T03:12:30Z

+	Timestamps                   string `query:"timestamps"`
+}
+
+type ExecInfo struct {


Why param. in ExecInfo don't need the format query:"xxx" like LogsInfo?

The logs is run on the edged restful API, these tags is used to generate query parameters. exec calls the remoteRuntimeService methods (ExecSync and Exec) without generating query parameters.

Shelley-BaoYue · 2024-10-29T03:16:03Z

@@ -0,0 +1,286 @@
+package handlerfactory


Please add copyright for new file

Shelley-BaoYue · 2024-10-29T03:19:09Z

@@ -0,0 +1,96 @@
+/*


Please djust the copyright format, like adding blank line between L5-6 and L6-7

Shelley-BaoYue · 2024-10-29T03:36:37Z

 	case cloudmodules.PolicyControllerModuleName:
 		resp := message.NewRespByMessage(&message, OK)
 		sendToCloud(resp)
+	case modules.MetaManagerModuleName:


It may cause confused using metamanager module here because we already have a module named meta. How about name this module metaserver because it seems the request are from metaserver? but metaserver is not an independent module, I'm not sure if metaserver is suitable. @fisherxu @luomengY @WillardHu @wbc6080

It may cause confused using metamanager module here because we already have a module named meta. How about name this module metaserver because it seems the request are from metaserver? but metaserver is not an independent module, I'm not sure if metaserver is suitable. @fisherxu @luomengY @WillardHu @wbc6080

I think using a metaserver is possible, as the source here comes from the metaserver.

It may cause confused using metamanager module here because we already have a module named meta. How about name this module metaserver because it seems the request are from metaserver? but metaserver is not an independent module, I'm not sure if metaserver is suitable. @fisherxu @luomengY @WillardHu @wbc6080

@WillardHu @fisherxu

I think using a metaserver is possible, as the source here comes from the metaserver.

+1

OK, I added and registered metaserver module, and set the msgsource to metaserver.

Shelley-BaoYue · 2024-10-29T03:40:38Z

+			if device.Spec.NodeName == nodeName {
+				deviceListFilter.Items = append(deviceListFilter.Items, *device)
+			} else {
+				klog.Infof("can't query device: \"%s\" for node: \"%s\"", device.Name, device.Spec.NodeName)


Suggested change

klog.Infof("can't query device: \"%s\" for node: \"%s\"", device.Name, device.Spec.NodeName)

klog.Errof("can't query device: \"%s\" for node: \"%s\"", device.Name, device.Spec.NodeName)

Shelley-BaoYue · 2024-10-29T03:45:41Z

+		return fmt.Errorf("unable to find describer for %v", m)
+	}
+
+	first := true


What's the actual effect of first?

describe supports printing information about multiple resources. If the first is false, blank lines is printed to separate resources.

Shelley-BaoYue · 2024-10-29T03:49:16Z

+			if pod.Spec.NodeName == nodeName {
+				var apiPod api.Pod
+				if err := k8s_v1_api.Convert_v1_Pod_To_core_Pod(pod, &apiPod, nil); err != nil {
+					klog.Infof("failed to convert pod with err:%v\n", err)


Suggested change

klog.Infof("failed to convert pod with err:%v\n", err)

klog.Errof("failed to convert pod with err:%v\n", err)

There are several other places using Infof but not Errof. What's the consideration?

dcdc831 · 2024-10-29T08:18:15Z

I have some questions about the results of integrationtest. The error occurred in eventbus test, and I did not change eventbus code.

But I executed the 'make Integrationtest' in my local development environment and found that the above error was possible. There are also cases where the test was successfully passed:

go version and ginkgo version:

go version: 1.21.13
Ginkgo CLI Version: 2.9.5 （Then I run make integrationtest several times with Ginkgo CLI Version:2.20.2, this error seems not happening again.）

Shelley-BaoYue · 2024-10-29T11:45:23Z

@dcdc831 integration test error is not introduced by this PR. You can ignore it.

Signed-off-by: YulinLi <1826549123@qq.com>

dcdc831 · 2024-10-30T09:06:16Z

I undo the changes to metaserver and keep the msgsource that sends information to devicetwin as metamanager.

Shelley-BaoYue

/lgtm
CIFuzz error can be ignored in this PR, the viaduct migration caused this error.

Shelley-BaoYue · 2024-10-30T10:26:41Z

/assign @fisherxu @WillardHu @luomengY

fisherxu · 2024-10-30T10:34:54Z

/approve
Thanks for @luomengY @dcdc831 your great work!

kubeedge-bot · 2024-10-30T10:35:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fisherxu

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [fisherxu]

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

kubeedge-bot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Sep 29, 2024

kubeedge-bot requested review from WillardHu, fisherxu, gy95 and kadisi September 29, 2024 11:11

kubeedge-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 29, 2024

wbc6080 reviewed Oct 14, 2024

View reviewed changes

wbc6080 reviewed Oct 23, 2024

View reviewed changes

dcdc831 force-pushed the offline-data-control branch from 2ce68e4 to 7092f6b Compare October 25, 2024 02:29

dcdc831 changed the title ~~WIP: Support offline-operations of data surface resources~~ Support offline-operations of data surface resources Oct 25, 2024

kubeedge-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 25, 2024

dcdc831 force-pushed the offline-data-control branch from 7092f6b to 408146a Compare October 26, 2024 08:24

kubeedge-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 26, 2024

dcdc831 closed this Oct 28, 2024

dcdc831 reopened this Oct 28, 2024

luomengY reviewed Oct 28, 2024

View reviewed changes

dcdc831 force-pushed the offline-data-control branch from 408146a to a65cf88 Compare October 28, 2024 08:58

dcdc831 closed this Oct 29, 2024

dcdc831 reopened this Oct 29, 2024

Shelley-BaoYue reviewed Oct 29, 2024

View reviewed changes

Support offline-operations of data surface resources

63ed57c

Signed-off-by: YulinLi <1826549123@qq.com>

dcdc831 force-pushed the offline-data-control branch 3 times, most recently from 3ddcad0 to 63ed57c Compare October 30, 2024 08:57

Shelley-BaoYue reviewed Oct 30, 2024

View reviewed changes

kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2024

kubeedge-bot assigned fisherxu, luomengY and WillardHu Oct 30, 2024

kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 30, 2024

kubeedge-bot merged commit e3827d7 into kubeedge:master Oct 30, 2024

Shelley-BaoYue added this to the v1.20 milestone Nov 11, 2024

luomengY mentioned this pull request Nov 26, 2024

Optimize the logic of edge update devices #5979

Open

		"github.com/kubeedge/kubeedge/edge/pkg/edged/config"
		"github.com/kubeedge/kubeedge/edge/pkg/metamanager/metaserver/common"

		"github.com/kubeedge/kubeedge/keadm/cmd/keadm/app/cmd/ctl/client"
		"github.com/spf13/cobra"

	return nil, fmt.Errorf("fail to create executor: %v", err)
	return nil, fmt.Errorf("failed to create executor: %v", err)

	ErrMessages: []string{"fail to get logs for container"},
	ErrMessages: []string{"failed to get logs for container"},

	var edgeDescribeDeviceShortDescription = `Describe devices in edge node`
	var edgeDescribeDeviceShortDescription = `Describe device in edge node`

	var edgeDescribePodShortDescription = `Describe pods in edge node`
	var edgeDescribePodShortDescription = `Describe pod in edge node`

	json.Unmarshal([]byte(tt.expectedBody), &expectedResponse)
	err := json.Unmarshal([]byte(tt.expectedBody), &expectedResponse)
	....

	stdout := (stdoutStr == trueStr)
	stdout := stdoutStr == trueStr

	stderr := (stderrStr == trueStr)
	stderr := stderrStr == trueStr

	klog.Infof("can't query device: \"%s\" for node: \"%s\"", device.Name, device.Spec.NodeName)
	klog.Errof("can't query device: \"%s\" for node: \"%s\"", device.Name, device.Spec.NodeName)

	klog.Infof("failed to convert pod with err:%v\n", err)
	klog.Errof("failed to convert pod with err:%v\n", err)

Uh oh!

Conversation

dcdc831 commented Sep 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbc6080 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luomengY commented Oct 15, 2024

Uh oh!

dcdc831 commented Oct 15, 2024

Uh oh!

wbc6080 left a comment

Choose a reason for hiding this comment

Uh oh!

dcdc831 commented Oct 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbc6080 Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dcdc831 commented Sep 29, 2024 •

edited

Loading

wbc6080 Oct 30, 2024 •

edited

Loading