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

Initial commit for introducing the SONiC Platform Development Environ… #3778

Closed
wants to merge 10 commits into from

Conversation

wbschwar
Copy link

What I did
This PR is for the build infrastructure changes to support the PDE (platform development environment). For additional detail see:
sonic-net/SONiC#407

How I did it
The PDE silicon test harness and platform test harness can be found in
src/sonic-platform-pdk-pde

How to verify it
You must have both this PR and PR below.
Azure/sonic-platform-pdk-pde#28

Enable PDE by un-commenting line in rules/config
ENABLE_PDE=y

build PDE enabled SONiC install image
make target/sonic-$MACHINE-pde.bin

Currently only Broadcom support has been added.

Description for the changelog
Adding support for Platform Development Environment (PDE)

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

@wbschwar
Copy link
Author

wbschwar commented Dec 3, 2019

retest this please

hpersh
hpersh previously approved these changes Dec 5, 2019
src/sonic-platform-pde \
src/sonic-ztp

.PHONY: sonic-slave-build sonic-slave-bash init initpde reset
Copy link
Collaborator

Choose a reason for hiding this comment

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

get rid of initpde

Copy link
Author

Choose a reason for hiding this comment

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

We need to be able to build a minimal subset of SONiC with the PDE docker enabled. We can either handle this as part of make init (or in this case make initpde), or we would have to modify slave.mk to ignore NON-PDE required build targets. In this case, we think its easier to maintain a list of essential submodules for the PDE and make it part of the "make initpde" step for doing a build. Can we leave this change as it is or do yo have an alternative suggestion?

We automate the PDE build internally by doing the following.

make initpde
make configure PLATFORM=broadcom
make target/sonic-broadcom-pde.bin

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lguohan, @jleveque - can you please address these responses from Bill (following comments also)? We have come up with a way of doing a minimal build for PDE (reduced build time etc), but it seems that you didn't like this. Do you have an alternative in mind? Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lguohan, @jleveque - can you please address these responses from Bill (following comments also)? We have come up with a way of doing a minimal build for PDE (reduced build time etc), but it seems that you didn't like this. Do you have an alternative in mind? Thanks.

@lguohan, @jleveque - I never heard back. Should I setup a call to discuss?

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to run this "make initpde" to differentiate the Full SONIC image build. Actually, ODM will not run this step, since the step is done by BRCM internally to generate the light weight PDE repo for ODM. Another reason for this is to protect the BRCM SONIC changes with just platform related common change exposing to ODM when working with ODM on ODM HWQ projects.

For PDE, we also have another option which is to run PDE as a docker in Full SONIC image. In the FULL sonic image runtime, we can launch PDE by pde.sh script. The script will disable most dockers in SONIC image and just reserve the necessary dockers with a lightweight environment for ODM platform testing. By default, the PDE docker will not be triggered in Full SONIC image.

Geans

make initpde

make configure PLATFORM=broadcom

make target/sonic-broadcom-pde.bin

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lguohan, @jleveque - no movement on this topic - please offer some morning slots and I will try to setup a call - thx.

$(SONIC_ONE_PDE_IMAGE)_IMAGE_TYPE = onie
$(SONIC_ONE_PDE_IMAGE)_INSTALLS += $(BRCM_OPENNSL_KERNEL)
$(SONIC_ONE_PDE_IMAGE)_INSTALLS += $(PDDF_PLATFORM_MODULE)
$(SONIC_ONE_PDE_IMAGE)_LAZY_INSTALLS += $(DELL_S6000_PLATFORM_MODULE) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is duplicate module in one-image.mk

DEBUG_IMG="$(INSTALL_DEBUG_TOOLS)" \
DEBUG_SRC_ARCHIVE_FILE="$(DBG_SRC_ARCHIVE_FILE)" \
USERNAME="$(USERNAME)" \
PASSWORD="$(PASSWORD)" \
BUILD_TARGET="$@" \
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not this BUILD_TARGET used anyway. is there something else missing?

Copy link
Author

Choose a reason for hiding this comment

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

We pass the BUILD_TARGET to the docker build slave so we can support the build target sonic-broadcom-pde.bin to differenciate it from the normal build. Please let me know if there is a better way to handle this.

rules/config Outdated Show resolved Hide resolved
dockers/docker-pde/base_image_files/port_breakout.py Outdated Show resolved Hide resolved
dockers/docker-pde/base_image_files/port_breakout.py Outdated Show resolved Hide resolved
dockers/docker-pde/base_image_files/port_breakout.py Outdated Show resolved Hide resolved
@wbschwar wbschwar closed this May 10, 2020
@wbschwar wbschwar reopened this Jun 2, 2020
@wbschwar
Copy link
Author

Please see build tools pull request for PDE

Azure/sonic-build-tools#107

dockers/docker-pde/supervisord.conf Outdated Show resolved Hide resolved
@wbschwar wbschwar requested a review from jleveque September 24, 2020 13:57
nodaemon=true

[program:rsyslogd]
command=/usr/sbin/rsyslogd -n -iNONE"
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

@@ -216,7 +218,18 @@ SONIC_BUILD_INSTRUCTION := make \
ENABLE_SYNCHRONOUS_MODE=$(ENABLE_SYNCHRONOUS_MODE) \
$(SONIC_OVERRIDE_BUILD_VARS)

.PHONY: sonic-slave-build sonic-slave-bash init reset
PDESUBMODULES = src/sonic-linux-kernel \
Copy link
Collaborator

Choose a reason for hiding this comment

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

i do not think this is needed.

Copy link
Contributor

@geans-pin geans-pin left a comment

Choose a reason for hiding this comment

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

/azp run

@geans-pin
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 3778 in repo Azure/sonic-buildimage

@xumia
Copy link
Collaborator

xumia commented Apr 28, 2021

@geans-pin , wbschwar (PR author) can rerun it by command "/azpw run".
We need to fix the code conflict before running it.

@xumia
Copy link
Collaborator

xumia commented Apr 28, 2021

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@geans-pin
Copy link
Contributor

/azp run

@geans-pin , wbschwar (PR author) can rerun it by command "/azpw run".
We need to fix the code conflict before running it.

I think we will need to resubmit this PR

@adyeung
Copy link
Collaborator

adyeung commented May 26, 2021

Original submitter has left, pls close this code PR. Updated Code PR is tracked under #7510

@jleveque
Copy link
Contributor

Closing in favor of #7510

@jleveque jleveque closed this May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants