Skip to content

[config] config reload should generate sysinfo if missing #3031

Merged
wen587 merged 6 commits intosonic-net:masterfrom
wen587:config_reload_f
Nov 2, 2023
Merged

[config] config reload should generate sysinfo if missing #3031
wen587 merged 6 commits intosonic-net:masterfrom
wen587:config_reload_f

Conversation

@wen587
Copy link
Contributor

@wen587 wen587 commented Oct 31, 2023

ADO:17746072

What I did

Missing platform and mac in CONFIG_DB will result in container failure. We should make the config reload generate those info if missing.

How I did it

Add missing sys info if config_db.json doesn't contain it.

How to verify it

Unit test

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@wen587 wen587 marked this pull request as ready for review November 1, 2023 08:54
@wen587 wen587 merged commit a4eeb69 into sonic-net:master Nov 2, 2023
StormLiangMS pushed a commit that referenced this pull request Nov 2, 2023
What I did
Missing platform and mac in CONFIG_DB will result in container failure. We should make the config reload generate those info if missing.

How I did it
Add missing sys info if config_db.json doesn't contain it.

How to verify it
Unit test
wen587 added a commit to wen587/sonic-utilities that referenced this pull request Jan 18, 2024
…3031)

What I did
Missing platform and mac in CONFIG_DB will result in container failure. We should make the config reload generate those info if missing.

How I did it
Add missing sys info if config_db.json doesn't contain it.

How to verify it
Unit test
with open(fileName, 'w') as f:
json.dump(json_input, f, indent=4)
except Exception as e:
raise Exception(str(e))
Copy link
Contributor

Choose a reason for hiding this comment

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

except and raise immediately does not add value. It is the same as no except at all.

# Save to tmpfile in case of stdin input which can only be read once
if file == "/dev/stdin":
file_input = read_json_file(file)
(_, tmpfname) = tempfile.mkstemp(dir="/tmp", suffix="_configReloadStdin")
Copy link
Contributor

Choose a reason for hiding this comment

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

Look around the login inside this function, it seems one time reading of file will be enough. Let's take further effort to reorg the existing code and optimize the logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sonic-cfggen -j read it first. Then sonic-cfggen read it second.

yxieca pushed a commit that referenced this pull request Jan 22, 2024
)

What I did
Missing platform and mac in CONFIG_DB will result in container failure. We should make the config reload generate those info if missing.

How I did it
Add missing sys info if config_db.json doesn't contain it.

How to verify it
Unit test
nmoray pushed a commit to nmoray/sonic-utilities that referenced this pull request Jun 25, 2025
…3031)

What I did
Missing platform and mac in CONFIG_DB will result in container failure. We should make the config reload generate those info if missing.

How I did it
Add missing sys info if config_db.json doesn't contain it.

How to verify it
Unit test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants