Add iot provision tool #190
Conversation
There was a problem hiding this comment.
There is a lot of stuff in this PR which doesn't seem relevant for a device connector(e.g. scheduling, notifications, running of test jobs commands through checkbox etc).
Jobs should be scheduled by submission to the testflinger server. The code should not contain configuration for things like e-mail (including passwords!).
I think maybe some more working through the architecture of the testflinger components could be necessary to land the bits we want.
Hi Jonathan |
There was a problem hiding this comment.
It's a little hard to review this right now because I don't see any way that it gets called by the device connector, and it also seems to be some completely external tool. It almost feels like a dependency that should get installed rather than something that just pertains to the device connector at this point, but I'm not sure. I have a few comments below though, which I hope are helpful. Thanks for working on this!
| @@ -0,0 +1,16 @@ | |||
| [project] | |||
There was a problem hiding this comment.
I don't think you're going to want to have a separate pyproject.toml here. It won't be installed as part of the normal process with how this is deployed. Instead, I would recommend using the existing one, and integrate the code from this tool into something that can be called from the device connector. Otherwise you're relying on potentially another setup task for these, which isn't currently covered by the charm that deploys it.
There was a problem hiding this comment.
Hi Paul
This entire folder would be copied to zapper and installed then run on zapper (through zapper API), it would not be called by device-connector, this is an design and agreement in spec CR051 (https://docs.google.com/document/d/12cvc9l7v8ltQVlwx9hqqkoPxLdFW0nVEoDZ9vmPLMVw/edit?usp=sharing) that was created by Paolo.
There was a problem hiding this comment.
This seems a slightly bizarre design and not something I read from the spec. My understanding from the spec is that the device connector should be implementing rpyc calls to a remote process on the zapper. I don't see that reflected in this PR
There was a problem hiding this comment.
It's in the IoT method section, and due to this tool need to use usb serial to control target devices. Maciej think run this tool in device connector through rpyc is not realistic. The method you method was our first approach but was rejected by Maciej and Paolo, so we have this version.
There was a problem hiding this comment.
@kiya956 @p-gentili @kissiel please make sure that the spec is in line with the implementation.
| @@ -0,0 +1,6 @@ | |||
| { | |||
There was a problem hiding this comment.
You also don't need another renovate json - this is already covered in the project.
|
|
||
|
|
||
| def start_agent(cfg): | ||
| lanncher_parser = LauncherParser(cfg) |
| @@ -0,0 +1,3 @@ | |||
| from setuptools import setup | |||
There was a problem hiding this comment.
This file is also not needed, please use the existing project-wide setup if you really need it, but as mentioned earlier, I would hope this could be just called from the device connector rather than installed as a separate script if at all possible.
There was a problem hiding this comment.
This file is necessary due to we need to install this tool on zapper after device-connector copy the folder to zapper
There was a problem hiding this comment.
I'm not really a Python expert, but would it not be a bit easier to issue a pip install on the zapper from a private Git URL instead of copying from the device connector to the Zapper and then doing this?
There was a problem hiding this comment.
@mz2 I have proposed this method and was rejected by Rex and ...... you XD
There was a problem hiding this comment.
I think I came to a similar suggestion when catching up this topic just now, I don't see the justification in testflinger becoming a package manager.
There was a problem hiding this comment.
Leaving a comment here for posterity: we have resolved this confusion I believe successfully last week, and landed on a different design, where Testflinger device connector simply relays on (over RPC) data to the service running on Zapper, which also houses the Zapper deployed part to this.
| - checkbox: | ||
| snap_name: checkbox name | ||
| launcher: launcher file name | ||
| secure_id: device secure id on C3 |
There was a problem hiding this comment.
normally things like secure_id, serial data, etc are configured in the device connector config. I hope we can reuse that from the environment data and configure it the way we configure all the existing connectors?
There was a problem hiding this comment.
Thank you, just remove it
| case "seed_override_lk": | ||
| login(con) | ||
| ADDR = get_ip(con) | ||
| if ADDR == FAILED: |
There was a problem hiding this comment.
based on how it looks like get_ip() and check_net_connection() are getting used, I think you should consider just passing the exception and catching as specific of an exception as possible, then use logging to print the error and exit or raise as necessary so that calling code can do the right thing with it. I don't feel like replacing the exception handling with SUCCESS/FAILED is really making it much easier to read or follow.
impact:
new: iot-auto-sanity
description:
Add iot provision tool that would be copied to zapper and run on zapper
test:
N/A
Signed-off-by: ChunAn Wu <[email protected]>
impact:
new: iot-auto-sanity
description:
Add iot provision tool that would be copied to zapper and run on zapper
test:
N/A
Description
Resolved issues
Documentation
Tests