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

Feat/windows nodes edgecore #4967

Merged
merged 11 commits into from Sep 22, 2023
Merged

Feat/windows nodes edgecore #4967

merged 11 commits into from Sep 22, 2023

Conversation

wujunyi792
Copy link
Contributor

What type of PR is this?
/kind feature

What this PR does / why we need it:
I try to make kubeedge run on windows server as an edge node. This PR is mainly related to edgecore itself, especially edged.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:
Mainly modified the following content:

  1. Add the windows-related configurations that were missed when the edged configuration was converted to kubelet
  2. Change the linux file path to the windows path through conditional compilation (it will not affect linux)
  3. Rewrite some functions through conditional compilation (for example, it is not necessary to check cgroup in the windows environment)
  4. Modify the function environmentCheck to avoid reporting an error and exiting directly after failing to obtain the process name
  5. Modify the initSock function of dmi (state a sock file under windows always fails)
    Does this PR introduce a user-facing change?:

@wujunyi792
feat: add config especial for Windows server
05127db 
Signed-off-by: JunYi Wu <wu65830600@163.com>
@wujunyi792
feat: use conditional compilation to complete edgecore windows config…
25aa216 
…uration

Signed-off-by: JunYi Wu <wu65830600@163.com>
@kubeedge-bot kubeedge-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 30, 2023
@kubeedge-bot kubeedge-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 30, 2023
@wujunyi792 wujunyi792 mentioned this pull request Aug 30, 2023
Merged
@Shelley-BaoYue Shelley-BaoYue added this to the v1.15 milestone Aug 30, 2023
@Shelley-BaoYue
Copy link
Collaborator

/sig node

@kubeedge-bot kubeedge-bot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Aug 30, 2023
@wujunyi792
feat: clean code
fd84a6a 
Signed-off-by: JunYi Wu <wu65830600@163.com>
@wujunyi792
fix: bug
a80397f 
Signed-off-by: JunYi Wu <wu65830600@163.com>
fisherxu
fisherxu reviewed Sep 13, 2023
View reviewed changes
edge/cmd/edgecore/app/server.go
@@ -176,7 +176,7 @@ func environmentCheck() error {
for _, process := range processes {
processName, err := process.Name()
if err != nil {
return err
continue
Copy link
Member

Choose a reason for hiding this comment

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

For the environmentCheck, can we also put it into files for windows and others?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we use continue to skip check for some processes which failed to get name. I think it can be kept in Linux as well, but we need add a comment here.

edge/pkg/devicetwin/dmiserver/server.go Outdated Show resolved Hide resolved
pkg/apis/componentconfig/edgecore/v1alpha2/default_kubelet_configuration.go Outdated Show resolved Hide resolved
pkg/apis/componentconfig/edgecore/v1alpha2/default.go Outdated Show resolved Hide resolved
@wujunyi792
chore: remake dmiserver initSock && add a comment in ConvertConfigEdg…
e33ddee 
…edFlagToConfigKubeletFlag

Signed-off-by: JunYi Wu <wu65830600@163.com>
@wujunyi792
chore: update
c973383 
Signed-off-by: JunYi Wu <wu65830600@163.com>
fisherxu
fisherxu reviewed Sep 21, 2023
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.

edge/pkg/devicetwin/dmiserver/server_others.go Outdated Show resolved Hide resolved
edge/pkg/devicetwin/dmiserver/server_windows.go Outdated Show resolved Hide resolved
@wujunyi792
chore: sort import
f2a25b5 
Signed-off-by: wujunyi <wu65830600@163.com>
@wujunyi792
chore: avoid using 'if runtime.GOOS'
8a0d3f3 
Signed-off-by: wujunyi <wu65830600@163.com>
@wujunyi792
fix: bugs
5b7c468 
Signed-off-by: wujunyi <wu65830600@163.com>
@wujunyi792
chore: update
9e74768 
1. sort import
2. fix missing import
3. rename filename

Signed-off-by: wujunyi <wu65830600@163.com>
@wujunyi792
chore: update import sort
df567aa 
Signed-off-by: wujunyi <wu65830600@163.com>
fisherxu
fisherxu reviewed Sep 21, 2023
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
/assign @Shelley-BaoYue

@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 Sep 21, 2023
@Shelley-BaoYue
Copy link
Collaborator

/lgtm

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2023
@kubeedge-bot kubeedge-bot merged commit f710546 into kubeedge:master Sep 22, 2023
18 checks passed
@wujunyi792 wujunyi792 mentioned this pull request Nov 13, 2023
Closed
7 tasks
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

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

@Baoqiang-Zhang Baoqiang-Zhang Awaiting requested review from Baoqiang-Zhang

@daixiang0 daixiang0 Awaiting requested review from daixiang0

@sids-b sids-b Awaiting requested review from sids-b

Assignees

@Shelley-BaoYue Shelley-BaoYue

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.15
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wujunyi792 @Shelley-BaoYue @kubeedge-bot @fisherxu