-
Notifications
You must be signed in to change notification settings - Fork 0
cvm的update函数添加电源状态管理 #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 Walkthrough📝 WalkthroughWalkthrough此次更改主要涉及 Changes
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
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-lintlevel=warning msg="[config_reader] The configuration option 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
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 typetencentcloud/resource_tc_instance.go (1)
1331-1348
: 考虑将实例停止和检查状态的逻辑封装为函数在第1331行至第1348行,您实现了停止实例并等待其停止的逻辑。这段代码与之前的代码段(例如在恢复实例电源状态时)具有相似的模式。建议将这个重复的逻辑提取为一个通用函数,以提高代码的复用性并减少重复代码。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
tencentcloud/resource_tc_instance.go
(2 hunks)tencentcloud/service_tencentcloud_cvm.go
(1 hunks)
5d9a23b
to
2425fb5
Compare
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.
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
: 建议改进错误处理机制当前的错误处理逻辑可以进一步优化:
- 建议使用更具体的错误类型来区分不同的错误场景
- 可以考虑使用
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
: 建议优化实例停止逻辑实例停止前的状态检查逻辑可以改进:
- 建议使用 errors.Wrap 来保留错误上下文
- 可以添加停止超时保护
- 建议增加优雅停机的处理
+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
📒 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
:
需要防止空指针解引用
在访问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.
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
tencentcloud/service_tencentcloud_cvm.go (1)
289-305
: 实现符合腾讯云API最佳实践代码实现遵循了腾讯云API文档建议的最佳实践,在修改实例类型前先停止实例。错误处理和重试机制的实现也很合理。
建议考虑以下几点优化:
- 可以添加日志记录实例停止的开始和结束状态
- 考虑将重试超时时间(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
📒 Files selected for processing (2)
tencentcloud/resource_tc_instance.go
(2 hunks)tencentcloud/service_tencentcloud_cvm.go
(1 hunks)
添加cvm电源状态管理并在所有涉及关机的操作前预先安全关机,避免强制关机
Summary by CodeRabbit
新功能
Bug 修复
文档