-
Notifications
You must be signed in to change notification settings - Fork 196
ci: detect the existence of yq before using filter scheme on aarch64 #868
Conversation
.ci/aarch64/filter_docker_aarch64.sh
Outdated
# install yq if not exist | ||
if ! command -v yq >/dev/null; then | ||
install_yq | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix the indentation here - you seen to have three levels of it :)
btw, you could simplify to something like the following:
[ -z "$(command -v yq)" ] && install_yq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right. prefer the simplification. 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @Pennyzct, this looks good, let me know what you think about the comments I left.
Also, theres' a small typo in the commit body (schme).
.ci/lib.sh
Outdated
@@ -118,7 +118,7 @@ function install_yq() { | |||
yq_version=$(basename "${yq_latest_url}") | |||
|
|||
local yq_url="https://${yq_pkg}/releases/download/${yq_version}/yq_${goos}_${goarch}" | |||
curl -o "${yq_path}" -L ${yq_url} | |||
curl -o "${yq_path}" -Ls ${yq_url} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add -S
(uppercase) to the flags so that errors could still be printed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right. originally, I just wanted to get rid of these download info,
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 116 0 116 0 0 87 0 --:--:-- 0:00:01 --:--:-- 87
100 64230 0 64230 0 0 26662 0 --:--:-- 0:00:02 --:--:-- 120k
which I don't know why it has been sent into stderr. but only -s
could also skip real err info.
.ci/aarch64/filter_docker_aarch64.sh
Outdated
@@ -33,6 +35,10 @@ filter_and_build() | |||
|
|||
main() | |||
{ | |||
# install yq if not exist | |||
if ! command -v yq >/dev/null; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yq is installed in the non standard PATH ${GOPATH_LOCAL}/bin/
, so maybe it's better to check for ${GOPATH_LOCAL}/bin/yq
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but since CI on aarch64 is on bare-metal machine, there are possibilities that yq has already been installed in some other PATH, /usr/bin
, /usr/local/bin
, etc. ☹
.ci/aarch64/filter_test_aarch64.sh
Outdated
main() | ||
{ | ||
# install yq if not exist | ||
if ! command -v yq >/dev/null; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment as before
btw, the ARM job for this repo can be triggered using: /test-arm |
@chavafg
I will add a new commit to fix it. |
/test-arm |
ARM CI seemed to have a network timeout issue - have nudged a rebuild. |
Getting the following error now, that seems to be related to time settings not correctly configured on the machine:
@Pennyzct could you try to fix this for ARM? You may need to do something like |
Hi~ I will lately log into CI ARM machine on packet.net to try to fix this. However, when I was trying to run the whole CI tests on my local machine, and found that there are quite a few new docker integration tests uploaded, and large portion of them couldn't work well on ARM, such as the new memory hot pluggable test(memory hot plug not supported on arm for now). so maybe I will raise a new issue to configure the filter to skip these tests. |
thanks @Pennyzct, that's a good idea :) |
/test-arm |
Hi~@marcov I have already |
let's hope :) |
Think armCI hit a timeout:
let's nudge it again - and see if that is repeatable etc. /test-arm |
And now we are still getting this:
This happens in a container images, so I exclude it's something related to the SSL certs installed, and the image sha in the log is up to date. Checking for
@Pennyzct some things you could try doing:
|
/test |
Hi~@marcov thanks for the detailed instruction!;) The whole stress image building was successful on my local ARM machine, and I will try it on packet.net's machine asap. |
The time in container is inconsistent with the time in host, which i think it should be the same in default.
I have tried multiple images, such like alpine, ubuntu, etc, it share the same output.
Above errors only occur in this machine on packet.net, and won't occur in my other local ARM machine. |
hi @Pennyzct, thanks for the investigation! |
Nice find, and yeah, that to me feels like it is to do with how the time (actually, I suspect the timezone) is mapped (or not) into the container. More investigation needed. I agree with @marcov that I doubt a docker re-install will change this. |
We have resolved it! 🎊🎉🎊🎉 it’s because that the device rtc is missing in VM. And Wei will refine the kernel config to fix it asap. |
@Pennyzct kata-containers/packaging#239 this one fix it : ) |
kata-containers/packaging#239 was just merged, so let's re-test |
@Pennyzct can you please check the docker installation on the ARM CI machine? It's not finding the docker binary
|
Hmm, that error looks to be in the cleanup scripts - maybe that is not invalid (and/or should not be a fatal error).
I wonder if the docker repos are down maybe... |
@grahamwhaley, docker released new 18.09, but seems like now the package is |
Since that filter scheme on aarch64 bare-metal machine would fail if it is lack of yq, we should detect the existence of yq before going into and also install it if it is really missing. Fixes: kata-containers#867 Jira: ENTOS-480 Change-Id: I361e55449df618e2129f4957aff537fd38995b9b Signed-off-by: Penny Zheng penny.zheng@arm.com
/test |
/test-arm |
still getting the Seems like we still have an issue on baremetal machines where docker is not installed. /cc @grahamwhaley |
Ah, OK - we should probably either not have |
so sorry for the delay, got offline for a few days due to the offsite meeting. |
/test-arm |
I have reserved 10 mins for
I have logged into ARM CI machine and run the all docker integration tests, and it turned out that it had passed the
|
Hi~ @jodh-intel the missing docker has already been mentioned and resolved here, and after that, @marcov has helped me re-triggered ARM CI here. However, it still got stuck by time-out. |
I think the correct route to enable you to do this @Pennyzct is to add you to the kata github community (project) :-) To do that I think iirc you need to become a kata maintainer. First, let me ask, are you OK taking on the responsibility that brings? :-) |
@Pennyzct - heh, I'm now running into the 'docker not found' thing on my metrics machines - let me go do that 'cmd docker' checking PR to the cleanup scripts.... yeah, the cleanup scripts should never fail - I might also just |
@Pennyzct I increased the timeout from 5 to 10 minutes to see how this goes. I have also added you as an admin on this arm job, so you should be able to re-trigger using: /test-arm Let me know if you find an issue triggering the job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Finally, we got into this output😭:
@chavafg thanks for admin authority and increasing the time-out level. |
@grahamwhaley thanks for the proposal, very honored to be taking on responsibility ;) and very happy to continue working on ARM CI and related ARM issues, with extreme help from my mentor @Weichen81. |
@Pennyzct looking forward to getting more contributions from you and other ARM folks. 👍 |
/test |
Hi~all @marcov @grahamwhaley @jodh-intel this one also got green-check on ARM CI ;). |
Since that filter schme on aarch64 bare-metal machine would fail if it is lack of yq, we should detect the existence of yq before going into and also install it if it is really missing.
Fixes: #867
Signed-off-by: Penny Zheng penny.zheng@arm.com