Skip to content

Conversation

@tenthirtyam
Copy link
Collaborator

@tenthirtyam tenthirtyam commented Dec 11, 2025

Description

Refactored tools_sync_time from bool to *bool in ConfigParamsConfig to distinguish explicit true/false from omission, and adjust logic to handle nil values. The change makes tools_sync_time optional and clarifies its behavior when omitted, ensuring more flexible and accurate time synchronization settings for virtual machines.

Added tools_sync_time_periodically configuration option for periodic time synchronization between the guest operating system and the ESX host.

Configuration and Validation Updates:

  • Added the tools_sync_time_periodically option to both vsphere-iso and vsphere-clone builders, allowing users to enable or disable periodic time synchronization. This is only recommended if the guest OS does not have native time sync. [1] [2] [3]
  • Updated the tools_sync_time option to accept a pointer (*bool), clarifying its meaning and inheritance: if omitted, the builder uses defaults or inherits from the source VM.
  • Implemented validation logic so that tools_sync_time_periodically can only be enabled if tools_sync_time is also set to true.

Documentation Improvements:

  • Revised documentation in Markdown and MDX files to explain the new and updated options, their defaults, and inheritance behavior for both builders. [1] [2] [3]

Internal Code Changes:

  • Updated configuration structs and HCL2 specs to support the new option and pointer types. [1] [2] [3]
  • Modified the step logic to correctly set vSphere API fields for both time synchronization options.
  • Ensured the new validation is called during config preparation for both builders. [1] [2]

Resolved Issues

Closes #547
Closes #499

Rollback Plan

Revert commit.

Changes to Security Controls

None.

@tenthirtyam tenthirtyam added this to the v2.1.0 milestone Dec 11, 2025
@tenthirtyam tenthirtyam self-assigned this Dec 11, 2025
@tenthirtyam tenthirtyam added builder/vsphere-iso Builder: vsphere-iso builder/vsphere-clone Builder: vsphere-clone refactor Refactor labels Dec 11, 2025
@tenthirtyam tenthirtyam force-pushed the refactor/time-sync-setting branch 3 times, most recently from 9f63f00 to bb6eeb9 Compare December 11, 2025 18:20
@tenthirtyam tenthirtyam changed the title refactor: tri-state time synchronization setting refactor: time synchronization settings Dec 11, 2025
@tenthirtyam tenthirtyam marked this pull request as ready for review December 11, 2025 18:24
@tenthirtyam tenthirtyam requested a review from a team as a code owner December 11, 2025 18:24
Copilot AI review requested due to automatic review settings December 11, 2025 18:24
@tenthirtyam tenthirtyam requested a review from kp2099 December 11, 2025 18:25
- Refactored `tools_sync_time` from `bool` to `*bool` in `ConfigParamsConfig` to distinguish explicit `true/false` from omission, and adjust logic to handle `nil` values.
- Added `tools_sync_time_periodically` configuration option for periodic time synchronization between the guest operating system and the ESX host.

Signed-off-by: Ryan Johnson <[email protected]>
@tenthirtyam tenthirtyam force-pushed the refactor/time-sync-setting branch from bb6eeb9 to 14d4e05 Compare December 11, 2025 18:25
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.

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


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

@kp2099 kp2099 merged commit 28708b5 into main Dec 15, 2025
14 checks passed
@kp2099 kp2099 deleted the refactor/time-sync-setting branch December 15, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

builder/vsphere-clone Builder: vsphere-clone builder/vsphere-iso Builder: vsphere-iso refactor Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tools_sync_time default or explicit false ignored Update time synchonization settings and defaults

2 participants