Skip to content

[Issue 323] add support for disk replication flag - #330

Merged
tanmay-hc merged 4 commits into
hashicorp:mainfrom
domrim:disk-replication-flag
Sep 7, 2026
Merged

[Issue 323] add support for disk replication flag#330
tanmay-hc merged 4 commits into
hashicorp:mainfrom
domrim:disk-replication-flag

Conversation

@domrim

@domrim domrim commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

Description

Added support for configuring the "replication" parameter of disks.

Resolved Issues

Closes #329
Closes #323

Rollback Plan

If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

NA

@domrim
domrim requested a review from a team as a code owner July 18, 2025 10:16
@hashicorp-cla-app

hashicorp-cla-app Bot commented Jul 18, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@domrim domrim changed the title [Issue 329] add support for disk replication flag [Issue 323] add support for disk replication flag Jul 18, 2025
@PendaGTP

PendaGTP commented Feb 1, 2026

Copy link
Copy Markdown

Ping

Any timeline for review/merge? I'm also interested in this feature

@PendaGTP

Copy link
Copy Markdown

@nywilken @lbajolet-hashicorp

Sorry for pinging directly, idk which people are maintaining this repo.

@ksl28

ksl28 commented Jul 1, 2026

Copy link
Copy Markdown

I am now facing the same issue, and was surprised to see, that the only thing missing - is the code review.
@tanmay-hc - sorry for tagging, but could you be of assistance here?

@tanmay-hc tanmay-hc 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.

  1. The go test TestGenerateProxmoxDisks fails because the existing expected fixtures were not updated for the new Replicate field in the "bunch of disks" cases.
  2. Please add/adjust coverage for the default path as well as skip_replication=true. Right now the change proves the opt-out case, but it should also assert that existing builds keep the same effective behavior when skip_replication is unset.
  3. Behavior looks additive, but this touches disk creation for every bus type. Please update the existing fixture expectations in the two multi-disk cases so the suite passes before merge.

@domrim
domrim force-pushed the disk-replication-flag branch from 726304d to 70f96a2 Compare August 9, 2026 15:41
@domrim
domrim force-pushed the disk-replication-flag branch from 7916bd0 to 26ad02f Compare August 9, 2026 16:05
@domrim

domrim commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author
  1. The go test TestGenerateProxmoxDisks fails because the existing expected fixtures were not updated for the new Replicate field in the "bunch of disks" cases.

    1. Please add/adjust coverage for the default path as well as skip_replication=true. Right now the change proves the opt-out case, but it should also assert that existing builds keep the same effective behavior when skip_replication is unset.

    2. Behavior looks additive, but this touches disk creation for every bus type. Please update the existing fixture expectations in the two multi-disk cases so the suite passes before merge.

thx for the review. i adjusted the tests according to your requests. please have a look if this is now sufficient, if anything is still missing i can implement them.

@domrim
domrim requested a review from tanmay-hc August 9, 2026 16:09
@tanmay-hc

Copy link
Copy Markdown
Contributor

@domrim The changes look good to me now. Can you just run a make generate?

@domrim

domrim commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@domrim The changes look good to me now. Can you just run a make generate?

documentation should be up2date now

@tanmay-hc
tanmay-hc merged commit a65d883 into hashicorp:main Sep 7, 2026
12 checks passed
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.

Allow replication flag of disks to be configured Add feature to set disk replication

4 participants