Skip to content

Conversation

shinny-taojiachun
Copy link
Collaborator

@shinny-taojiachun shinny-taojiachun commented Nov 21, 2024

添加cvm电源状态管理并在所有涉及关机的操作前预先安全关机,避免强制关机

Summary by CodeRabbit

  • 新功能

    • 增强了实例更新过程中的错误处理,确保实例的电源状态正确。
    • 修改实例类型时,确保实例已停止,增加了重试机制。
    • 优化了安全组的处理,允许更新安全组属性。
  • Bug 修复

    • 优化了 API 调用的错误处理和日志记录,确保错误信息的准确返回。
  • 文档

    • 更新了方法签名和逻辑,简化了实例更新逻辑,移除了强制重建实例的要求。

Copy link

coderabbitai bot commented Nov 21, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

此次更改主要涉及 tencentcloud/resource_tc_instance.gotencentcloud/service_tencentcloud_cvm.go 文件。对 resourceTencentCloudInstance 函数进行了显著修改,增加了错误处理和实例电源状态的管理逻辑。同时,移除了 CustomizeDiff 函数,以简化更新逻辑。CvmService 方法的 ModifyInstanceType 也进行了调整,确保在修改实例类型之前实例处于停止状态,并增加了重试机制。此外,日志记录和错误处理得到了改进,以增强整体控制流和稳定性。

Changes

文件路径 更改摘要
tencentcloud/resource_tc_instance.go 修改 resourceTencentCloudInstance 函数以增强错误处理和实例电源状态管理,移除 CustomizeDiff 函数。更新逻辑简化,增加对实例收费类型、预付费周期和续费标志的检查。
tencentcloud/service_tencentcloud_cvm.go 修改 ModifyInstanceType 方法,确保实例在修改类型前处于停止状态,移除 ForceStop 参数,增加重试机制。标准化日志记录和错误处理。

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CvmService
    participant API

    User->>CvmService: ModifyInstanceType(instanceId, instanceType)
    CvmService->>CvmService: Check instance state
    CvmService->>API: StopInstance(instanceId)
    API-->>CvmService: Instance stopped
    CvmService->>API: ModifyInstanceType(instanceId, instanceType)
    API-->>CvmService: Modification success
    CvmService-->>User: Success response
Loading

🐰 在云端的兔子跳跃,
实例状态不再困扰,
错误处理如春风,
修改类型不再烦恼。
让我们一起欢庆,
云服务更流畅,
兔子快乐在此刻! 🌟

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 golangci-lint

level=warning msg="[config_reader] The configuration option linters.errcheck.ignore is deprecated, please use linters.errcheck.exclude-functions."
level=warning msg="[lintersdb] The name "vet" is deprecated. The linter has been renamed to: govet."
level=warning msg="The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused."
level=warning msg="The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused."
level=warning msg="The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused."


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (2)
tencentcloud/service_tencentcloud_cvm.go (1)

289-289: 建议统一代码注释的语言

代码注释应与代码的其他部分保持一致,建议将中文注释转换为英文。

建议修改如下:

-    // 根据文档 https://cloud.tencent.com/document/api/213/15744 中的建议,先停止实例再修改实例类型
+    // According to the recommendation in https://cloud.tencent.com/document/api/213/15744, stop the instance before modifying its type
tencentcloud/resource_tc_instance.go (1)

1331-1348: 考虑将实例停止和检查状态的逻辑封装为函数

在第1331行至第1348行,您实现了停止实例并等待其停止的逻辑。这段代码与之前的代码段(例如在恢复实例电源状态时)具有相似的模式。建议将这个重复的逻辑提取为一个通用函数,以提高代码的复用性并减少重复代码。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 8f22168 and 5d9a23b.

📒 Files selected for processing (2)
  • tencentcloud/resource_tc_instance.go (2 hunks)
  • tencentcloud/service_tencentcloud_cvm.go (1 hunks)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (3)
tencentcloud/service_tencentcloud_cvm.go (1)

294-306: 建议优化重试逻辑的错误处理

当前的重试逻辑在遇到错误时可能会继续重试内部错误,建议对不同类型的错误采取不同的处理策略。

建议添加对特定错误类型的处理:

 err = resource.Retry(2*readRetryTimeout, func() *resource.RetryError {
     instance, errRet := me.DescribeInstanceById(ctx, instanceId)
     if errRet != nil {
+        if strings.Contains(errRet.Error(), "ResourceNotFound") {
+            return resource.NonRetryableError(errRet)
+        }
         return retryError(errRet, InternalError)
     }
     // ... rest of the code
 })
tencentcloud/resource_tc_instance.go (2)

1134-1144: 建议改进错误处理机制

当前的错误处理逻辑可以进一步优化:

  1. 建议使用更具体的错误类型来区分不同的错误场景
  2. 可以考虑使用 errors.Join() 来合并多个错误
-var powerStatusRecoverErr error
+type powerStatusError struct {
+    op  string
+    err error
+}
+
+func (e *powerStatusError) Error() string {
+    return fmt.Sprintf("电源状态恢复失败 - %s: %v", e.op, e.err)
+}
+
+var powerStatusRecoverErr *powerStatusError

1331-1348: 建议优化实例停止逻辑

实例停止前的状态检查逻辑可以改进:

  1. 建议使用 errors.Wrap 来保留错误上下文
  2. 可以添加停止超时保护
  3. 建议增加优雅停机的处理
+const maxStopTimeout = 5 * time.Minute

 err = cvmService.StopInstance(ctx, instanceId, "KEEP_CHARGING")
 if err != nil {
-    return
+    return errors.Wrap(err, "停止实例失败")
 }

+stopTimer := time.NewTimer(maxStopTimeout)
+defer stopTimer.Stop()
+
 err = resource.Retry(2*readRetryTimeout, func() *resource.RetryError {
     instance, errRet := cvmService.DescribeInstanceById(ctx, instanceId)
     if errRet != nil {
         return retryError(errRet, InternalError)
     }
+    select {
+    case <-stopTimer.C:
+        return resource.NonRetryableError(fmt.Errorf("停止实例超时"))
+    default:
+    }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 5d9a23b and 2425fb5.

📒 Files selected for processing (2)
  • tencentcloud/resource_tc_instance.go (2 hunks)
  • tencentcloud/service_tencentcloud_cvm.go (1 hunks)
🔇 Additional comments (2)
tencentcloud/service_tencentcloud_cvm.go (2)

289-290: 实现了安全关机机制

根据腾讯云API文档的建议,在修改实例类型前先停止实例,这是一个很好的实践。使用KEEP_CHARGING模式确保实例在停机期间继续计费,避免了意外的资源释放。


299-302: ⚠️ Potential issue

需要防止空指针解引用

在访问instance.InstanceState之前需要进行空指针检查,以避免潜在的运行时错误。

建议按如下方式修改代码:

-        if instance != nil && *instance.InstanceState == CVM_STATUS_STOPPED {
+        if instance != nil && instance.InstanceState != nil && *instance.InstanceState == CVM_STATUS_STOPPED {
             return nil
         }
-        return resource.RetryableError(fmt.Errorf("cvm instance status is %s, retry...", *instance.InstanceState))
+        if instance != nil && instance.InstanceState != nil {
+            return resource.RetryableError(fmt.Errorf("cvm instance status is %s, retry...", *instance.InstanceState))
+        }
+        return resource.RetryableError(fmt.Errorf("instance or instance state is nil, retry..."))

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (1)
tencentcloud/service_tencentcloud_cvm.go (1)

289-305: 实现符合腾讯云API最佳实践

代码实现遵循了腾讯云API文档建议的最佳实践,在修改实例类型前先停止实例。错误处理和重试机制的实现也很合理。

建议考虑以下几点优化:

  1. 可以添加日志记录实例停止的开始和结束状态
  2. 考虑将重试超时时间(2*readRetryTimeout)作为可配置参数
 err := me.StopInstance(ctx, instanceId, "KEEP_CHARGING")
 if err != nil {
+    log.Printf("[ERROR] Failed to stop instance %s: %v", instanceId, err)
     return err
 }
+log.Printf("[INFO] Successfully stopped instance %s", instanceId)

-err = resource.Retry(2*readRetryTimeout, func() *resource.RetryError {
+err = resource.Retry(waitForStopTimeout, func() *resource.RetryError {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 2425fb5 and 38ff892.

📒 Files selected for processing (2)
  • tencentcloud/resource_tc_instance.go (2 hunks)
  • tencentcloud/service_tencentcloud_cvm.go (1 hunks)

@shinny-taojiachun
Copy link
Collaborator Author

shinny-taojiachun commented Dec 3, 2024

image
能够成功识别为update并部署成功,耗时约45s

@shinny-taojiachun shinny-taojiachun merged commit d866d4a into master Dec 3, 2024
5 checks passed
@shinny-taojiachun shinny-taojiachun deleted the add-cvm-poweroff branch December 3, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants