Copilot commented on code in PR #13714:
URL: https://github.com/apache/cloudstack/pull/13714#discussion_r3655506357
##########
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
Review Comment:
`validate-cks-node` is executed for cluster nodes without a hypervisor-type
guard (e.g. KubernetesClusterAddWorker.validateNodeCompatibility), but
`qemu-guest-agent` is only relevant for KVM/QEMU guests. Requiring it
unconditionally can block adding nodes on other hypervisors/templates. Make the
`qemu-guest-agent` requirement conditional on detecting KVM/QEMU virtualization.
##########
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, even
though the script defines GUEST_AGENT_MAX_TICKS. If the tick constants change,
the log output becomes misleading. Use GUEST_AGENT_MAX_TICKS (and derive
seconds from it) in the messages.
##########
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
Review Comment:
This script runs with `set -e`, and `virsh qemu-agent-command` can return
non-zero while the guest agent is still coming up. Because it's executed inside
a command substitution, bash may abort the script immediately instead of
retrying and eventually timing out. Append `|| true` (and quote $name) so the
loop controls the retry/timeout behavior.
--
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]
