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.

Sign up for GitHub

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

Jump to bottom

.travis: fix failure to install clang-10 on Arm64 #11418

Merged
merged 1 commit into from May 8, 2020
Merged

.travis: fix failure to install clang-10 on Arm64 #11418

merged 1 commit into from May 8, 2020

Conversation

Jianlin-lv
Copy link
Contributor

@Jianlin-lv Jianlin-lv commented May 8, 2020 •
edited by tklauser

source.list repository does not have a release clang-10,
Install clang-10 pre-built binaries from github release

Signed-off-by: Jianlin Lv Jianlin.Lv@arm.com

issue as:
https://travis-ci.com/github/cilium/cilium/jobs/329541260#L390

@Jianlin-lv Jianlin-lv requested a review from a team as a code owner May 8, 2020 07:56
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 8, 2020
@tklauser tklauser added the release-note/misc This PR makes changes that have no direct user impact. label May 8, 2020
tklauser
tklauser requested changes May 8, 2020
View reviewed changes
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Would be nice if we could dowload the .sig file for the clang tarball as well and verify the signature before installing. Do you think this would be possible to add?

.travis/prepare.sh Outdated
Comment on lines 31 to 32
NEWPATH="/usr/local/clang/bin"
export PATH="$NEWPATH:$PATH"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: these two lines could be combined as

Suggested change
NEWPATH="/usr/local/clang/bin"
export PATH="$NEWPATH:$PATH"
export PATH="/usr/local/clang/bin:$PATH"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, Please review.

@tklauser tklauser requested a review from pchaigno May 8, 2020 08:07
pchaigno
pchaigno approved these changes May 8, 2020
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the build @Jianlin-lv!

.travis/prepare.sh Show resolved Hide resolved
@coveralls
Copy link

coveralls commented May 8, 2020 •
edited

Coverage Status

Coverage decreased (-0.01%) to 37.867% when pulling 45e9665 on Jianlin-lv:pr-fix-clang-10-arm into dac186c on cilium:master.

tklauser
tklauser requested changes May 8, 2020
View reviewed changes
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Thanks, looks good mostly. Some small remarks inline.

.travis/prepare.sh Outdated

if gpg --verify $CLANG_FILE_SIG $CLANG_FILE
then
echo $CLANG_FILE signed sucessfully
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo $CLANG_FILE signed sucessfully
echo $CLANG_FILE verified successfully

.travis/prepare.sh Outdated
then
echo $CLANG_FILE signed sucessfully
else
echo ERROR: Failed to sign $CLANG_FILE
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo ERROR: Failed to sign $CLANG_FILE
echo ERROR: Failed to verify $CLANG_FILE

.travis/prepare.sh Show resolved Hide resolved
.travis/prepare.sh Outdated
CLANG_URL="https://github.com/llvm/llvm-project/releases/download/llvmorg-$CLANG_VERSION/$CLANG_FILE_SIG"
wget -nv $CLANG_URL

wget -nv https://releases.llvm.org/3.7.0/hans-gpg-key.asc
Copy link
Member

Choose a reason for hiding this comment

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

For consistency we might want to use the public key file for the same release:

Suggested change
wget -nv https://releases.llvm.org/3.7.0/hans-gpg-key.asc
wget -nv https://releases.llvm.org/$CLANG_VERSION/hans-gpg-key.asc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good comments, Thanks
I have updated this patch.

@Jianlin-lv
.travis: fix install clang-10 fail issue on Arm64
45e9665 
source.list repository does not have a release clang-10,
Install clang-10 pre-built binaries from github release

Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
tklauser
tklauser approved these changes May 8, 2020
View reviewed changes
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@joestringer
Copy link
Member

joestringer commented May 8, 2020 •
edited

I noticed this in the aarch64 build on Travis:

go build k8s.io/kube-openapi/pkg/util/proto: write /tmp/go-build285141712/b636/_pkg_.a: disk quota exceeded
# cover github.com/cilium/cilium/pkg/client
2020/05/08 12:37:42 cover: open $WORK/b224/client.cover.go: disk quota exceeded
FAIL	github.com/cilium/cilium/pkg/k8s [build failed]
FAIL
make: *** [unit-tests] Error 1
Makefile:184: recipe for target 'unit-tests' failed

Let's keep an eye on this. It might make sense to also remove the clang tarball from the filesystem before running the tests to prevent it from consuming disk quota that we need for writing test artifacts.

@joestringer joestringer merged commit d3d0f82 into cilium:master May 8, 2020
1.8.0 automation moved this from In progress to Merged May 8, 2020
@Jianlin-lv
Copy link
Contributor Author

I noticed this in the aarch64 build on Travis:

go build k8s.io/kube-openapi/pkg/util/proto: write /tmp/go-build285141712/b636/_pkg_.a: disk quota exceeded
# cover github.com/cilium/cilium/pkg/client
2020/05/08 12:37:42 cover: open $WORK/b224/client.cover.go: disk quota exceeded
FAIL	github.com/cilium/cilium/pkg/k8s [build failed]
FAIL
make: *** [unit-tests] Error 1
Makefile:184: recipe for target 'unit-tests' failed

Let's keep an eye on this. It might make sense to also remove the clang tarball from the filesystem before running the tests to prevent it from consuming disk quota that we need for writing test artifacts.

I think this may be an issue of Travis LXD container,
Someone has reported this issue in Travis CI Community Forums, I have also feedback on our situation and keep track of the progress

https://travis-ci.community/t/disk-quota-exceeded-on-ppc64le/8006/8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tklauser tklauser tklauser approved these changes

@joestringer joestringer joestringer approved these changes

@pchaigno pchaigno pchaigno approved these changes

Assignees
No one assigned
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Jianlin-lv @coveralls @joestringer @tklauser @pchaigno