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.

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
merged 13 commits into from
Aug 5, 2020
Merged

[sonic-py-common] get_platform(): Refactor method of retrieving platform identifier #5094

merged 13 commits into from
Aug 5, 2020

Conversation

jleveque
Copy link
Contributor

@jleveque jleveque commented Aug 3, 2020

- 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:

    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 when loading config)
    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()

- How to verify it
Do the following from:

  1. The host OS of a standard SONiC image
  2. Inside a Docker container running on a standard SONiC image
  3. Inside the sonic-docker-vs virtual switch container
root@sonic:/# python2
Python 2.7.16 (default, Oct 10 2019, 22:02:15) 
[GCC 8.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from sonic_py_common.device_info import get_platform
>>> get_platform()

Ensure the platform identifier is correct, and is not None.

- Which release branch to backport (provide reason below if seleted)

  • 201811
  • 201911
  • 202006

@jleveque jleveque changed the title [sonic-py-common] get_platform(): Fall back to Config DB if machine.conf not accessible [sonic-py-common] get_platform(): Read from Config DB if called from inside Docker container Aug 3, 2020
@jleveque jleveque changed the title [sonic-py-common] get_platform(): Read from Config DB if called from inside Docker container [sonic-py-common] get_platform(): Always read from Config DB Aug 4, 2020
@lgtm-com

This comment has been minimized.

@jleveque jleveque changed the title [sonic-py-common] get_platform(): Always read from Config DB [sonic-py-common] get_platform(): Refactor method of retrieving platform identifier Aug 4, 2020
@jleveque jleveque requested a review from lguohan August 5, 2020 00:11
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants