Updated MS maintenance APIs - #134
Conversation
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates the CloudStack ManagementService API surface and generated layout to include newly introduced maintenance endpoints and removeManagementServer, along with corresponding tests.
Changes:
- Added ManagementService APIs:
removeManagementServer,prepareForMaintenance,cancelMaintenance(and updated ordering ofcancelShutdown). - Updated generator layout to include these APIs.
- Added/updated ManagementService tests to cover the new endpoints.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/ManagementService_test.go | Adds test cases invoking the new ManagementService endpoints. |
| generate/layout.go | Registers new ManagementService API names in the generator layout. |
| generate/generate.go | Normalizes generator log message capitalization (“API”). |
| cloudstack/ManagementService.go | Adds new ManagementService params/response types and request methods for maintenance + server removal. |
Files not reviewed (1)
- cloudstack/ManagementService_mock.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
@sureshanaparti These changes are required to avoid the same panic as in #160
| return u | ||
| } | ||
| if v, found := p.p["managementserverid"]; found { | ||
| u.Set("managementserverid", v.(string)) |
There was a problem hiding this comment.
| u.Set("managementserverid", v.(string)) | |
| u.Set("managementserverid", string(v.(UUID))) |
| return u | ||
| } | ||
| if v, found := p.p["managementserverid"]; found { | ||
| u.Set("managementserverid", v.(string)) |
There was a problem hiding this comment.
| u.Set("managementserverid", v.(string)) | |
| u.Set("managementserverid", string(v.(UUID))) |
| u.Set("forced", vv) | ||
| } | ||
| if v, found := p.p["managementserverid"]; found { | ||
| u.Set("managementserverid", v.(string)) |
There was a problem hiding this comment.
| u.Set("managementserverid", v.(string)) | |
| u.Set("managementserverid", string(v.(UUID))) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- cloudstack/ManagementService_mock.go: Generated file
Suppressed comments (1)
cloudstack/ManagementService.go:779
RemoveManagementServerResponsedoes not define anostypeidfield, so this conversion block appears to be leftover/cargo-culted from another response type. It adds confusing, unrelated behavior to the custom unmarshal and should be removed unless this response actually includes and exposesostypeid.
if ostypeid, ok := m["ostypeid"].(float64); ok {
m["ostypeid"] = strconv.Itoa(int(ostypeid))
b, err = json.Marshal(m)
if err != nil {
return err
}
}
| if v, found := p.p["managementserverid"]; found { | ||
| u.Set("managementserverid", v.(string)) | ||
| } |
| if v, found := p.p["managementserverid"]; found { | ||
| u.Set("managementserverid", v.(string)) | ||
| } |
| if v, found := p.p["managementserverid"]; found { | ||
| u.Set("managementserverid", v.(string)) | ||
| } |
MS maintenance APIs (prepareForMaintenance , cancelMaintenance) were introduced in 4.21.0. removeManagementServer API as well.