-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[sonic-py-common] get_platform(): Refactor method of retrieving platform identifier #5094
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…onf not accessible
lguohan
reviewed
Aug 4, 2020
…arses machine.conf
This comment has been minimized.
This comment has been minimized.
…icitly parses machine.conf" This reverts commit 2bb68ee.
lguohan
approved these changes
Aug 5, 2020
11 tasks
abdosi
pushed a commit
that referenced
this pull request
Aug 9, 2020
…orm identifier (#5094) Applications running in the host OS can read the platform identifier from /host/machine.conf. When loading configuration, sonic-config-engine *needs* to read the platform identifier from machine.conf, as it it responsible for populating the value in Config DB. When an application is running inside a Docker container, the machine.conf file is not accessible, as the /host directory is not mounted. So we need to retrieve the platform identifier from Config DB if get_platform() is called from inside a Docker container. However, we can't simply check that we're running in a Docker container because the host OS of the SONiC virtual switch is running inside a Docker container. So I refactored `get_platform()` to: 1. Read from the `PLATFORM` environment variable if it exists (which is defined in a virtual switch Docker container) 2. Read from machine.conf if possible (works in the host OS of a standard SONiC image, critical for sonic-config-engine at boot) 3. Read the value from Config DB (needed for Docker containers running in SONiC, as machine.conf is not accessible to them) - Also fix typo in daemon_base.py - Also changes to align `get_hwsku()` with `get_platform()`
santhosh-kt
pushed a commit
to santhosh-kt/sonic-buildimage
that referenced
this pull request
Feb 25, 2021
…orm identifier (sonic-net#5094) Applications running in the host OS can read the platform identifier from /host/machine.conf. When loading configuration, sonic-config-engine *needs* to read the platform identifier from machine.conf, as it it responsible for populating the value in Config DB. When an application is running inside a Docker container, the machine.conf file is not accessible, as the /host directory is not mounted. So we need to retrieve the platform identifier from Config DB if get_platform() is called from inside a Docker container. However, we can't simply check that we're running in a Docker container because the host OS of the SONiC virtual switch is running inside a Docker container. So I refactored `get_platform()` to: 1. Read from the `PLATFORM` environment variable if it exists (which is defined in a virtual switch Docker container) 2. Read from machine.conf if possible (works in the host OS of a standard SONiC image, critical for sonic-config-engine at boot) 3. Read the value from Config DB (needed for Docker containers running in SONiC, as machine.conf is not accessible to them) - Also fix typo in daemon_base.py - Also changes to align `get_hwsku()` with `get_platform()`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
- Why I did it
Applications running in the host OS can read the platform identifier from /host/machine.conf. When loading configuration, sonic-config-engine needs to read the platform identifier from machine.conf, as it it responsible for populating the value in Config DB.
When an application is running inside a Docker container, the machine.conf file is not accessible, as the /host directory is not mounted. So we need to retrieve the platform identifier from Config DB if get_platform() is called from inside a Docker
container. However, we can't simply check that we're running in a Docker container because the host OS of the SONiC virtual switch is running inside a Docker container.
- How I did it
Refactor
get_platform()
to:PLATFORM
environment variable if it exists (which is defined in a virtual switch Docker container)Also fix typo in daemon_base.py
Also changes to align
get_hwsku()
withget_platform()
- How to verify it
Do the following from:
Ensure the platform identifier is correct, and is not
None
.- Which release branch to backport (provide reason below if seleted)