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

.travis: Adjust travis related code #10327

Merged
merged 1 commit into from
Feb 25, 2020
Merged

Conversation

iecedge
Copy link

@iecedge iecedge commented Feb 25, 2020

Add arch key to facilitate subsequent support of multiple CPU arch;
Add function to install clang distinguish by CPU

Signed-off-by: Jianlin Lv [email protected]

Fixes: #9898


This change is Reviewable

@iecedge iecedge requested a review from a team as a code owner February 25, 2020 07:29
@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

@pchaigno pchaigno added area/CI Continuous Integration testing issue or flake pending-review release-note/misc This PR makes changes that have no direct user impact. labels Feb 25, 2020
@coveralls
Copy link

coveralls commented Feb 25, 2020

Coverage Status

Coverage decreased (-0.02%) to 45.482% when pulling c56ec30 on iecedge:pr-travis-multi-arch into 37dd01d on cilium:master.

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.

I checked that the initial Travis CI build setup works for aarch64 as well. (It fails in the unit tests.)

@iecedge
Copy link
Author

iecedge commented Feb 25, 2020

I checked that the initial Travis CI build setup works for aarch64 as well. (It fails in the unit tests.)

I checked that the initial Travis CI build setup works for aarch64 as well. (It fails in the unit tests.)

I`m working on unit tests to supprot Arm64 and trying to fixed currently issue;
Also welcome other developers to participate in this feature.

;;
'aarch64' )
CLANG_DIR="clang+llvm-$CLANG_VERSION-aarch64-linux-gnu"
sed -i 's/etcd:v3.2.17/etcd:v3.2.17-arm64/g' $GOPATH/src/github.com/cilium/cilium/Makefile.defs
Copy link
Member

Choose a reason for hiding this comment

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

instead of using sed, can't you run make with ETCD_IMAGE set depending on the architecture?

Copy link
Author

@iecedge iecedge Feb 25, 2020

Choose a reason for hiding this comment

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

instead of using sed, can't you run make with ETCD_IMAGE set depending on the architecture?

Conditional statements can be added to Makefile.defs, for example:

+ifeq ($(shell uname -m),aarch64)
+    ETCD_IMAGE=quay.io/coreos/etcd:v3.2.17-arm64
+else
+    ETCD_IMAGE=quay.io/coreos/etcd:v3.2.17
+endif

How about you opinion on this modification?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking on having it in ./.travis/build.sh but the conditional statement in the Makefile.defs sounds good to me as well.

Copy link
Author

Choose a reason for hiding this comment

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

Done

.travis.yml Show resolved Hide resolved
Add arch key to facilitate subsequent support of multiple CPU arch;
Add function to install clang distinguish by CPU

Signed-off-by: Jianlin Lv <[email protected]>
@iecedge iecedge force-pushed the pr-travis-multi-arch branch from 9c0dd4e to c56ec30 Compare February 25, 2020 13:26
@aanm
Copy link
Member

aanm commented Feb 25, 2020

test-me-please

@tgraf tgraf merged commit 3d81edb into cilium:master Feb 25, 2020
@iecedge iecedge deleted the pr-travis-multi-arch branch February 26, 2020 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cilium enable on aarch64
6 participants