Copilot commented on code in PR #13715:
URL: https://github.com/apache/cloudstack/pull/13715#discussion_r3655507252
##########
scripts/vm/hypervisor/kvm/patch.sh:
##########
@@ -59,16 +59,38 @@ send_file() {
virsh qemu-agent-command $name "{\"execute\":\"guest-file-close\",
\"arguments\":{\"handle\":$fd}}" > /dev/null
}
-# Wait for the guest agent to come online
+# Wait for the guest agent to come online (max 120s to avoid indefinite hang)
+# FIX: Added timeout + clear error message (GitHub Issue #13471)
+GUEST_AGENT_WAIT_TICK=0
+GUEST_AGENT_MAX_TICKS=1200 # 120s = 1200 x 0.1s
while ! virsh qemu-agent-command $name '{"execute":"guest-ping"}' >/dev/null
2>&1
do
sleep 0.1
+ GUEST_AGENT_WAIT_TICK=$((GUEST_AGENT_WAIT_TICK + 1))
+ if [ $((GUEST_AGENT_WAIT_TICK % 100)) -eq 0 ]; then
+ echo "Waiting for qemu-guest-agent to respond...
(${GUEST_AGENT_WAIT_TICK}/1200 ticks, ~$((GUEST_AGENT_WAIT_TICK / 10))s
elapsed)"
+ fi
+ if [ $GUEST_AGENT_WAIT_TICK -ge $GUEST_AGENT_MAX_TICKS ]; then
+ echo "ERROR: qemu-guest-agent not responding after 120 seconds."
Review Comment:
The progress/error messages hard-code `1200` ticks and `120` seconds, but
the loop already has `GUEST_AGENT_MAX_TICKS`. If the timeout is ever tuned, the
messages will drift from the actual limit. Use `GUEST_AGENT_MAX_TICKS` (and
derive seconds from it) in the output strings.
##########
plugins/integrations/kubernetes-service/src/main/resources/script/validate-cks-node:
##########
@@ -16,9 +16,13 @@
# specific language governing permissions and limitations
# under the License.
+# Fixed: Added qemu-guest-agent to REQUIRED_PACKAGES (GitHub Issue #13471)
+# Also removes duplicate cloud-init and gnupg entries
+
OS=`awk -F= '/^NAME/{print $2}' /etc/os-release`
REQUIRED_PACKAGES=(cloud-init cloud-guest-utils conntrack apt-transport-https
ca-certificates curl gnupg gnupg-agent \
- software-properties-common gnupg lsb-release
python3-json-pointer python3-jsonschema cloud-init containerd.io)
+ software-properties-common lsb-release
python3-json-pointer python3-jsonschema containerd.io \
+ qemu-guest-agent)
declare -a MISSING_PACKAGES
if [[ $OS == *"Ubuntu"* || $OS == *"Debian"* ]]; then
Review Comment:
In the Ubuntu/Debian path, missing packages are appended with
`MISSING_PACKAGES+="$package"`, which concatenates into a single string rather
than adding array elements. That makes `${#MISSING_PACKAGES[@]}` incorrect (it
will be `1` once any package is missing) and produces a hard-to-read package
list—more likely now that `qemu-guest-agent` was added to `REQUIRED_PACKAGES`.
Append to the array instead.
##########
scripts/vm/hypervisor/kvm/patch.sh:
##########
@@ -59,16 +59,38 @@ send_file() {
virsh qemu-agent-command $name "{\"execute\":\"guest-file-close\",
\"arguments\":{\"handle\":$fd}}" > /dev/null
}
-# Wait for the guest agent to come online
+# Wait for the guest agent to come online (max 120s to avoid indefinite hang)
+# FIX: Added timeout + clear error message (GitHub Issue #13471)
+GUEST_AGENT_WAIT_TICK=0
+GUEST_AGENT_MAX_TICKS=1200 # 120s = 1200 x 0.1s
while ! virsh qemu-agent-command $name '{"execute":"guest-ping"}' >/dev/null
2>&1
do
sleep 0.1
+ GUEST_AGENT_WAIT_TICK=$((GUEST_AGENT_WAIT_TICK + 1))
+ if [ $((GUEST_AGENT_WAIT_TICK % 100)) -eq 0 ]; then
+ echo "Waiting for qemu-guest-agent to respond...
(${GUEST_AGENT_WAIT_TICK}/1200 ticks, ~$((GUEST_AGENT_WAIT_TICK / 10))s
elapsed)"
+ fi
+ if [ $GUEST_AGENT_WAIT_TICK -ge $GUEST_AGENT_MAX_TICKS ]; then
+ echo "ERROR: qemu-guest-agent not responding after 120 seconds."
+ echo "The VM template is missing 'qemu-guest-agent' or the service is
not running."
+ echo "Required packages: cloud-init, qemu-guest-agent,
cloud-guest-utils, conntrack, containerd.io"
+ echo "See:
https://docs.cloudstack.apache.org/en/latest/kubernetes/kubernetes-cluster-requirements.html";
+ exit 1
+ fi
done
+echo "qemu-guest-agent is responsive."
-# Test guest agent sanity
-while [ "$(virsh qemu-agent-command $name
'{"execute":"guest-sync","arguments":{"id":1234567890}}' 2>/dev/null)" !=
'{"return":1234567890}' ]
-do
+# Test guest agent sanity (bounded to 30s)
+# FIX: Added timeout (GitHub Issue #13471)
+GUEST_SYNC_TICK=0
+GUEST_SYNC_MAX_TICKS=300 # 30s
+while [ "$(virsh qemu-agent-command $name
'{"execute":"guest-sync","arguments":{"id":1234567890}}' 2>/dev/null)" !=
'{"return":1234567890}' ]; do
sleep 0.1
+ GUEST_SYNC_TICK=$((GUEST_SYNC_TICK + 1))
+ if [ $GUEST_SYNC_TICK -ge $GUEST_SYNC_MAX_TICKS ]; then
+ echo "ERROR: qemu-guest-agent sanity check (guest-sync) failed after
30 seconds."
Review Comment:
The guest-sync timeout message hard-codes "30 seconds" even though the limit
is defined by `GUEST_SYNC_MAX_TICKS`. Derive the seconds from the constant to
keep the message accurate if the timeout is adjusted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
