feat: move yurt-device-controller into yurt-manager #1607

wangxye · 2023-07-11T13:27:21Z

What type of PR is this?

/kind feature
/sig iot

What this PR does / why we need it:

This PR integrates the yurt-device-controller for iot into the yurt manager. Users can deploy it using PlatformAdmin.

Which issue(s) this PR fixes:

Fixes #1580

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

openyurt-bot · 2023-07-11T13:27:43Z

Welcome @wangxye! It looks like this is your first PR to openyurtio/openyurt 🎉

LavenderQAQ · 2023-07-12T07:53:53Z

/assign @LavenderQAQ

LavenderQAQ

IoT-sig finally decided to change yurt-device-controller to yurt-iot-dock (YurtIoTDock), please revise it.

LavenderQAQ · 2023-07-13T04:04:32Z

cmd/yurt-iot-carrier/app/options/options.go

+		Namespace:            "default",
+		CoreDataAddr:         "edgex-core-data:59880",
+		CoreMetadataAddr:     "edgex-core-metadata:59881",
+		CoreCommandAddr:      "edgex-core-command:59882",


We need to keep these parameters in sync with the configuration in PlatformAdmin in the future, could you open an issue to track this?

Of course, I will continue to track and extend the yurt-iot-dock using the ability to customize configurations in PlatformAdmin.

LavenderQAQ · 2023-07-13T04:11:29Z

pkg/apis/iot/v1alpha1/device_types.go

+)
+
+const (
+	DeviceFinalizer = "iot.device.finalizer"


I think it might be better to change this to "iot.openyurt.io/device".

LavenderQAQ · 2023-07-13T04:13:15Z

pkg/apis/iot/v1alpha1/deviceprofile_types.go

+)
+
+const (
+	DeviceProfileFinalizer = "iot.deviceProfile.finalizer"


I think it might be better to change this to "iot.openyurt.io/deviceprofile".

LavenderQAQ · 2023-07-13T04:13:26Z

pkg/apis/iot/v1alpha1/deviceservice_types.go

+)
+
+const (
+	DeviceServiceFinalizer = "iot.deviceService.finalizer"


I think it might be better to change this to "iot.openyurt.io/deviceservice".

LavenderQAQ · 2023-07-13T04:23:36Z

pkg/controller/platformadmin/platformadmin_controller.go

+		return nil, err
+	}
+	// YurtIoTCarrier doesn't need a service yet
+	yurtIoTCarrierConfig := map[string]interface{}{


Here don't use the map[string]interface, please use appsv1.DeploymentSpec and appsv1alpha1.YurtAppSet.

LavenderQAQ · 2023-07-13T04:27:22Z

pkg/controller/platformadmin/platformadmin_controller.go

+	err = json.Unmarshal([]byte(yurtIoTCarrierConfigBytes), &yurtIoTCarrierDeployment)
+	if err != nil {
+		return nil, err
+	}


The serialization and deserialization here looks a little redundant.

LavenderQAQ · 2023-07-13T04:40:18Z

pkg/controller/platformadmin/platformadmin_controller.go

@@ -600,3 +611,111 @@ func annotationToComponent(annotation map[string]string) ([]*config.Component, e

 	return components, nil
 }
+
+func NewYurtIoTCarrierComponent(platformAdmin *iotv1alpha2.PlatformAdmin) (*config.Component, error) {


The build method can be considered as an internal function, and the build Deployment process can be placed in the util package.

This function is still needed, but keep it as simple as possible.

LavenderQAQ · 2023-07-13T04:42:04Z

pkg/controller/platformadmin/utils/version.go

+
+const IotCtrlName = "yurt-iot-carrier"
+
+func DefaultVersion(platformAdmin *iotv1alpha2.PlatformAdmin) (string, string, error) {


You can set it as an internal function to put the component building process into util as well.

LavenderQAQ · 2023-07-13T04:51:33Z

pkg/yurtiotcarrier/controllers/well_known_labels.go

+package controllers
+
+const (
+	EdgeXObjectName = "device-controller/edgex-object.name"


Please update this label.

LavenderQAQ · 2023-07-14T02:05:45Z

pkg/controller/platformadmin/platformadmin_controller.go

@@ -226,6 +226,12 @@ func (r *ReconcilePlatformAdmin) reconcileDelete(ctx context.Context, platformAd
 	}
 	desiredComponents = append(desiredComponents, additionalComponents...)

+	yurtIotDock, err := util.NewYurtIoTDockComponent(platformAdmin)


Rebase may be required here after #1596 is combined.

LavenderQAQ

The current code looks fine. There may be some changes here after #1596 is merged, you could wait a while to rebase. I will review and test it again after rebase.

LavenderQAQ · 2023-07-14T02:24:21Z

/hold

LavenderQAQ · 2023-07-17T02:22:47Z

@wangxye #1596 has been merged, please use /unhold to unlock this pr after rebase.

Signed-off-by: wangxye <1031989637@qq.com>

wangxye · 2023-07-17T12:53:34Z

/unhold

LavenderQAQ · 2023-07-18T04:41:48Z

/unhold

LavenderQAQ · 2023-07-23T04:46:12Z

@wangxye #1626 could be resolved after this pr merged.

Rui-Gan · 2023-07-23T06:42:56Z

pkg/controller/platformadmin/util.go

+				Containers: []corev1.Container{
+					{
+						Name:            "yurt-iot-dock",
+						Image:           fmt.Sprintf("leoabyss/yurt-iot-dock:%s", ver),


It would be better to replace "leoabyss" with "openyurt" to make it more appropriate.

Rui-Gan · 2023-07-23T07:01:36Z

pkg/yurtiotdock/clients/edgex-foundry/util.go

+	return iotv1alpha1.Device{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      toKubeName(ed.Name),
+			Namespace: "default",


Would it be more appropriate to set the namespace here to match that of yurt-iot-dock?

Rui-Gan · 2023-07-23T07:05:25Z

pkg/yurtiotdock/clients/edgex-foundry/util.go

+	return iotv1alpha1.DeviceService{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      toKubeName(ds.Name),
+			Namespace: "default",


Rui-Gan · 2023-07-23T07:06:30Z

pkg/yurtiotdock/clients/edgex-foundry/util.go

+	return iotv1alpha1.DeviceProfile{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      toKubeName(dp.Name),
+			Namespace: "default",


Rui-Gan

some suggestions

Signed-off-by: wangxye <1031989637@qq.com>

sonarqubecloud · 2023-07-23T11:17:29Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
3 Security Hotspots
25 Code Smells

No Coverage information
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

LavenderQAQ

/lgtm

openyurt-bot · 2023-07-27T02:06:44Z

@LavenderQAQ: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

LavenderQAQ · 2023-07-27T02:12:30Z

@rambohe-ch This pr involves some CI changes (adding a new image build process for YurtIoTDock), needs to test the workflow.

LavenderQAQ · 2023-07-27T02:26:12Z

@rambohe-ch We found some problems with YurtIoTDock and have been tracking them in #1626 and #1627. Since these are subsequent enhancements, @wangxye will submit two more pr to address these issues.

github-advanced-security · 2023-08-09T03:36:18Z

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

codecov · 2023-08-09T03:43:05Z

Codecov Report

Merging #1607 (10835d3) into master (fe46a9d) will increase coverage by 0.70%.
Report is 3 commits behind head on master.
The diff coverage is 63.79%.

@@            Coverage Diff             @@
##           master    #1607      +/-   ##
==========================================
+ Coverage   51.43%   52.13%   +0.70%     
==========================================
  Files         134      142       +8     
  Lines       15947    16911     +964     
==========================================
+ Hits         8202     8817     +615     
- Misses       7000     7273     +273     
- Partials      745      821      +76

Flag	Coverage Δ
unittests	`52.13% <63.79%> (+0.70%)`	⬆️

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

Files Changed	Coverage Δ
pkg/yurtiotdock/controllers/util/conditions.go	`0.00% <0.00%> (ø)`
pkg/yurtiotdock/controllers/util/fieldindexer.go	`0.00% <0.00%> (ø)`
pkg/yurtiotdock/controllers/util/tools.go	`42.59% <42.59%> (ø)`
...dock/clients/edgex-foundry/deviceservice_client.go	`56.07% <56.07%> (ø)`
...yurtiotdock/clients/edgex-foundry/device_client.go	`56.41% <56.41%> (ø)`
...dock/clients/edgex-foundry/deviceprofile_client.go	`56.81% <56.81%> (ø)`
pkg/yurtiotdock/clients/edgex-foundry/util.go	`92.19% <92.19%> (ø)`
pkg/yurtiotdock/controllers/util/string.go	`100.00% <100.00%> (ø)`

openyurt-bot · 2023-08-09T07:35:55Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~OWNERS~~ [rambohe-ch]

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

rambohe-ch · 2023-08-09T07:36:06Z

/lgtm

openyurt-bot added do-not-merge/work-in-progress do-not-merge/work-in-progress kind/feature kind/feature labels Jul 11, 2023

openyurt-bot requested a review from charleszheng44 July 11, 2023 13:27

openyurt-bot added the sig/iot sig/iot label Jul 11, 2023

openyurt-bot requested a review from Fei-Guo July 11, 2023 13:27

openyurt-bot added the size/XXL label Jul 11, 2023

wangxye marked this pull request as ready for review July 12, 2023 02:52

openyurt-bot removed the do-not-merge/work-in-progress do-not-merge/work-in-progress label Jul 12, 2023

openyurt-bot assigned LavenderQAQ Jul 12, 2023

LavenderQAQ requested changes Jul 12, 2023

View reviewed changes

LavenderQAQ reviewed Jul 13, 2023

View reviewed changes

LavenderQAQ requested changes Jul 13, 2023

View reviewed changes

wangxye force-pushed the device branch from 11af566 to 053d13f Compare July 13, 2023 14:26

LavenderQAQ reviewed Jul 14, 2023

View reviewed changes

openyurt-bot added the do-not-merge/hold do-not-merge/hold label Jul 14, 2023

wangxye added 6 commits July 17, 2023 20:37

add a new crd iot.device/deviceservice/deviceprofile and incorporate it

ec1c329

Signed-off-by: wangxye <1031989637@qq.com>

enable PlatformAdmin to deploy yurt-device-controller

4a43885

Signed-off-by: wangxye <1031989637@qq.com>

add ci for yurt-device-controller

4422019

Signed-off-by: wangxye <1031989637@qq.com>

enables yurt-device-controller to be deployed in any namespace

fd0d04a

Signed-off-by: wangxye <1031989637@qq.com>

rename yurt-device-controller to yurt-iot-dock

9827724

Signed-off-by: wangxye <1031989637@qq.com>

extend yurt-iot-dock to sync it with the configuration in PlatformAdmin

1c50158

Signed-off-by: wangxye <1031989637@qq.com>

wangxye force-pushed the device branch from 053d13f to 1c50158 Compare July 17, 2023 12:41

openyurt-bot removed the do-not-merge/hold do-not-merge/hold label Jul 18, 2023

LavenderQAQ mentioned this pull request Jul 23, 2023

fix: add platformadmin crd to the manifests script #1604

Closed

Rui-Gan reviewed Jul 23, 2023

View reviewed changes

Rui-Gan suggested changes Jul 23, 2023

View reviewed changes

ensure that yurt-iot-dock and CRs are in the same namespace

10835d3

Signed-off-by: wangxye <1031989637@qq.com>

LavenderQAQ approved these changes Jul 27, 2023

View reviewed changes

rambohe-ch approved these changes Aug 9, 2023

View reviewed changes

openyurt-bot added the approved approved label Aug 9, 2023

openyurt-bot assigned rambohe-ch Aug 9, 2023

openyurt-bot added the lgtm lgtm label Aug 9, 2023

rambohe-ch merged commit d89a805 into openyurtio:master Aug 9, 2023

open-digger-bot bot mentioned this pull request Aug 16, 2023

[WeeklyReport] Weekly report for openyurt 8/9/2023 to 8/16/2023 #1657

Closed

wangxye mentioned this pull request Aug 25, 2023

[REQUEST Membership]: New membership for <wangxye> openyurtio/community#74

Closed

6 tasks


		const IotCtrlName = "yurt-iot-carrier"

		func DefaultVersion(platformAdmin *iotv1alpha2.PlatformAdmin) (string, string, error) {

feat: move yurt-device-controller into yurt-manager #1607

feat: move yurt-device-controller into yurt-manager #1607

Conversation

wangxye commented Jul 11, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

openyurt-bot commented Jul 11, 2023

LavenderQAQ commented Jul 12, 2023

LavenderQAQ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LavenderQAQ left a comment

Choose a reason for hiding this comment

LavenderQAQ commented Jul 14, 2023

LavenderQAQ commented Jul 17, 2023

wangxye commented Jul 17, 2023

LavenderQAQ commented Jul 18, 2023

LavenderQAQ commented Jul 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rui-Gan left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jul 23, 2023

LavenderQAQ left a comment

Choose a reason for hiding this comment

openyurt-bot commented Jul 27, 2023

LavenderQAQ commented Jul 27, 2023

LavenderQAQ commented Jul 27, 2023

github-advanced-security bot commented Aug 9, 2023

codecov bot commented Aug 9, 2023 • edited Loading

Codecov Report

openyurt-bot commented Aug 9, 2023

rambohe-ch commented Aug 9, 2023

codecov bot commented Aug 9, 2023 •

edited

Loading