Skip to content

Conversation

@FrankYang0529
Copy link

Issue: harvester/harvester#5764

Solution

Add reserved memory field.

Testing

Engineering Testing

Manual Testing

Automated Testing

QA Testing Considerations

Regressions Considerations

@FrankYang0529 FrankYang0529 force-pushed the HARV-5764 branch 3 times, most recently from 80e5a97 to 13bf428 Compare June 3, 2025 10:00
@FrankYang0529 FrankYang0529 changed the title feat: add reserved memory field (wip) feat: add reserved memory field Jun 4, 2025
Copy link
Member

@m-ildefons m-ildefons left a comment

Choose a reason for hiding this comment

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

I think there is a mixup in rancher2/structure_machine_config_v2_harvester.go

obj["memory_size"] = in.MemorySize
}

if len(in.DiskSize) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean len(in.ReservedMemorySize) > 0?

Copy link
Author

Choose a reason for hiding this comment

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

You're correct. Updated it. Thanks.

Copy link
Member

@m-ildefons m-ildefons left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@matttrach matttrach left a comment

Choose a reason for hiding this comment

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

This change looks good from a Terraform perspective.
How was it tested?

@matttrach matttrach self-assigned this Jun 9, 2025
@matttrach matttrach added area/harvester release/v8 Release with v8.x correlating to Rancher's v2.12.x labels Jun 9, 2025
@matttrach matttrach added this to the July Release milestone Jun 9, 2025
@matttrach
Copy link
Collaborator

What version of Rancher or the provider is this targeting?

@matttrach
Copy link
Collaborator

Ignore the validate commit message error, it is from the branch update.

@FrankYang0529
Copy link
Author

@matttrach Thanks for the review. We may not merge it now. It's waiting for harvester/docker-machine-driver-harvester#75 and updating related driver in Rancher.

@FrankYang0529
Copy link
Author

Wait for PR rancher/rancher#51078.

@matttrach
Copy link
Collaborator

What release versions is this targeting?

@matttrach matttrach removed the release/v8 Release with v8.x correlating to Rancher's v2.12.x label Sep 10, 2025
@matttrach matttrach removed this from the July Release milestone Sep 10, 2025
@FrankYang0529
Copy link
Author

@matttrach Sorry for the late reply. This PR can work for Rancher version >= v2.12.0.

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