<fix>[compute]: set vm bind with current cluster uuid#4155
Open
zstack-robot-2 wants to merge 1 commit into
Open
<fix>[compute]: set vm bind with current cluster uuid#4155zstack-robot-2 wants to merge 1 commit into
zstack-robot-2 wants to merge 1 commit into
Conversation
when vm migrate to other cluster, set vm bind with current cluster uuid, and remove the bind cluster uuid from system tag, and update system tag value Resolves: ZSTAC-66476 Change-Id: I617a776b726161746c686f696e68637565676c72 (cherry picked from commit 06af535)
Walkthrough在 Changes集群绑定标签同步
🎯 评估🎯 2 (Simple) | ⏱️ ~8 分钟 Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java (1)
93-104: 💤 Low value建议抽取独立方法以降低嵌套层级。
此清理逻辑形成了三层嵌套（
if (!resourceConfig)→if (containsKey)→if (!contains)）。可将集群绑定的对齐/清理逻辑抽成一个语义清晰的私有方法（如syncClusterBindingTag()），既符合单一职责，也便于阅读与维护。As per coding guidelines: “限制嵌套层级最多为两层，且内层不应再出现 else 分支”，并“将条件判断中的处理逻辑提前结束或抽成独立方法”。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java` around lines 93 - 104, The nested cleanup/alignment logic around cluster binding in ResourceBindingAllocatorFlow (the block using resources, clusterCollector, spec and VmSystemTags.VM_RESOURCE_BINGDING/VM_RESOURCE_BINGDING_TOKEN) should be extracted into a private method (e.g., syncClusterBindingTag()) that takes the resources map, clusterCollector and spec as parameters; move the three-level-nested ifs into that method, perform early exits inside it, and replace the original inline block with a single call to syncClusterBindingTag() to reduce nesting and improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java`:
- Around line 96-101: The update currently calls
VmSystemTags.VM_RESOURCE_BINGDING.updateTagByToken(...) with
newTag="Cluster:..." which replaces the entire resourceUUids token and wipes
other bindings (Host:...). Change the flow to read the existing token via
getBindedResourcesFromTag(spec.getVmInstance().getUuid(),
VM_RESOURCE_BINGDING_TOKEN), parse it into a map of type->uuid, replace or add
the Cluster entry with spec.getVmInstance().getClusterUuid(), reserialize the
map back to the resourceUUids string, and then call
VM_RESOURCE_BINGDING.updateTagByToken(...) with the original oldToken and the
new token string; keep ResourceBindingHostCollector.getType() handling and only
remove resources from the local resources map as before. Also extract the nested
if logic into a helper method to satisfy code structure requirements.
---
Nitpick comments:
In
`@compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java`:
- Around line 93-104: The nested cleanup/alignment logic around cluster binding
in ResourceBindingAllocatorFlow (the block using resources, clusterCollector,
spec and VmSystemTags.VM_RESOURCE_BINGDING/VM_RESOURCE_BINGDING_TOKEN) should be
extracted into a private method (e.g., syncClusterBindingTag()) that takes the
resources map, clusterCollector and spec as parameters; move the
three-level-nested ifs into that method, perform early exits inside it, and
replace the original inline block with a single call to syncClusterBindingTag()
to reduce nesting and improve readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fc55616a-1a7e-4306-9621-6b95ef0838ca
📒 Files selected for processing (1)
compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java
Comment on lines
+96
to
+101
| if (!uuids.contains(spec.getVmInstance().getClusterUuid())) { | ||
| String tag = String.format("Cluster:%s", spec.getVmInstance().getClusterUuid()); | ||
| VmSystemTags.VM_RESOURCE_BINGDING.updateTagByToken(spec.getVmInstance().getUuid(), | ||
| VmSystemTags.VM_RESOURCE_BINGDING_TOKEN, tag); | ||
| } | ||
| resources.remove(clusterCollector.getType()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查找所有 ResourceBindingCollector 实现及其 getType() 返回值
ast-grep --pattern 'class $_ implements ResourceBindingCollector {
$$$
}'
echo "----- getType 返回值 -----"
rg -nP -C2 'class\s+\w+\s+implements\s+ResourceBindingCollector' --type=java
rg -nP -A6 'public String getType\(\)' --type=javaRepository: MatheMatrix/zstack
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List ResourceBindingCollector implementations (compute allocator package only)
fd -e java "ResourceBinding*Collector*.java" compute/src/main/java/org/zstack/compute/allocator -a \
| head -n 200
echo "----- classes implementing ResourceBindingCollector (allocator package) -----"
rg -n "implements\s+ResourceBindingCollector" compute/src/main/java/org/zstack/compute/allocator --type=java -S
echo "----- getType() values for those collectors -----"
rg -n "class\s+(\w+)\s+implements\s+ResourceBindingCollector" compute/src/main/java/org/zstack/compute/allocator --type=java -S -o --replace '$1' \
| sort -u > /tmp/collector_classes.txt
python3 - <<'PY'
import re, pathlib
base = pathlib.Path("compute/src/main/java/org/zstack/compute/allocator")
txt = pathlib.Path("/tmp/collector_classes.txt").read_text().splitlines()
classes = [t.strip() for t in txt if t.strip()]
for cls in classes:
# Find the file for the class
matches = list(base.glob(f"**/{cls}.java"))
if not matches:
# fallback: any file containing class declaration
for p in base.glob("**/*.java"):
s = p.read_text(errors="ignore")
if re.search(rf"class\s+{re.escape(cls)}\s+extends?\s+\w*.*implements\s+ResourceBindingCollector', s):
matches.append(p)
break
if not matches:
continue
p = matches[0]
# Extract getType return value line(s) in that file
content = p.read_text(errors="ignore")
m = re.search(r"public\s+String\s+getType\s*\(\)\s*\{\s*return\s*([^;]+);", content, re.S)
if m:
expr = " ".join(m.group(1).split())
print(f"{cls}: {expr} (file: {p.as_posix()})")
else:
# if getType not in simple return form, just print the method block header
m2 = re.search(r"public\s+String\s+getType\s*\([^)]*\)\s*\{", content)
print(f"{cls}: getType() found but return not in simple form (file: {p.as_posix()})")
PYRepository: MatheMatrix/zstack
Length of output: 765
🏁 Script executed:
#!/bin/bash
set -euo pipefail
HOST_FILE="compute/src/main/java/org/zstack/compute/allocator/ResourceBindingHostCollector.java"
CLUSTER_FILE="compute/src/main/java/org/zstack/compute/allocator/ResourceBindingClusterCollector.java"
echo "----- collectors in allocator package -----"
rg -n "class\s+ResourceBinding\w+Collector\s+implements\s+ResourceBindingCollector" compute/src/main/java/org/zstack/compute/allocator --type=java
echo "----- getType() return values -----"
echo "== $HOST_FILE =="
rg -n -C2 "String\s+getType\s*\(" "$HOST_FILE" --type=java
rg -n -C2 "return\s" "$HOST_FILE" --type=java | rg -n "getType|return" --max-count 20 || true
echo "== $CLUSTER_FILE =="
rg -n -C2 "String\s+getType\s*\(" "$CLUSTER_FILE" --type=java
rg -n -C2 "return\s" "$CLUSTER_FILE" --type=java | rg -n "getType|return" --max-count 20 || trueRepository: MatheMatrix/zstack
Length of output: 1059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java"
# Show surrounding code for the reported lines
sed -n '60,140p' "$FILE" | cat -n
echo "----- find getBindedResourcesFromTag -----"
rg -n "getBindedResourcesFromTag|bindedResourcesFromTag|SPLIT|split\\(\"\\,\"\\)|VM_RESOURCE_BINGDING" "$FILE" --type=java
echo "----- show full helper(s) around parsing tag -----"
# print 20 lines before/after the match locations (limit output)
for m in $(rg -n "getBindedResourcesFromTag|VM_RESOURCE_BINGDING_TOKEN" "$FILE" --type=java -o -S | head -n 20); do
:
done
# Use awk to extract blocks near likely helpers (simple heuristic)
rg -n "getBindedResourcesFromTag|VM_RESOURCE_BINGDING" "$FILE" --type=java | head -n 20
# Locate VmSystemTags definition
echo "----- locate VmSystemTags VM_RESOURCE_BINGDING -----"
rg -n "VM_RESOURCE_BINGDING" -S . --type=java --hidden --glob '!**/target/**' --max-count 50
# Show VmSystemTags.updateTagByToken implementation
echo "----- locate updateTagByToken implementation -----"
rg -n "updateTagByToken\\(" -S . --type=java --max-count 50Repository: MatheMatrix/zstack
Length of output: 4239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) 找 ResourceBindingAllocatorFlow 里的 getBindedResourcesFromTag 实现
FILE="compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java"
echo "----- getBindedResourcesFromTag definition (in flow file) -----"
rg -n "getBindedResourcesFromTag" "$FILE" --type=java
rg -n "Map<\s*String\s*,\s*List<\s*String\s*>\s*>\s+getBindedResourcesFromTag|getBindedResourcesFromTag\s*\(" "$FILE" --type=java -n
# 输出方法附近若干行（避免太长）
line=$(rg -n "getBindedResourcesFromTag" "$FILE" --type=java | head -n 1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-20))
end=$((line+120))
sed -n "${start},${end}p" "$FILE" | cat -n
fi
# 2) 找 VmSystemTags 里 VM_RESOURCE_BINGDING / updateTagByToken 的实现
echo "----- locate VmSystemTags VM_RESOURCE_BINGDING -----"
rg -n "VM_RESOURCE_BINGDING" -S . --type=java --hidden --glob '!**/target/**' --max-count 50
# locate the class that declares VM_RESOURCE_BINGDING
TAG_DEF_FILE=$(rg -n "VM_RESOURCE_BINGDING" -S . --type=java --hidden --glob '!**/target/**' --max-count 50 | head -n 1 | cut -d: -f1 || true)
echo "TAG_DEF_FILE=$TAG_DEF_FILE"
# 输出 VM_RESOURCE_BINGDING 相关定义附近
if [ -n "${TAG_DEF_FILE:-}" ] && [ -f "$TAG_DEF_FILE" ]; then
sed -n '1,200p' "$TAG_DEF_FILE" | cat -n | head -n 200 || true
# 更精确：输出含 VM_RESOURCE_BINGDING 这行附近
tline=$(rg -n "VM_RESOURCE_BINGDING" "$TAG_DEF_FILE" --type=java | head -n 1 | cut -d: -f1 || true)
if [ -n "${tline:-}" ]; then
start=$((tline-40))
end=$((tline+120))
sed -n "${start},${end}p" "$TAG_DEF_FILE" | cat -n
fi
fi
# 3) 直接搜 updateTagByToken 的方法体（可能在 SystemTag 或某个 tag class）
echo "----- locate updateTagByToken implementation -----"
rg -n "updateTagByToken\s*\(" -S . --type=java --hidden --glob '!**/target/**' --max-count 50
UPD_FILE=$(rg -n "updateTagByToken\s*\(" -S . --type=java --hidden --glob '!**/target/**' --max-count 50 | head -n 1 | cut -d: -f1 || true)
echo "UPD_FILE=$UPD_FILE"
if [ -n "${UPD_FILE:-}" ] && [ -f "$UPD_FILE" ]; then
uline=$(rg -n "updateTagByToken\s*\(" "$UPD_FILE" --type=java | head -n 1 | cut -d: -f1 || true)
start=$((uline-60))
end=$((uline+140))
sed -n "${start},${end}p" "$UPD_FILE" | cat -n
fiRepository: MatheMatrix/zstack
Length of output: 25782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tag/src/main/java/org/zstack/tag/PatternedSystemTag.java"
echo "----- getTokenByResourceUuid implementation -----"
line=$(rg -n "getTokenByResourceUuid\\(" "$FILE" --type=java | head -n 1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-40))
end=$((line+140))
sed -n "${start},${end}p" "$FILE" | cat -n
fi
echo "----- tagFormat for VM_RESOURCE_BINGDING (VmSystemTags.java) -----"
VM_FILE="compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java"
rg -n "VM_RESOURCE_BINGDING_TOKEN|VM_RESOURCE_BINGDING\s*=" "$VM_FILE" --type=java
tline=$(rg -n "VM_RESOURCE_BINGDING\\s*=" "$VM_FILE" --type=java | head -n 1 | cut -d: -f1)
sed -n "$((tline-10)),$((tline+30))p" "$VM_FILE" | cat -nRepository: MatheMatrix/zstack
Length of output: 9263
VM_RESOURCE_BINGDING.updateTagByToken 仅写入 Cluster，会覆盖整个 resourceUUids 串导致 Host 等绑定丢失（major）
getBindedResourcesFromTag()把VM_RESOURCE_BINGDING_TOKEN=resourceUUids解析成type:uuid列表（按,/:拆分），且resourceBindings::{resourceUUids}的 tagFormat 里只有这一段 token 值。updateTagByToken()内部是vo.getTag().replace(oldTag, newTag)，oldTag是“整个 resourceUUids”字符串，因此newTag="Cluster:<current>"会把Host:<...>等同 token 内的其他类型一并清空。当前ResourceBindingHostCollector.getType()返回"Host"，与"Cluster"并存时风险成立。- 同一逻辑块内
if (!resourceConfig)下再嵌套多层 if（<=2 层）不满足现有代码结构要求，可考虑提取方法（minor）。
🐛 建议修复（保留非集群绑定）
if (!uuids.contains(spec.getVmInstance().getClusterUuid())) {
- String tag = String.format("Cluster:%s", spec.getVmInstance().getClusterUuid());
+ List<String> entries = new ArrayList<>();
+ resources.forEach((type, ids) -> {
+ if (!type.equals(clusterCollector.getType())) {
+ ids.forEach(id -> entries.add(type + ":" + id));
+ }
+ });
+ entries.add(String.format("Cluster:%s", spec.getVmInstance().getClusterUuid()));
+ String tag = String.join(SPLIT, entries);
VmSystemTags.VM_RESOURCE_BINGDING.updateTagByToken(spec.getVmInstance().getUuid(),
VmSystemTags.VM_RESOURCE_BINGDING_TOKEN, tag);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@compute/src/main/java/org/zstack/compute/allocator/ResourceBindingAllocatorFlow.java`
around lines 96 - 101, The update currently calls
VmSystemTags.VM_RESOURCE_BINGDING.updateTagByToken(...) with
newTag="Cluster:..." which replaces the entire resourceUUids token and wipes
other bindings (Host:...). Change the flow to read the existing token via
getBindedResourcesFromTag(spec.getVmInstance().getUuid(),
VM_RESOURCE_BINGDING_TOKEN), parse it into a map of type->uuid, replace or add
the Cluster entry with spec.getVmInstance().getClusterUuid(), reserialize the
map back to the resourceUUids string, and then call
VM_RESOURCE_BINGDING.updateTagByToken(...) with the original oldToken and the
new token string; keep ResourceBindingHostCollector.getType() handling and only
remove resources from the local resources map as before. Also extract the nested
if logic into a helper method to satisfy code structure requirements.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport ZSTAC-85578 to 4.8.38
Cherry-pick of original fix from 原单 ZSTAC-66476 (source: 5.3.20).
Original commit: 06af535
Local cherry-pick HEAD: f42cb04
Changes
Related: ZSTAC-85578
sync from gitlab !10060