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.

Sign up for GitHub

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

Jump to bottom

support module restarting feature #5513

Merged
merged 20 commits into from
Apr 23, 2024
Merged

support module restarting feature #5513

merged 20 commits into from
Apr 23, 2024

Conversation

micplus
Copy link
Contributor

@micplus micplus commented Apr 7, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

To achieve the goal of module restarting on edge site.

  1. Modify BeeHive to support restarting for local modules
  2. Add feature gate moduleRestart
  3. Remove additional goroutine of MetaManager's main loop
  4. Modify edged.syncPod to support early exit
  5. Enable module-restarting in edgecore's main function

Which issue(s) this PR fixes:

Fixes #5134

Special notes for your reviewer:

Alpha version

Does this PR introduce a user-facing change?:

NONE

micplus and others added 13 commits April 7, 2024 14:17
@micplus
feat: module auto restart feature
d514068 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: add feature-gate for module restarting
016f639 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: remove metamanger main loop goroutine
9e9cb45 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: improve localModuleKeeper
26ae906 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: edged auto restart support
082a7f8 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: edged compatibility
fb97bb8 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
chore: modify comments
57401e7 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: improve edged init logic
2067da5 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: add feature
52cfef8 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: remove unnecessary msg
09d2b29 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
chore: correct log
1997ac6 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
feat: modify syncPod starting logic
2e96f0f 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
chore: remove unused var
85ac98b 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus micplus changed the title Feat/module restart support module restarting feature Apr 7, 2024
@Shelley-BaoYue Shelley-BaoYue added this to the v1.17 milestone Apr 10, 2024
@micplus
refactor: improve code & go fmt
bf3683f 
Signed-off-by: micplus <ain01a@outlook.com>
@kubeedge-bot kubeedge-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 11, 2024
Shelley-BaoYue
Shelley-BaoYue reviewed Apr 11, 2024
View reviewed changes
edge/cmd/edgecore/app/server.go Outdated Show resolved Hide resolved
edge/pkg/edged/edged.go Outdated Show resolved Hide resolved
staging/src/github.com/kubeedge/beehive/pkg/core/core.go Show resolved Hide resolved
@micplus
fix: duplicate import
0368ccd 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
fix: remove drain channel operation
b73a4ac 
Signed-off-by: micplus <ain01a@outlook.com>
wbc6080
wbc6080 reviewed Apr 12, 2024
View reviewed changes
staging/src/github.com/kubeedge/beehive/pkg/core/module.go Outdated
@@ -30,6 +32,7 @@ func init() {
type ModuleInfo struct {
contextType string
remote bool
active bool
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the significance of the newly added active field? Does it mean that a module was started normally?

Copy link
Contributor Author

@micplus micplus Apr 12, 2024 •
edited

Choose a reason for hiding this comment

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

Yes, it means a module is started and running normally. In this PR it's true if module.Start() doesn't exits. It's for further implementation to check whether a module is online/active.

staging/src/github.com/kubeedge/beehive/pkg/core/core.go Outdated Show resolved Hide resolved
fisherxu
fisherxu reviewed Apr 22, 2024
View reviewed changes
edge/pkg/metamanager/process.go Show resolved Hide resolved
staging/src/github.com/kubeedge/beehive/pkg/core/core.go
if backoffDuration < 30*time.Second {
backoffDuration *= 2
if backoffDuration > 30*time.Second {
backoffDuration = 30 * time.Second
Copy link
Member

Choose a reason for hiding this comment

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

We need to set the Max retry time, so process can exit after that, and the backoffDuration also should be configurable for users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can make it configurable in the next phase of development.

Now it's a feature in development. So we haven't add configurations yet.

staging/src/github.com/kubeedge/beehive/pkg/core/core.go Outdated Show resolved Hide resolved
edge/pkg/edged/edged.go Outdated Show resolved Hide resolved
fisherxu
fisherxu reviewed Apr 22, 2024
View reviewed changes
edge/pkg/edged/edged.go Show resolved Hide resolved
edge/pkg/edged/edged.go Outdated Show resolved Hide resolved
edge/pkg/edged/edged.go Outdated Show resolved Hide resolved
fisherxu
fisherxu reviewed Apr 22, 2024
View reviewed changes
Copy link
Member

@fisherxu fisherxu left a comment

Choose a reason for hiding this comment

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

Overall looks good, thanks for the great work @micplus @ttlv

@micplus
chore: remove unneccesary of sync pod loop
784d508 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
style: move pre-conditions of syncpod to outside
28fd6f7 
Signed-off-by: micplus <ain01a@outlook.com>
@micplus
refactor: remove unnecessary var
76aefa5 
Signed-off-by: micplus <ain01a@outlook.com>
fisherxu
fisherxu reviewed Apr 22, 2024
View reviewed changes
edge/pkg/edged/edged.go Outdated
@@ -221,18 +242,18 @@ func newEdged(enable bool, nodeName, namespace string) (*edged, error) {
}

func (e *edged) syncPod(podCfg *config.PodConfig) {
time.Sleep(10 * time.Second)

Copy link
Member

Choose a reason for hiding this comment

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

Delete this line to fix the lint error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry, I will fixt it now

@micplus
fix: lint: unnecessary leading newline
39d2829 
Signed-off-by: micplus <ain01a@outlook.com>
fisherxu
fisherxu reviewed Apr 23, 2024
View reviewed changes
Copy link
Member

@fisherxu fisherxu left a comment

Choose a reason for hiding this comment

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

/approve

@kubeedge-bot
Copy link
Collaborator

[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

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 Apr 23, 2024
Shelley-BaoYue
Shelley-BaoYue reviewed Apr 23, 2024
View reviewed changes
Copy link
Collaborator

@Shelley-BaoYue Shelley-BaoYue left a comment

Choose a reason for hiding this comment

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

Thanks for your contributions!
/lgtm

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 23, 2024
@kubeedge-bot kubeedge-bot merged commit 574f5d2 into kubeedge:master Apr 23, 2024
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fisherxu fisherxu fisherxu left review comments

@wbc6080 wbc6080 wbc6080 left review comments

@Shelley-BaoYue Shelley-BaoYue Shelley-BaoYue left review comments

Assignees
No one assigned
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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.17
Development

Successfully merging this pull request may close these issues.

Support auto-restarting for edge-site modules
5 participants
@micplus @kubeedge-bot @fisherxu @wbc6080 @Shelley-BaoYue