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

[Proposal] add proposal of daemonset update model #921

Merged
merged 3 commits into from
Aug 2, 2022
Merged

[Proposal] add proposal of daemonset update model #921

merged 3 commits into from
Aug 2, 2022

Conversation

xavier-hou
Copy link
Member

Signed-off-by: hxcGit houxc_mail@163.com

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:
/kind bug
/kind documentation
/kind enhancement
/kind good-first-issue
/kind feature
/kind question
/kind design
/sig ai
/sig iot
/sig network
/sig storage
/sig storage

/kind design

What this PR does / why we need it:

Optimize workload update model in cloud-edge collaboration scenarios

Which issue(s) this PR fixes:

#914

Special notes for your reviewer:

/assign @rambohe-ch

Does this PR introduce a user-facing change?

other Note

@openyurt-bot
Copy link
Collaborator

@HXCGIT: GitHub didn't allow me to assign the following users: your_reviewer.

Note that only openyurtio members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

Signed-off-by: hxcGit houxc_mail@163.com

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:
/kind bug
/kind documentation
/kind enhancement
/kind good-first-issue
/kind feature
/kind question
/kind design
/sig ai
/sig iot
/sig network
/sig storage
/sig storage

/kind design

What this PR does / why we need it:

Optimize workload update model in cloud-edge collaboration scenarios

Which issue(s) this PR fixes:

#914

Special notes for your reviewer:

/assign @rambohe-ch

Does this PR introduce a user-facing change?

other Note

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openyurt-bot openyurt-bot added the kind/design kind/design label Jul 19, 2022
@openyurt-bot openyurt-bot added the size/L size/L: 100-499 label Jul 19, 2022
@xavier-hou xavier-hou force-pushed the proposal-workload-update-model branch from 9590ad6 to 2d9b71d Compare July 19, 2022 01:57
@codecov
Copy link

codecov bot commented Jul 19, 2022 •
edited
Loading

Codecov Report

Merging #921 (4119405) into master (2d5938d) will increase coverage by 4.92%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #921      +/-   ##
==========================================
+ Coverage   35.37%   40.30%   +4.92%     
==========================================
  Files          76       80       +4     
  Lines        9913    11003    +1090     
==========================================
+ Hits         3507     4435     +928     
- Misses       6141     6241     +100     
- Partials      265      327      +62
Flag Coverage Δ
unittests 40.30% <ø> (+4.92%) ⬆️

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

Impacted Files Coverage Δ
pkg/controller/kubernetes/util/node/node.go 82.35% <0.00%> (ø)
...troller/nodelifecycle/node_lifecycle_controller.go 56.15% <0.00%> (ø)
pkg/controller/kubernetes/util/taints/taints.go 82.08% <0.00%> (ø)
pkg/controller/nodelifecycle/metrics.go 100.00% <0.00%> (ø)
...ontroller/nodelifecycle/scheduler/timed_workers.go 100.00% <0.00%> (+10.29%) ⬆️
...ontroller/nodelifecycle/scheduler/taint_manager.go 84.52% <0.00%> (+84.52%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@xavier-hou xavier-hou force-pushed the proposal-workload-update-model branch from 2d9b71d to ba49d5c Compare July 19, 2022 02:05
@rambohe-ch
Copy link
Member

@HXCGIT Very appreciate you for posting proposal to upgrade edge workload. and i will take a look soon.

@rambohe-ch
Copy link
Member

/assign @zyjhtangtang

@Congrool Congrool mentioned this pull request Jul 22, 2022
Merged
@rambohe-ch
Copy link
Member

rambohe-ch commented Jul 25, 2022 •
edited
Loading

@HXCGIT codecov error has been fixed, so please rebase the master branch and push --force this pull reqeust.

@xavier-hou
add proposal of daemosnet update model
9ff0305 
Signed-off-by: hxcGit <houxc_mail@163.com>
@xavier-hou xavier-hou force-pushed the proposal-workload-update-model branch from ba49d5c to 9ff0305 Compare July 25, 2022 09:40
@rambohe-ch
Copy link
Member

@HXCGIT yes, we need to call RESTFUL api on edge nodes to upgrade edge pods. maybe we can define the RESTFUL api as following:

  1. get all pods on edge node
    GET /pods HTTP/1.1

  2. upgrade specified pod on edge node

POST /openyurt.io/v1/{ns}/{podname} HTTP/1.1
BODY: {
    operation: upgrade,
}

@kadisi
Copy link
Member

kadisi commented Jul 26, 2022

@HXCGIT yes, we need to call RESTFUL api on edge nodes to upgrade edge pods. maybe we can define the RESTFUL api as following:

  1. get all pods on edge node
    GET /pods HTTP/1.1
  2. upgrade specified pod on edge node
POST /openyurt.io/v1/{ns}/{podname} HTTP/1.1
BODY: {
    operation: upgrade,
}

The API for upgrading, suspending, and resuming the POD can be designed in accordance with the API for POD logs and EXEC

/api/v1/namespaces/{ns}/pods/{podname}/exec
/api/v1/namespaces/{ns}/pods/{podname}/logs

So I suggest the following format：

  • get all pods on edge node
GET /pods HTTP/1.1
  • upgrade specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/upgrade  HTTP/1.1
  • suspend specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/suspend  HTTP/1.1
  • resume specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/resume  HTTP/1.1

@rambohe-ch
Copy link
Member

@HXCGIT yes, we need to call RESTFUL api on edge nodes to upgrade edge pods. maybe we can define the RESTFUL api as following:

  1. get all pods on edge node
    GET /pods HTTP/1.1
  2. upgrade specified pod on edge node
POST /openyurt.io/v1/{ns}/{podname} HTTP/1.1
BODY: {
    operation: upgrade,
}

The API for upgrading, suspending, and resuming the POD can be designed in accordance with the API for POD logs and EXEC

/api/v1/namespaces/{ns}/pods/{podname}/exec
/api/v1/namespaces/{ns}/pods/{podname}/logs

So I suggest the following format：

  • get all pods on edge node
GET /pods HTTP/1.1
  • upgrade specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/upgrade  HTTP/1.1
  • suspend specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/suspend  HTTP/1.1
  • resume specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/resume  HTTP/1.1

@kadisi Thanks for your suggestion. agree+1

@rambohe-ch
Copy link
Member

@HXCGIT beside OnDelete update strategy, we need to add a new annotation(apps.openyurt.io/ota: true) for DaemonSet to specify OTA update strategy.

@xavier-hou
Copy link
Member Author

@HXCGIT yes, we need to call RESTFUL api on edge nodes to upgrade edge pods. maybe we can define the RESTFUL api as following:

  1. get all pods on edge node
    GET /pods HTTP/1.1
  2. upgrade specified pod on edge node
POST /openyurt.io/v1/{ns}/{podname} HTTP/1.1
BODY: {
    operation: upgrade,
}

The API for upgrading, suspending, and resuming the POD can be designed in accordance with the API for POD logs and EXEC

/api/v1/namespaces/{ns}/pods/{podname}/exec
/api/v1/namespaces/{ns}/pods/{podname}/logs

So I suggest the following format：

  • get all pods on edge node
GET /pods HTTP/1.1
  • upgrade specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/upgrade  HTTP/1.1
  • suspend specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/suspend  HTTP/1.1
  • resume specified pod on edge node
POST /openyurt.io/v1/namespaces/{ns}/pods/{podname}/resume  HTTP/1.1

Many thanks to your suggestion! I will use this form of APIs.

@xavier-hou
Copy link
Member Author

@HXCGIT beside OnDelete update strategy, we need to add a new annotation(apps.openyurt.io/ota: true) for DaemonSet to specify OTA update strategy.

OK, a nice suggestion!

@xavier-hou xavier-hou force-pushed the proposal-workload-update-model branch from 3188c7d to e8b79d2 Compare August 1, 2022 05:02
@rambohe-ch
Copy link
Member

@HXCGIT I have a question, how to select one of the ways(OTA-update and NewController) for daemonset by end users?

@xavier-hou
modify restAPI format
862cc23 
Signed-off-by: hxcGit <houxc_mail@163.com>
@xavier-hou xavier-hou force-pushed the proposal-workload-update-model branch from e8b79d2 to 862cc23 Compare August 1, 2022 08:21
@xavier-hou
Copy link
Member Author

@rambohe-ch I think we can use annotations apps.openyurt.io/upgrade-strategy: ota(for OTA-update), apps.openyurt.io/upgrade-strategy: immediate(for NewController) to distinguish them.

@xavier-hou
update annotation for auto upgrade model
4119405 
Signed-off-by: hxcGit <houxc_mail@163.com>
@kadisi
Copy link
Member

kadisi commented Aug 2, 2022

@rambohe-ch I think we can use annotations apps.openyurt.io/upgrade-strategy: ota(for OTA-update), apps.openyurt.io/upgrade-strategy: immediate(for NewController) to distinguish them.

why use apps.openyurt.io/upgrade-strategy: immediate ?

kadisi
kadisi reviewed Aug 2, 2022
View reviewed changes
Copy link
Member

@kadisi kadisi left a comment

Choose a reason for hiding this comment

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

just some nits

@rambohe-ch
Copy link
Member

@rambohe-ch I think we can use annotations apps.openyurt.io/upgrade-strategy: ota(for OTA-update), apps.openyurt.io/upgrade-strategy: immediate(for NewController) to distinguish them.

why use apps.openyurt.io/upgrade-strategy: immediate ?

@HXCGIT @kadisi maybe apps.openyurt.io/upgrade-strategy: auto is more reasonable.

@kadisi
Copy link
Member

kadisi commented Aug 2, 2022

@rambohe-ch I think we can use annotations apps.openyurt.io/upgrade-strategy: ota(for OTA-update), apps.openyurt.io/upgrade-strategy: immediate(for NewController) to distinguish them.

why use apps.openyurt.io/upgrade-strategy: immediate ?

@HXCGIT @kadisi maybe apps.openyurt.io/upgrade-strategy: auto is more reasonable.

agree : apps.openyurt.io/upgrade-strategy: auto

@rambohe-ch
Copy link
Member

/lgtm
/approve

@openyurt-bot openyurt-bot added approved approved lgtm lgtm labels Aug 2, 2022
@kadisi
Copy link
Member

kadisi commented Aug 2, 2022

/lgtm

@openyurt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hxcGit, rambohe-ch

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

@openyurt-bot openyurt-bot merged commit fa083f8 into openyurtio:master Aug 2, 2022
lorrielau pushed a commit to lorrielau/openyurt that referenced this pull request Aug 10, 2022
@xavier-hou @lorrielau
[Proposal] add proposal of daemosnet update model (openyurtio#921)
f9173a9 
* add proposal of daemosnet update model

Signed-off-by: hxcGit <houxc_mail@163.com>

* modify restAPI format

Signed-off-by: hxcGit <houxc_mail@163.com>

* update annotation for auto upgrade model

Signed-off-by: hxcGit <houxc_mail@163.com>
@xavier-hou xavier-hou mentioned this pull request Aug 26, 2022
Merged
@rambohe-ch rambohe-ch mentioned this pull request Aug 30, 2022
Closed
@xavier-hou xavier-hou mentioned this pull request Sep 14, 2022
Merged
@rambohe-ch rambohe-ch changed the title [Proposal] add proposal of daemosnet update model [Proposal] add proposal of daemonset update model Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kadisi kadisi kadisi left review comments

@huangyuqi huangyuqi huangyuqi left review comments

@rambohe-ch rambohe-ch rambohe-ch approved these changes

@charleszheng44 charleszheng44 Awaiting requested review from charleszheng44

Assignees

@zyjhtangtang zyjhtangtang

@rambohe-ch rambohe-ch

Labels
approved approved kind/design kind/design lgtm lgtm size/L size/L: 100-499
Projects
No open projects
controlplane-v1.1
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xavier-hou @openyurt-bot @rambohe-ch @kadisi @huangyuqi @zyjhtangtang