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

[mellanox]: Disable caching for the issu-version file #9235

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

saiarcot895
Copy link
Contributor

Signed-off-by: Saikrishna Arcot sarcot@microsoft.com

Why I did it

This is to work around the current build issue where Mellanox builds are
failing. This is so that issu-version is always "built", so that copy is
made into the bullseye directory.

How I did it

The issu-version file for Mellanox is generated from the Mellanox SDK
libraries. The SDK is installed into a Buster docker container, but the
issu-version file goes onto the base OS, which is Bullseye. To work
around this, the issu-version build rules explicitly copies the
issu-version file to target/files/bullseye/ during the Buster build.

Because of our build infra, if caching is enabled and a cache is being
used, then for issu-version, since it is technically built as part of
Buster, then only target/files/buster/issu-version is saved into the
cache, and target/files/bullseye/issu-version isn't cached. If this
cache gets used, then target/files/bullseye/issu-version is missing, and
the final image build fails.

How to verify it

The CI build for the PR should be sufficient to verify it. If the mellanox build there succeeds, then the regular master branch build should succeed as well.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

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

The issu-version file for Mellanox is generated from the Mellanox SDK
libraries. The SDK is installed into a Buster docker container, but the
issu-version file goes onto the base OS, which is Bullseye. To work
around this, the issu-version build rules explicitly copies the
issu-version file to target/files/bullseye/ during the Buster build.

Because of our build infra, if caching is enabled and a cache is being
used, then for issu-version, since it is technically built as part of
Buster, then only target/files/buster/issu-version is saved into the
cache, and target/files/bullseye/issu-version isn't cached. If this
cache gets used, then target/files/bullseye/issu-version is missing, and
the final image build fails.

This is to work around the current build issue where Mellanox builds are
failing. This is so that issu-version is always "built", so that copy is
made into the bullseye directory.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
@lguohan
Copy link
Collaborator

lguohan commented Nov 12, 2021

mellanox effort: #9234

@saiarcot895
Copy link
Contributor Author

I should've checked for opened PRs first.

@alexrallen, BLDENV isn't available in the Makefiles because it's not exported from slave.mk, which explains why your changes are failing. Otherwise, it should've worked. This also means that the kernel modules for mft aren't getting built, since that uses BLDENV as well. (That being said, the image build should've tried to install the kernel modules and failed, so I'm not sure why.)

@lguohan lguohan merged commit a980e83 into sonic-net:master Nov 12, 2021
@lguohan
Copy link
Collaborator

lguohan commented Nov 12, 2021

@alexrallen , i merged this pr to unblock.

@saiarcot895 saiarcot895 deleted the fix-mellanox-bullseye branch November 15, 2021 18:30
qiluo-msft pushed a commit that referenced this pull request Nov 16, 2021
#### Why I did it

Mellanox builds were failing intermittently due to the `issue_version` file and MFT package not building correctly in the Azure pipeline environment (both of these packages were patched to build correctly with bullseye running on the host and buster running on the dockers)

#### How I did it

Fixed two problems:

1. BLDENV is not passed to the Makefiles so the references to this were replaced with correct logic
2. `issue_version` was not defined as a target for bullseye and as such was not cached. Altered the build such that it is defined as a target for bullseye (in the case of buster it builds the file, in the case of bullseye it copies from buster) 

The previous PR fixing this was reverted as it is no longer necessary for a passing build and was not a long-term fix. #9235

#### How to verify it

Build on AZP and verify success.
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.

3 participants