-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add sensor packages to syncd for bfn #1997
base: 201807
Are you sure you want to change the base?
Conversation
why the sensors and fancontrol packages are installed in the syncd container? |
@@ -27,6 +27,8 @@ RUN apt-get update \ | |||
python-dev \ | |||
wget \ | |||
cmake \ | |||
sensord \ | |||
fancontrol \ |
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.
sensord and fancontrol are already in pmon docker, why do we need them in syncd docker?
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.
For bfn platforms, sensors information is not available on the host cpu and is fetched from the BMC. The infra needed to retrieve this information is available present in the syncd container, which we leverage for retrieving needed information from the BMC. Hence the need to have these packages in syncd
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.
can you pass the information from syncd to the pmon docker? we'd like to unify the those controller in one place for all platforms.
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.
That would need us to install all python libraries on pmon container along with some rpc infra to communicate with syncd container. I do not know of any other clean mechanism to pass data between two containers. Hence the need to have these packages on syncd itself
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.
sensord and fancontrol are already in pmon docker.
could you resolve the conflicts? |
@lguohan - what else do we need to do here? |
Add P4orch code and subdir Co-authored-by: PINS Working Group <[email protected]>
- What I did
Added sensor packages to bfn syncd container
- How I did it
Added packages to install to Docker files
- How to verify it
Build sonic binary and verify sensor utility present inside syncd container
- Description for the changelog
Add linux sensor utility to syncd for bfn
- A picture of a cute animal (not mandatory but encouraged)