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

[Pmon] Add new daemon "syseepromd" to pmon docker #2866

Merged
merged 14 commits into from
Jun 18, 2019
Merged

[Pmon] Add new daemon "syseepromd" to pmon docker #2866

merged 14 commits into from
Jun 18, 2019

Conversation

keboliu
Copy link
Collaborator

@keboliu keboliu commented May 5, 2019 •
edited
Loading

- What I did
Introduce a new daemon syseepromd to pmon, which will post syseeprom info to pmon when starting pmon, and will clear those info when stopping pmon, and will keep monitoring the DB in case the info was cleared.

this PR has a dependency on sonic-net/sonic-platform-daemons#25

- How I did it

  1. modify make file to add the new daemon syseepromd
  2. add code to generate supervisord conf and start.sh for the new daemon

- How to verify it
build an image include this PR to verify the state DB, thes syseeprom info should be there after pmon started, stop pmon and those info should be removed from the state DB.

- Description for the changelog

dockers/docker-platform-monitor/start.sh.j2
/dockers/docker-platform-monitor/docker-pmon.supervisord.conf.j2
rules/docker-platform-monitor.mk
rules/sonic-post-syseeprom.mk

- A picture of a cute animal (not mandatory but encouraged)

@keboliu keboliu mentioned this pull request May 5, 2019
Merged
@liat-grozovik
Copy link
Collaborator

retest please

qiluo-msft
qiluo-msft previously requested changes May 18, 2019
View reviewed changes
Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comment

@keboliu keboliu mentioned this pull request May 20, 2019
Closed
qiluo-msft
qiluo-msft approved these changes May 22, 2019
View reviewed changes
Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

Only one minor comment, otherwise looks good to me.

jleveque
jleveque suggested changes May 30, 2019
View reviewed changes
Copy link
Contributor

@jleveque jleveque left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@keboliu: The event listener does not take care of the situation Guohan was concerned about. I think you misunderstood his comment. We are not concerned with clearing the syseeprom data from the DB when PMON exits; syseeprom data is static and should not change. However, the concern Guohan raised was that the STATE_DB could get cleared by other services (config reload, swss restart, etc.). We need a way to recognize the STATE_DB was cleared and repopulate the syseeprom info into the DB.

The SwSS restart scenario shouldn't be a problem because we have configured the PMon service as a dependent of SwSS via systemd. Thus, if SwSS gets restarted, so too should PMon get restarted. However, we still need a way to get notified that the STATE_DB was cleared so we can repopulate the info.

@keboliu
Copy link
Collaborator Author

keboliu commented May 30, 2019

@keboliu: The event listener does not take care of the situation Guohan was concerned about. I think you misunderstood his comment. We are not concerned with clearing the syseeprom data from the DB when PMON exits; syseeprom data is static and should not change. However, the concern Guohan raised was that the STATE_DB could get cleared by other services (config reload, swss restart, etc.). We need a way to recognize the STATE_DB was cleared and repopulate the syseeprom info into the DB.

The SwSS restart scenario shouldn't be a problem because we have configured the PMon service as a dependent of SwSS via systemd. Thus, if SwSS gets restarted, so too should PMon get restarted. However, we still need a way to get notified that the STATE_DB was cleared so we can repopulate the info.

@jleveque I get the point. Given the demand for repopulating syseeprom info to state DB in any case if it was cleared, what's your view to change the one-shot scripts to a new daemon for populate syseeprom and monitor it?

@jleveque
Copy link
Contributor

@lguohan: Will there ever be an instance where STATE_DB will get cleared which will not be accompanied by a SwSS restart? If so, I agree with @keboliu that we will most likely need a daemon to monitor the DB.

@keboliu keboliu changed the title [Pmon] Add new utility "post-syseeprom" to pmon docker [Pmon] Add new daemon "syseepromd" to pmon docker Jun 5, 2019
@jleveque
Copy link
Contributor

Approved. However, do not merge until submodule update has been included in this PR.

@jleveque
Copy link
Contributor

@keboliu: sonic-net/sonic-platform-daemons#25 has been merged. Please update the submodule in this PR.

@keboliu
Copy link
Collaborator Author

keboliu commented Jun 18, 2019

retest vs please

@keboliu
Copy link
Collaborator Author

keboliu commented Jun 18, 2019

@jleveque submodule has been updated, ready for merge:)

@jleveque jleveque merged commit 8a08595 into sonic-net:master Jun 18, 2019
yxieca pushed a commit that referenced this pull request Jun 16, 2023
@mssonicbld
[submodule] Update submodule sonic-utilities to the latest HEAD autom…
b0f2cb3 
…atically (#15510)

src/sonic-utilities

* 43c85ef5 - (HEAD -> 202205, origin/202205) [dhcp-relay] Fix dhcp6relay counter issue (#2866) (10 hours ago) [Yaqiang Zhu]
* e0333b5c - Revert "[config]config reload should generate sysinfo if missing (#2778)" (#2865) (10 hours ago) [jingwenxie]
* e7eac6bf - [config] Generate sysinfo in single asic (#2856) (10 hours ago) [jingwenxie]
* e42291bb - [config] Generate sysinfo as needed when override config (#2836) (10 hours ago) [jingwenxie]
qiluo-msft pushed a commit that referenced this pull request Jul 6, 2023
@yaqiangz
[submodule][202012] Update submodule sonic-utilities to the latest HE…
6e7c000 
…AD (#15532)

#### Why I did it
Update submodule sonic-utilities to the latest HEAD
```
50296b90 - [202012][dhcp-relay] Fix dhcp6relay counter issue (#2866) (#2873) (Fri Jun 30 18:08:53 2023 +0800) <Yaqiang Zhu>
160030c4 - [202012][dhcp_relay] Add "Reconfigure", "Information-Request", "Malformed" counter options (#2844) (Sun Jun 11 20:35:30 2023 -0700) <kellyyeh>
e6289ced - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan (#2852) (Fri Jun 2 18:20:21 2023 +0000) <Yaqiang Zhu>
885082ec - correctly parsing complete ipv6 vnet info (#2827) (Thu May 25 06:01:40 2023 +0000) <Keith Lu>
```

##### Work item tracking
- Microsoft ADO 22635770
github-actions bot pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Aug 29, 2023
@yejianquan
Merged PR 8419202: [202012]Fix merge conflict
b5fe131 
Related work items: #118, #124, sonic-net#2827, sonic-net#2844, sonic-net#2852, sonic-net#2866, sonic-net#2873, sonic-net#15532, sonic-net#15543, sonic-net#15637
mssonicbld added a commit that referenced this pull request Oct 18, 2023
@mssonicbld
[submodule] Update submodule sonic-swss to the latest HEAD automatica…
0aa0854 
…lly (#16889)

#### Why I did it
src/sonic-swss
```
* f31ccd09 - (HEAD -> master, origin/master, origin/HEAD) Add refillToSync() into ConsumerBase to support warmboot. (#2866) (2 days ago) [mint570]
```
#### How I did it
#### How to verify it
#### Description for the changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jleveque jleveque jleveque approved these changes

@qiluo-msft qiluo-msft qiluo-msft approved these changes

Assignees
No one assigned
Labels
Enhancement ➕ Request for 201904 Branch 🏂
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@keboliu @liat-grozovik @jleveque @qiluo-msft