-
Notifications
You must be signed in to change notification settings - Fork 172
add device anomaly detection demo #163
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
base: master
Are you sure you want to change the base?
add device anomaly detection demo #163
Conversation
|
Welcome @HaojieZhang6848! It looks like this is your first PR to kubeedge/examples 🎉 |
Summary of ChangesHello @HaojieZhang6848, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a fully functional demonstration of device anomaly detection integrated with KubeEdge. It provides a practical example for developers to understand and implement ML-driven anomaly detection for IoT devices, leveraging KubeEdge's device management and communication protocols to update device statuses in Kubernetes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a KubeEdge-based device anomaly detection demo. The changes include a new external-anomaly-detector Python gRPC service that uses a Bayesian Network model to detect anomalies in IoT device data, a room-simulator FastAPI application to simulate light devices and a brightness sensor with fault injection capabilities, and modifications to the me-restful-mapper Go application to integrate with the anomaly detection service via gRPC. The mapper now collects device property updates and sends them to the external anomaly detector, which then updates Kubernetes DeviceStatus CRDs with anomaly information. New Protobuf definitions, Docker Compose configurations, and detailed READMEs (English and Chinese) are added to support the demo. Review comments highlight several issues: a critical logical error where the anomaly status is updated for all devices instead of the specific one, masked errors in the Go mapper's SetDeviceData and GetDeviceData functions, inconsistencies in port numbers between the architecture diagram and implementation, incorrect plain text vs. JSON body expectations in fault injection examples across documentation and code, an improper string formatting in SetDeviceValue, and a security concern regarding the use of grpc.WithInsecure() in the demo. Additionally, there are suggestions to replace print statements with proper logging and to handle Kubernetes config loading errors more gracefully.
| for d_name in ["light-1", "light-2", "brightness-sensor-1"]: | ||
| api.patch_namespaced_custom_object( | ||
| group="devices.kubeedge.io", | ||
| version="v1beta1", | ||
| namespace="default", | ||
| plural="devicestatuses", | ||
| name=d_name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update_anomaly_status function currently updates the anomaly status for all three devices (light-1, light-2, brightness-sensor-1) regardless of which device the anomaly was detected for. This is a critical logical error. The anomaly status should only be updated for the specific device where the anomaly was observed (e.g., brightness-sensor-1 if its predicted brightness doesn't match the actual value).
api.patch_namespaced_custom_object(
group="devices.kubeedge.io",
version="v1beta1",
namespace="default",
plural="devicestatuses",
name="brightness-sensor-1", # Only update the sensor that reports brightness
body=patch
)| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| klog.Errorf("GetDeviceData failed for http://%s:%d%s, error: %v", c.ProtocolConfig.ConfigData.Ip, c.ProtocolConfig.ConfigData.Port, c.ProtocolConfig.ConfigData.Url, err) | ||
| return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an error occurs in GetDeviceData, it returns nil, nil. This masks the error, making it difficult for callers to distinguish between a successful call with no data and a failed call. It should return nil, err to propagate the error.
| klog.Errorf("GetDeviceData failed for http://%s:%d%s, error: %v", c.ProtocolConfig.ConfigData.Ip, c.ProtocolConfig.ConfigData.Port, c.ProtocolConfig.ConfigData.Url, err) | |
| return nil, nil | |
| return nil, err |
| subgraph "Cloud/Control Plane" | ||
| CloudCore[CloudCore] | ||
| K8s[Kubernetes API Server] | ||
| Detector[External Anomaly Detector<br/>gRPC Service<br/>Port 5000] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The architecture diagram indicates the External Anomaly Detector runs on Port 5000. However, the docker-compose.yaml and external-anomaly-detector/main.py configure it to run on Port 6804. This discrepancy should be corrected in the diagram for accuracy.
Detector[External Anomaly Detector<br/>gRPC Service<br/>Port 6804]
| response = requests.post( | ||
| f"{base_url}/lights/light1/fault", | ||
| json={"fault": True} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Python requests example for injecting faults uses json={"fault": True}. This is inconsistent with the Python main.py implementation which expects a plain text body ('0' or '1'). Please correct the example to match the expected plain text input.
| response = requests.post( | |
| f"{base_url}/lights/light1/fault", | |
| json={"fault": True} | |
| response = requests.post( | |
| f"{base_url}/lights/light1/fault", | |
| data="1", | |
| headers={"Content-Type": "text/plain"} | |
| ) |
| # Fetch current value from k8s apiserver and store in memory | ||
| self._store_device_data(request.deviceName, request.propertyName, request.propertyValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment Fetch current value from k8s apiserver and store in memory is misleading. The propertyValue is received directly from the gRPC request, not fetched from the Kubernetes API server at this point.
| # Fetch current value from k8s apiserver and store in memory | |
| self._store_device_data(request.deviceName, request.propertyName, request.propertyValue) | |
| # Store received device data in memory |
| # Fetch current value from k8s apiserver and store in memory | ||
| self._store_device_data(request.deviceName, request.propertyName, request.propertyValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment Fetch current value from k8s apiserver and store in memory is misleading. The propertyValue is received directly from the gRPC request, not fetched from the Kubernetes API server at this point.
| # Fetch current value from k8s apiserver and store in memory | |
| self._store_device_data(request.deviceName, request.propertyName, request.propertyValue) | |
| # Store received device data in memory |
| ports: | ||
| - "6804:6804" | ||
| volumes: | ||
| - ~/.kube/config:/root/.kube/config:ro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mounting ~/.kube/config directly into the container can be a security risk as it exposes the host's Kubernetes credentials. While acceptable for a demo, consider more secure methods for production deployments, such as using Kubernetes Secrets or service accounts, or limiting the scope of the mounted config.
| $ docker ps | ||
| a2cc34c56101 huajuan6848/external-anomaly-detector:1.0.0 "uv run python main.…" About an hour ago Up About an hour 0.0.0.0:6804->6804/tcp, :::6804->6804/tcp external-anomaly-detector | ||
| 6bc6d74a36de huajuan6848/room-simulator:1.0.0 "uv run uvicorn main…" About an hour ago Up About an hour 0.0.0.0:8000->8000/tcp, :::8000->8000/tcp room-simulator | ||
| 098a371611f7 huajuan6848/me-restful-mappper:1.0.0 "/kubeedge/main --co…" About an hour ago Up About an hour me-restful-mapper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: "mappper" should be "mapper".
| 098a371611f7 huajuan6848/me-restful-mappper:1.0.0 "/kubeedge/main --co…" About an hour ago Up About an hour me-restful-mapper | |
| 098a371611f7 huajuan6848/me-restful-mapper:1.0.0 "/kubeedge/main --co…" About an hour ago Up About an hour me-restful-mapper |
| level=logging.INFO, | ||
| format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', | ||
| handlers=[ | ||
| logging.FileHandler('room_simulator.log'), | ||
| logging.StreamHandler() | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Haojie Zhang <hjzhang6848@163.com>
31c541e to
338cb78
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: HaojieZhang6848 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds a comprehensive Device Anomaly Detection Demo to showcase the device anomaly detection capabilities introduced in KubeEdge PR #6543. The demo provides a complete, runnable example that demonstrates:
This example serves as a reference implementation for developers who want to:
Which issue(s) this PR fixes:
Relates to kubeedge/kubeedge#6543
Special notes for your reviewer:
docker-compose upcommand