-
Notifications
You must be signed in to change notification settings - Fork 164
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
Wait more for newlog and vlans #2724
Conversation
pkg/newlog/newlogd-init.sh
Outdated
LOOP_COUNT=$((LOOP_COUNT + 1)) | ||
NEWLOGD_PID=$(cat /run/newlogd.pid) | ||
sleep 3 | ||
LOOP_COUNT=$((LOOP_COUNT + 3)) |
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.
now it is not LOOP_COUNT
anymore, but rather seconds_passed
. Also why 3 seconds?
Anyways, worth renaming even if it was 1 second each iteration as it better describes the purpose of the variable
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.
+1
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.
Actually it comes from device-steps. I added another message about in progress of waiting and will show it every 3 seconds as well.
pkg/newlog/newlogd-init.sh
Outdated
if [ ! -f "$NEWLOGD_TOUCH_FILE" ]; then | ||
echo "$(date -Ins -u) gave up waiting for $NEWLOGD_TOUCH_FILE" | ||
else | ||
echo "$(date -Ins -u) waited $LOOP_COUNT for $NEWLOGD_TOUCH_FILE" |
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.
NIT: waited $LOOP_COUNT seconds
pkg/newlog/newlogd-init.sh
Outdated
mkdir -p /run/watchdog/pid | ||
touch /run/watchdog/pid/newlogd.pid | ||
NEWLOGD_PID=$! | ||
echo "$(date -Ins -u) newlogd is starting..., pid $NEWLOGD_PID" |
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.
NIT: here and other invocations of data
- for readability it's always better to use full form of arguments in the scripts
--iso-8601=ns --utc
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.
Good point, modified. Another interesting thing here is that busybox's date from alpine 3.13 do not support -Ins
, only -Is
so I was forced to add coreutils
package.
BusyBox v1.32.1 () multi-call binary.
Usage: date [OPTIONS] [+FMT] [TIME]
Display time (using +FMT), or set time
[-s,--set] TIME Set time to TIME
-u,--utc Work in UTC (don't convert to local time)
-R,--rfc-2822 Output RFC-2822 compliant date string
-I[SPEC] Output ISO-8601 compliant date string
SPEC='date' (default) for date only,
'hours', 'minutes', or 'seconds' for date and
time to the indicated precision
-r,--reference FILE Display last modification time of FILE
-d,--date TIME Display TIME, not 'now'
-D FMT Use FMT (strptime format) for -d TIME conversion
Recognized TIME formats:
hh:mm[:ss]
[YYYY.]MM.DD-hh:mm[:ss]
YYYY-MM-DD hh:mm[:ss]
[[[[[YY]YY]MM]DD]hh]mm[.ss]
'date TIME' form accepts MMDDhhmm[[YY]YY][.ss] instead
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.
forced to add coreutils package
Yes, I ran into this earlier too. Adding coreutils is the right way to solve that, I think
We start newlogd as a long-running application, and do not expect that it will change its pid. Let's check for touch file as we do it for another agents and increase wait time. Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
Previously, in the switch_net_vlans test on rpi4 with hypervisor xen, an error was very likely to occur, now 10 out of 10 switch_net_vlans tests pass without problems. |
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
Tests for RPi (ROL) indicate that our timeouts for newlog and vlans do not cover real workloads (for slowest case with RPi running xen and zfs).