Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add object search controller #190

Merged
merged 5 commits into from
Sep 24, 2021

Conversation

EnfangCui
Copy link
Contributor

@EnfangCui EnfangCui commented Sep 9, 2021

  • Add the object search controller to GM

The user worker and ReID workers can be created on both cloud and edge. ( EdgeMesh v1.8 supports cross edge-cloud communication.)

@kubeedge-bot kubeedge-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 9, 2021
objectSearchUserWorker = "userworker"
objectSearchTrackingWorker = "trackingworker"
objectSearchReidWorker = "reidworker"
reidPort = 9378
Copy link
Contributor

Choose a reason for hiding this comment

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

it means reid service port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I changed it to reidServicePort.

c.enqueueController(service, immediate)
}

//When a deployment is created, enqueue the controller that manages it and update it's expectations.
Copy link
Contributor

Choose a reason for hiding this comment

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

leave a space after '//'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

c.enqueueByDeployment(deployment, true)
}

//deleteDeployment enqueues the ObjectSearchService obj When a deleteDeployment is deleted
Copy link
Contributor

Choose a reason for hiding this comment

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

same problem

var reason string
var message string

if podFailed > 0 || deploymentFailed > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

might can use switch-case instead of if-else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to switch-case.

"github.com/kubeedge/sedna/pkg/globalmanager/runtime"
)

// func (c *Controller) syncToEdge(eventType watch.EventType, obj interface{}) error {
Copy link

Choose a reason for hiding this comment

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

Remove these comment codes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and add TODO tag.

Copy link

Choose a reason for hiding this comment

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

Look like commented code not removed

// return c.sendToEdgeFunc(nodeName, eventType, joint)
// }

func (c *Controller) SetDownstreamSendFunc(f runtime.DownstreamSendFunc) error {
Copy link

Choose a reason for hiding this comment

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

Add some comment or TODO to explain that no need sync the object search service info to edge nodes(i.e. Local Controller).

*/

// updateFromEdge syncs the edge updates to k8s
func (c *Controller) updateFromEdge(name, namespace, operation string, content []byte) error {
Copy link

Choose a reason for hiding this comment

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

updateFromEdge can not be called since no update from LC.
Recommended that remove this untested implemention, and add a TODO comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and add TODO comment.

@@ -170,6 +180,107 @@ func CreatePodWithTemplate(client kubernetes.Interface, object CommonInterface,
return createdPod, nil
}

//CreateEdgeMeshService creates a kubeedge edgemesh service for an object, and returns an edgemesh service URL.
//Since edgemesh can realize Cross-Edge-Cloud communication, the service can be created both on the cloud or edge side.
func CreateEdgeMeshService(kubeClient kubernetes.Interface, object CommonInterface, workerType string, inputPort int32) (string, error) {
Copy link

Choose a reason for hiding this comment

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

What is the difference between CreateEdgeMeshService and CreateKubernetesService? I can't see it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are three main differences:

  1. ObjectMeta uses Name instead of GenerateName because it relates to the URL returned.

  2. A fixed format URL needs to be returned.

  3. Port Name is required.

Copy link

Choose a reason for hiding this comment

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

OK. You can extend CreateKubernetesService to support these differences.

@@ -170,6 +180,107 @@ func CreatePodWithTemplate(client kubernetes.Interface, object CommonInterface,
return createdPod, nil
}

//CreateEdgeMeshService creates a kubeedge edgemesh service for an object, and returns an edgemesh service URL.
Copy link

Choose a reason for hiding this comment

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

Add extra a space after //, same with other places.

Suggested change
//CreateEdgeMeshService creates a kubeedge edgemesh service for an object, and returns an edgemesh service URL.
// CreateEdgeMeshService creates a kubeedge edgemesh service for an object, and returns an edgemesh service URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

@EnfangCui EnfangCui force-pushed the add-object-search-controller branch from b6dbaf4 to 76c79d1 Compare September 22, 2021 07:52
@EnfangCui EnfangCui force-pushed the add-object-search-controller branch from 394de29 to e25fe23 Compare September 23, 2021 14:20
@EnfangCui EnfangCui force-pushed the add-object-search-controller branch from e25fe23 to ff28a1e Compare September 23, 2021 14:41
@llhuii
Copy link

llhuii commented Sep 24, 2021

/lgtm

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 24, 2021
@llhuii
Copy link

llhuii commented Sep 24, 2021

/approve
Thanks @EnfangCui

@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: llhuii

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:

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

@kubeedge-bot kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 24, 2021
@kubeedge-bot kubeedge-bot merged commit 3d88d0d into kubeedge:main Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants