Skip to content

Conversation

@JSpon
Copy link
Contributor

@JSpon JSpon commented Dec 18, 2024

Allows for multiple network ids on instance creation

@CodeBleu CodeBleu added the enhancement New feature or request label Dec 18, 2024
@CodeBleu CodeBleu added this to the v0.6.0 milestone Dec 18, 2024
@CodeBleu
Copy link
Collaborator

@JSpon Thanks for your contribution. Can you please add tests for this? It's also good to follow this once you've added your tests as this can help catch things before the github workflow runs it.

@vishesh92 vishesh92 requested a review from Copilot September 2, 2025 08:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces support for multiple network interfaces during CloudStack instance creation by adding a new network_ids field that accepts a list of network IDs as an alternative to the existing single network_id field.

  • Adds network_ids field as a list type to support multiple network interfaces
  • Implements mutual exclusivity between network_id and network_ids fields using ConflictsWith
  • Updates instance creation logic to handle the new list-based network configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if networks, ok := d.GetOk("network_ids"); ok {
var networkIds []string
for _, nw := range networks.([]interface{}) {
networkIds = append(networkIds, fmt.Sprintf("%v", nw))
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Using fmt.Sprintf with %v is unnecessary and potentially unsafe for type conversion. Since the schema defines the element type as TypeString, cast directly to string: networkIds = append(networkIds, nw.(string))

Suggested change
networkIds = append(networkIds, fmt.Sprintf("%v", nw))
networkIds = append(networkIds, nw.(string))

Copilot uses AI. Check for mistakes.
for _, nw := range networks.([]interface{}) {
networkIds = append(networkIds, fmt.Sprintf("%v", nw))
}
p.SetNetworkids(networkIds);
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Remove the semicolon at the end of the line. Go does not require semicolons at the end of statements.

Suggested change
p.SetNetworkids(networkIds);
p.SetNetworkids(networkIds)

Copilot uses AI. Check for mistakes.
@vishesh92
Copy link
Member

@JSpon Please check Copilot's comments and also update the documentation.

@kiranchavala
Copy link
Collaborator

@JSpon could you please let us know if you want the PR in the upcoming release?

Currently, we need the PR to pass the tests and the conflicts must be resolved inorder to merge it

@kiranchavala kiranchavala modified the milestones: v0.6.0, v0.7.0 Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants