Skip to content

Conversation

@Ntr0
Copy link

@Ntr0 Ntr0 commented Oct 19, 2025

What does this fix or implement?

Adds --ionoscloud-additional-volumes parameter to allow users to provision VMs with additional volumes. Using cloud-init, these can be partitioned, formatted and mounted.

Checklist

  • PR name added as appropriate (e.g. feat:/fix:/doc:/test:/refactor:)
  • Tests added or updated
  • Documentation updated
  • Sonar Cloud Scan
  • Changelog updated and version incremented (label: upcoming release)
  • Github Issue linked if any
  • Jira task updated

@Ntr0 Ntr0 requested a review from Copilot October 19, 2025 15:29
Copy link

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 adds support for provisioning VMs with additional volumes in the IonosCloud driver by introducing a new --ionoscloud-additional-disks parameter. Users can specify volume type (HDD/SSD) and size, which can then be configured via cloud-init.

Key Changes:

  • Added flag and configuration parsing for additional disk specifications
  • Modified server creation logic to provision multiple volumes with appropriate boot order settings
  • Implemented cleanup logic to remove additional volumes during VM deletion

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
ionoscloud.go Adds flag definition, disk properties struct, parsing/validation logic, and cleanup handling for additional volumes
ionos_create_machine.go Updates server creation to configure primary and additional volumes with correct boot orders and retrieves volume IDs

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

ionoscloud.go Outdated
mcnflag.StringSliceFlag{
Name: flagAdditionalDisks,
EnvVar: extflag.KebabCaseToEnvVarCase(flagAdditionalDisks),
Usage: "Additional Disks to attach to the VM, must provide volume name, type (HDD,SSD) and size (in GB). Example: \"HDD,10\"",
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

The usage description mentions 'must provide volume name, type (HDD,SSD) and size', but the actual format only requires 'type,size' (two fields, not three). The description should be corrected to match the implementation: 'Additional Disks to attach to the VM, must provide type (HDD,SSD) and size (in GB). Example: "HDD,10"'.

Suggested change
Usage: "Additional Disks to attach to the VM, must provide volume name, type (HDD,SSD) and size (in GB). Example: \"HDD,10\"",
Usage: "Additional Disks to attach to the VM, must provide type (HDD,SSD) and size (in GB). Example: \"HDD,10\"",

Copilot uses AI. Check for mistakes.
ionoscloud.go Outdated
return fmt.Errorf("invalid additional disk configuration: %s, must be \"type,size\"", disk)
}
if props[0] != "HDD" && props[0] != "SSD" {
return fmt.Errorf("invalid additional disk type: %s, must be HDD or SSD", disk)
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

The error message references the entire 'disk' string rather than the specific invalid type. It should reference 'props[0]' to show exactly which part is invalid: 'invalid additional disk type: %s, must be HDD or SSD', props[0].

Suggested change
return fmt.Errorf("invalid additional disk type: %s, must be HDD or SSD", disk)
return fmt.Errorf("invalid additional disk type: %s, must be HDD or SSD", props[0])

Copilot uses AI. Check for mistakes.
@Ntr0 Ntr0 requested a review from Copilot October 20, 2025 14:47
Copy link

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


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

ionoscloud.go Outdated
mcnflag.StringSliceFlag{
Name: flagAdditionalDisks,
EnvVar: extflag.KebabCaseToEnvVarCase(flagAdditionalDisks),
Usage: "Additional Disks to attach to the VM, must provide volume name, type (HDD,SSD) and size (in GB). Example: \"HDD,10\"",
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The usage documentation incorrectly states 'must provide volume name, type (HDD,SSD) and size' but the actual format only requires 'type,size' (two fields). Remove the reference to 'volume name' from the documentation.

Suggested change
Usage: "Additional Disks to attach to the VM, must provide volume name, type (HDD,SSD) and size (in GB). Example: \"HDD,10\"",
Usage: "Additional Disks to attach to the VM, must provide type (HDD,SSD) and size (in GB). Example: \"HDD,10\"",

Copilot uses AI. Check for mistakes.
@Ntr0
Copy link
Author

Ntr0 commented Oct 21, 2025

Example cloud init config with which I made the volume available

disk_setup:
  /dev/vdb:
    table_type: gpt
    layout: true
    overwrite: false

fs_setup:
  - label: longhorn
    filesystem: ext4
    device: /dev/vdb
    overwrite: false
    extra_opts:
      - -E
      - lazy_itable_init=1,lazy_journal_init=1

mounts:
  - ["LABEL=longhorn", "/var/lib/longhorn", "ext4", "defaults,nofail,discard", "0","2"]

Please note: Due to the implementation of the driver - the composition call to create the server along with its volumes - it might be difficult to identify volumes reliably using cloud-init, as the volume-UUIDs and therefore their externalIDs are not computable before creating the volumes.

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.

3 participants