Skip to content

Implement LHv2 volume expansion#2673

Merged
khushboo-rancher merged 1 commit into
harvester:mainfrom
albinsun:lhv2_volume_expansion
Jun 17, 2026
Merged

Implement LHv2 volume expansion#2673
khushboo-rancher merged 1 commit into
harvester:mainfrom
albinsun:lhv2_volume_expansion

Conversation

@albinsun

@albinsun albinsun commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Which issue(s) this PR fixes:

What this PR does / why we need it:

Feature

  1. Implement LHv2 volume expansion

Enhancement

  1. Use NVMe disk instead of just large disks
  2. Introduce run strategy coupling on blockdevice and setting components
  3. Add new component storageclass
  4. Add Common Suite/Test Teardown
    • Cleanup resources only when all tests Passed -> Keep env. for investigation (Resources will be cleanup in following run if tests are Pass)
    • Log Variables when tests Failed -> For Debugging
  5. Adjust arguments of VM is created

Fix

  1. Fix common_keywords.cleanup_volumes in Cleanup test resources

Special notes for your reviewer:

Additional documentation or context

Verification

  • log.html
    image
  • report.html
    image

Why Use NVMe disk instead of just large disks, NVMe vs. SAS

  • NVMe

    2026-05-29.12.23.25.mov
  • SAS

    2026-05-29.09.38.17.mov

@albinsun albinsun added this to the v1.8.1 milestone Jun 2, 2026
Copilot AI review requested due to automatic review settings June 2, 2026 07:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds end-to-end Robot Framework coverage for Longhorn v2 volume expansion (issue #2178), introduces a reusable storageclass component (CRD/REST layered like the other components), and reworks NVMe disk selection plus shared suite/test teardowns. As part of the change, the positional argument order of VM is created / vm.create(...) was swapped (image_id moved before cpu/memory) and the existing test_vm.robot caller was updated accordingly. The setting and blockdevice components are refactored from an env-var-driven strategy selector to a CRD-first / REST-fallback pattern via NotImplementedError.

Changes:

  • New test_longhorn_v2.robot flow covering provisioning, SC creation, VM-with-LHv2-volume, volume expansion (PVC-direct and VM-annotation-based), with NVMe disk picker and tag-based disk selector.
  • New libs/storageclass/ component + vm.update_disk_size / volume keyword resource for expansion, plus Longhorn-node disk tag helpers in host.
  • API-shape change: vm.create(...) signature reordered to (vm_name, image_id, cpu, memory, **kwargs); volume get_status renames statephase and adds conditions.

Reviewed changes

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

Show a summary per file
File Description
harvester_robot_tests/tests/regression/test_longhorn_v2.robot New LHv2 provisioning/expansion test flow and suite setup/teardown.
harvester_robot_tests/tests/regression/test_vm.robot Updates caller to new VM is created positional arg order.
harvester_robot_tests/libs/vm/{base,vm,rest,crd}.py Reorders create(...) args; adds update_disk_size + extra-disk support in CRD VM spec.
harvester_robot_tests/libs/volume/crd.py Returns phase/conditions from get_status (renames state).
harvester_robot_tests/libs/storageclass/{init,base,crd,rest,storageclass}.py New StorageClass component with CRD-first/REST-fallback pattern.
harvester_robot_tests/libs/setting/{base,crd,rest,setting}.py Refactors to CRD-first/REST-fallback via NotImplementedError.
harvester_robot_tests/libs/blockdevice/{base,crd,rest,blockdevice}.py Same CRD-first/REST-fallback refactor.
harvester_robot_tests/libs/host/{base,crd,host,rest}.py Adds add/remove_lh_node_disk_tag and switches to CRD-first/REST-fallback base.
harvester_robot_tests/libs/keywords/{vm,volume,storage,host,common}_keywords.py Exposes new component operations and get_timestamp.
harvester_robot_tests/libs/utility/utility.py Adds get_timestamp helper.
harvester_robot_tests/keywords/{volume,virtualmachine,storage,image,host,common,variables}.resource New volume keywords, refactored storage/SC keywords, NVMe disk picker, Common Suite/Test Teardown, env-overridable WAIT_TIMEOUT.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread harvester_robot_tests/libs/vm/crd.py Outdated
Comment thread harvester_robot_tests/libs/vm/crd.py Outdated
Comment thread harvester_robot_tests/tests/regression/test_longhorn_v2.robot Outdated
Comment thread harvester_robot_tests/keywords/common.resource
Comment thread harvester_robot_tests/libs/volume/crd.py
@albinsun albinsun force-pushed the lhv2_volume_expansion branch from ec4fdda to dcf6947 Compare June 2, 2026 10:52
@albinsun albinsun requested a review from a team June 2, 2026 10:55
@albinsun albinsun force-pushed the lhv2_volume_expansion branch from dcf6947 to 68d5327 Compare June 3, 2026 09:23
asc5543
asc5543 previously approved these changes Jun 4, 2026

@asc5543 asc5543 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@khushboo-rancher khushboo-rancher left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @albinsun for the PR, looks good. Few comments.

Comment thread harvester_robot_tests/keywords/storage.resource Outdated
Comment thread harvester_robot_tests/tests/regression/test_longhorn_v2.robot
Comment thread harvester_robot_tests/tests/regression/test_longhorn_v2.robot Outdated
Comment thread harvester_robot_tests/tests/regression/test_longhorn_v2.robot Outdated
@albinsun

Copy link
Copy Markdown
Contributor Author

Changes

  • Move storage class keywords to a independent storageclass.resource
  • Single layer Suite Setup

Test Report

image

@khushboo-rancher khushboo-rancher left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, few minor comments

Comment on lines +82 to +83
setting.LHv2 Data Engine Is Disabled
setting.Enable LHv2 Data Engine

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If the LHv2 already enabled, how is it handled? Will it fail at 'setting.LHv2 Data Engine Is Disabled'?
Should we handle this while enabling of v2 data engine, if already enabled -> pass otherwise enable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in commit 60cfed6

*** Settings ***
Documentation Longhorn V2 Data Engine Test Cases
Test Tags longhorn LHv2 LonghornV2 storage experimental
Test Tags longhorn lhv2 experimental

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IMO, let's keep the 'storage' tag. storage is the group where we can keep all the storage related test cases like v2, lvm etc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in commit 60cfed6


*** Test Cases ***
Provision LHv2 Storages
[Tags] p1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This, Create LHv2 Storage Class & Create VM With LHv2 Volume should be P0. Other tests like Expand LHv2 Volume Size should be P1.

We can follow below guideline:
P0: The core "happy path" or main workflow of the feature.
Impact: If this fails, the feature is completely unusable, and there is no workaround.

P1: Secondary paths, edge cases, or essential supporting functionality.
Impact: The core feature works, but key high-value additions or sub-features are broken.

P2: "Nice-to-have" features, or rarely used workflows.
Impact: Low user impact; the core value of the feature remains fully intact.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in commit 60cfed6

Signed-off-by: Albin Sun <albin.sun@suse.com>
@albinsun albinsun force-pushed the lhv2_volume_expansion branch from d90c2df to 60cfed6 Compare June 16, 2026 05:02
@albinsun

Copy link
Copy Markdown
Contributor Author

Changes

  1. Skip enabling Lhv2 data-engine if it's already enabled
  2. Keep storage tag
  3. Adjust test case priority

Report

image

@khushboo-rancher khushboo-rancher merged commit 67254dd into harvester:main Jun 17, 2026
5 checks passed
@albinsun albinsun deleted the lhv2_volume_expansion branch June 17, 2026 01:13
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.

4 participants