<feature>[ansible]: add SshFilesMd5Checker#15
Conversation
|
Warning Rate Limit Exceeded@MatheMatrix has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 18 minutes and 8 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Walkthrough在这次代码更新中，我们看到了几个Java类的变化。新增了 Changes
TipsChat with CodeRabbit Bot (
|
24189a5 to
ad1b4eb
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (3)
- core/src/main/java/org/zstack/core/ansible/SshFilesMd5Checker.java (1 hunks)
- utils/src/main/java/org/zstack/utils/SHAUtils.java (2 hunks)
- utils/src/main/java/org/zstack/utils/path/PathUtil.java (1 hunks)
Additional comments: 3
core/src/main/java/org/zstack/core/ansible/SshFilesMd5Checker.java (1)
- 11-112: 整体上，
SshFilesMd5Checker类的实现符合PR目标和生成的摘要。它实现了AnsibleChecker接口，并提供了检查和删除文件的方法，以及相关属性的getter和setter方法。代码风格和结构看起来都是清晰和一致的。utils/src/main/java/org/zstack/utils/SHAUtils.java (2)
23-24: 确认
encrypt方法没有发生变化，与PR目标和摘要中的描述相符。3-9: 新添加的导入语句
java.io.IOException,java.nio.file.Files, 和java.nio.file.Paths被getDigestOfFile方法使用，这是合理的。但是，如果其他部分的代码没有使用这些导入，应该检查是否有未使用的导入。
| public static Long getFileInode(String filePath) { | ||
| Path path = Paths.get(filePath); | ||
| try { | ||
| return (Long) Files.getAttribute(path, "unix:ino"); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } |
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.
The method getFileInode throws a generic RuntimeException without any logging or specific error message. Consider adding logging and throwing a more specific exception to provide better context for the error.
- throw new RuntimeException(e);
+ logger.error("Failed to get inode for file: " + filePath, e);
+ throw new RuntimeException("Failed to get inode for file: " + filePath, e);Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| public static Long getFileInode(String filePath) { | |
| Path path = Paths.get(filePath); | |
| try { | |
| return (Long) Files.getAttribute(path, "unix:ino"); | |
| } catch (Exception e) { | |
| throw new RuntimeException(e); | |
| } | |
| public static Long getFileInode(String filePath) { | |
| Path path = Paths.get(filePath); | |
| try { | |
| return (Long) Files.getAttribute(path, "unix:ino"); | |
| } catch (Exception e) { | |
| logger.error("Failed to get inode for file: " + filePath, e); | |
| throw new RuntimeException("Failed to get inode for file: " + filePath, e); | |
| } |
| @Override | ||
| public boolean needDeploy() { | ||
| Ssh ssh = new Ssh(); | ||
| ssh.setUsername(username).setPrivateKey(privateKey) | ||
| .setPassword(password).setPort(sshPort) | ||
| .setHostname(ip) | ||
| .setTimeout(5); | ||
| try { | ||
| ssh.command(String.format("echo %s | sudo -S md5sum %s 2>/dev/null", password, filePath)); | ||
| SshResult ret = ssh.run(); | ||
| if (ret.getReturnCode() != 0) { | ||
| return true; | ||
| } | ||
| ssh.reset(); | ||
| String md5 = ret.getStdout().split(" ")[0]; | ||
| if (!fileMd5sums.contains(md5)) { | ||
| logger.debug(String.format("file MD5 changed, dest[%s, md5, %s]", filePath, md5)); | ||
| return true; | ||
| } | ||
| } finally { | ||
| ssh.close(); | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
在 needDeploy 方法中，如果SSH命令执行失败（返回码非0），方法会直接返回 true，这意味着需要部署。但是，如果SSH命令的执行失败是由于网络问题或其他非文件MD5不匹配的原因，这可能会导致不必要的部署。建议添加更详细的错误处理来区分不同类型的错误，并且只在确认文件MD5不匹配时才返回 true。
| public void deleteDestFile() { | ||
| Ssh ssh = new Ssh(); | ||
| ssh.setUsername(username).setPrivateKey(privateKey) | ||
| .setPassword(password).setPort(sshPort) | ||
| .setHostname(ip).command(String.format("rm -f %s", filePath)).runAndClose(); | ||
| logger.debug(String.format("delete dest file[%s]", filePath)); | ||
| } |
There was a problem hiding this comment.
在 deleteDestFile 方法中，使用了 Ssh 类来删除文件，并且没有显式的错误处理。如果删除命令失败，这可能会导致问题。建议添加错误处理逻辑来捕获可能的异常，并记录相应的错误信息。
| public static String getDigestOfFile(String filePath, String algorithm, int precision) { | ||
| try { | ||
| byte[] fileData = Files.readAllBytes(Paths.get(filePath)); | ||
| MessageDigest md = MessageDigest.getInstance(algorithm); | ||
| md.reset(); | ||
| md.update(fileData); | ||
| BigInteger bigInteger = new BigInteger(1, md.digest()); | ||
| return String.format("%" + precision + "x", bigInteger); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
检查新添加的getDigestOfFile方法，确认其逻辑正确无误。注意，异常处理使用了一个通用的Exception捕获，这可能会隐藏潜在的错误类型。建议捕获更具体的异常类型，比如IOException和NoSuchAlgorithmException。
- } catch (Exception e) {
+ } catch (IOException | NoSuchAlgorithmException e) {Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| public static String getDigestOfFile(String filePath, String algorithm, int precision) { | |
| try { | |
| byte[] fileData = Files.readAllBytes(Paths.get(filePath)); | |
| MessageDigest md = MessageDigest.getInstance(algorithm); | |
| md.reset(); | |
| md.update(fileData); | |
| BigInteger bigInteger = new BigInteger(1, md.digest()); | |
| return String.format("%" + precision + "x", bigInteger); | |
| } catch (Exception e) { | |
| throw new RuntimeException(e); | |
| } | |
| } | |
| public static String getDigestOfFile(String filePath, String algorithm, int precision) { | |
| try { | |
| byte[] fileData = Files.readAllBytes(Paths.get(filePath)); | |
| MessageDigest md = MessageDigest.getInstance(algorithm); | |
| md.reset(); | |
| md.update(fileData); | |
| BigInteger bigInteger = new BigInteger(1, md.digest()); | |
| return String.format("%" + precision + "x", bigInteger); | |
| } catch (IOException | NoSuchAlgorithmException e) { | |
| throw new RuntimeException(e); | |
| } | |
| } |
aa0aa1c to
caa2dd1
Compare
1 Cache inodes and hashes of all architectur bin packages in mn memory 2 If the bin package hash value of the target physical machine is not in the in-memory hash list, redeploy the bin package. Resolves: ZSTAC-59604 Change-Id: I75676474797078746d757a7564716d736f6b7267
caa2dd1 to
8990551
Compare
1 Cache inodes and hashes of all architectur bin packages in mn memory
2 If the bin package hash value of the target physical machine is
not in the in-memory hash list, redeploy the bin package.
Resolves: ZSTAC-59604
Change-Id: I75676474797078746d757a7564716d736f6b7267
sync from gitlab !5410
Summary by CodeRabbit