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

Add platform cisco-8000 to platform path #7821

Closed
wants to merge 3 commits into from
Closed

Add platform cisco-8000 to platform path #7821

wants to merge 3 commits into from

Conversation

VenkatCisco
Copy link
Contributor

Why I did it

Add new platform cisco-8000 directory under sonic-buildimage.

How I did it

create a new directory under platform called "cisco-8000" and introduce rules.mk.

How to verify it

git clone [email protected]:Azure/sonic-buildimage.git
mkdir platform/cisco-8000

make init
make configure PLATFORM=cisco-8000
make all

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

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

@lguohan lguohan requested review from qiluo-msft and xumia June 8, 2021 20:03
@anshuv-mfst
Copy link

Hi @xumia - can you please help with running re-test (azp run)

##############################################################

CISCO_PLATFORM_BRANCH := cisco-8000/master
CISCO_PLATFORM_URL := https://github.com/Cisco-8000-sonic/platform-cisco-8000.git
Copy link
Collaborator

@xumia xumia Jun 8, 2021

Choose a reason for hiding this comment

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

It is a private repo, we are not able to access it from public, it will be an issue when building cisco 8000 in azure pipeline. If the repo can not change to public, we can create a private pipeline for your repo https://github.com/Cisco-8000-sonic/platform-cisco-8000.git.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack. Please help with creating a private pipeline if that solves your access issues, as cisco would like to keep this access privileges only to MSFT.

CISCO_PLATFORM_URL := https://github.com/Cisco-8000-sonic/platform-cisco-8000.git
CISCO_PLATFORM_SRC=$(PLATFORM_PATH)/src

$(shell if [ ! -d $(CISCO_PLATFORM_SRC) ]; then git clone -b $(CISCO_PLATFORM_BRANCH) $(CISCO_PLATFORM_URL) $(CISCO_PLATFORM_SRC); fi)
Copy link
Collaborator

@xumia xumia Jun 8, 2021

Choose a reason for hiding this comment

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

We'd better to clone a specific commit id/tag, not branch, for version control.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. CISCO_PLATFORM_BRANCH Is a configureable variable. This is the first commit that brings in cisco-8000 platform into sonic-buildimage repo hence its pointing to the head of the branch. This is eventually going to be controlled by MSFT that reflects the correct commit id of the platform code in their environment.

  2. As was discussed upon in mtg with Guhon and team today, the repo is accessible only to MSFT only. So privilege is necessary for accessing https://github.com/Cisco-8000-sonic/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like its failing in which seems to be a breakage of azure/master branch. There is no code intersection. Is there a fix for this issue ?

File "/sonic/src/sonic-config-engine/tests/test_j2files.py", line 108, in test_zebra_quagga

Copy link
Collaborator

@xumia xumia Jun 10, 2021

Choose a reason for hiding this comment

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

It is another issue, build break on master(has fixed), we can re-run azp to fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which issue is this ?.

Copy link
Collaborator

@xumia xumia Jun 10, 2021

Choose a reason for hiding this comment

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

The PR to fix master build issue: #7831

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All tests were successful. Can you please review & approve this PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

@VenkatCisco , FYI, @liushilongbuaa is working on a feature to make git repo reproducible, when the feature enabled, a commit control file will be generated, we will only download specified commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xumia, Thanks for the update. How does that change impact this PR ?. If not can you please approve/merge it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The PR is significantly updated based on the offline discussion cisco team had with the PR review team from MSFT. There are some more updates checked into the PR for review.

@xumia
Copy link
Collaborator

xumia commented Jun 9, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Jun 9, 2021

What is this binary file platform/cisco-8000/.rules.mk.swp? Seems a local temp file.


In reply to: 857422274

@VenkatCisco
Copy link
Contributor Author

What is this binary file platform/cisco-8000/.rules.mk.swp? Seems a local temp file.

this looks like a file that got added in 'git add' operation. Updated changes are checked in now.

@VenkatCisco
Copy link
Contributor Author

VenkatCisco commented Jun 10, 2021 via email

@xumia
Copy link
Collaborator

xumia commented Jun 10, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@VenkatCisco VenkatCisco marked this pull request as ready for review June 10, 2021 16:51
@VenkatCisco VenkatCisco requested a review from lguohan as a code owner June 10, 2021 16:51
@VenkatCisco
Copy link
Contributor Author

What is this binary file platform/cisco-8000/.rules.mk.swp? Seems a local temp file.

In reply to: 857422274

this is fix fixed and diffs uploaded.

@VenkatCisco VenkatCisco reopened this Jun 15, 2021
@anshuv-mfst
Copy link

6/15: Code upstreaming meeting notes

  • please point to commit id instead of branch as @xumia suggested.

xumia
xumia previously approved these changes Jun 23, 2021
@xumia xumia self-requested a review July 7, 2021 01:37
@anshuv-mfst
Copy link

@xumia - can you please help with merge asap, blocking image build.

@xumia
Copy link
Collaborator

xumia commented Jul 8, 2021

@xumia - can you please help with merge asap, blocking image build.

@VenkatCisco , I added two comments, it was resolved without any comments.
Could you please help reply it? Please help confirm the repo url you refer to.

CISCO_PLATFORM_BRANCH := cisco-8000/master
CISCO_PLATFORM_URL := https://$(GITUSER):$(GITTOKEN)@Cisco-sonic-8000/platform-cisco-8000.git
CISCO_PLATFORM_SRC=$(PLATFORM_PATH)/src
CISCO_8000_SHA1 := f224715

Do we need to change to
CISCO_PLATFORM_URL := https://$(GITUSER):$(GITTOKEN)@github.com/Cisco-sonic-8000/platform-cisco-8000.git

I cannot access https://github.com/Cisco-sonic-8000, only can access https://github.com/Cisco-8000-sonic/platform-cisco-8000
The URL is in the GITTOKEN?

@VenkatCisco
Copy link
Contributor Author

Closing it to track the fix via #8172

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.

5 participants