Skip to content

Conversation

@Tinyblargon
Copy link
Collaborator

Add ways to check if there are pending config changes.
Add a way to get the active config, as the normal config has the pending changes included.
Improved some tests due to the new mocks.

Closes #458

@Tinyblargon Tinyblargon requested a review from Copilot September 15, 2025 20:58
@Tinyblargon Tinyblargon self-assigned this Sep 15, 2025
@Tinyblargon Tinyblargon added type/feature Completely new functionality. modifies/go Pull requests that update Go code size/XL Denotes a PR that changes 500-999 lines, ignoring generated files test/needen This PR has to be tested labels Sep 15, 2025
Copy link
Contributor

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 functionality to check for pending configuration changes and retrieve the active configuration without pending changes. The PR introduces new methods to differentiate between the complete configuration (with pending changes) and the active configuration (without pending changes).

  • Adds methods to check if there are pending configuration changes for VMs and LXC containers
  • Provides new functions to get active configuration without pending changes
  • Improves existing code to use the new pending changes detection

Reviewed Changes

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

Show a summary per file
File Description
proxmox/vmref.go Adds core methods for checking pending changes and getting pending config
proxmox/vmref_test.go Tests for the new pending changes functionality
proxmox/config__qemu.go Updates VM config handling to use new pending changes methods
proxmox/config__qemu_test.go Tests for active QEMU configuration without pending changes
proxmox/config__lxc__new.go Updates LXC config handling to use new pending changes methods
proxmox/config__lxc__new_test.go Tests for active LXC configuration without pending changes
proxmox/config__guest.go Updates guest config to use new pending changes methods
proxmox/client__new.go Adds guest check functionality to new client interface
proxmox/client__new__mock.go Mock implementations for new client methods
proxmox/client__api.go Adds API method to get guest pending changes
proxmox/client__api__mock.go Mock implementation for API pending changes method
proxmox/client__api__reusable.go Updates API helper methods with better error handling

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

qemuPrefixApiKeyUSB string = "usb"
)

// NewRawConfigQemuFromApi returns the configuration of the LXC container.
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

The comment incorrectly refers to 'LXC container' but this function is for QEMU VMs. It should say 'QEMU VM' instead.

Suggested change
// NewRawConfigQemuFromApi returns the configuration of the LXC container.
// NewRawConfigQemuFromApi returns the configuration of the QEMU VM.

Copilot uses AI. Check for mistakes.
Comment on lines 752 to 753
// NewRawConfigLXCFromAPI returns the configuration of the LXC container.
// Including pending changes.
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

[nitpick] The comment is correct for LXC, but there's an inconsistency with the QEMU version which incorrectly mentions LXC. Consider standardizing the comment format.

Suggested change
// NewRawConfigLXCFromAPI returns the configuration of the LXC container.
// Including pending changes.
// NewRawConfigLXCFromAPI returns the configuration of the LXC container, including pending changes.

Copilot uses AI. Check for mistakes.
Comment on lines 776 to 777
// NewActiveRawConfigLXCFromApi returns the active configuration of the LXC container.
// Without pending changes.
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

[nitpick] The comment is correct for LXC, but there's an inconsistency with the QEMU version which incorrectly mentions LXC. Consider standardizing the comment format.

Suggested change
// NewActiveRawConfigLXCFromApi returns the active configuration of the LXC container.
// Without pending changes.
// NewActiveRawConfigLXCFromApi returns the active configuration of the LXC container, excluding pending changes.

Copilot uses AI. Check for mistakes.
Comment on lines 1246 to 1247
// NewActiveRawConfigQemuFromApi returns the active configuration of the LXC container.
// Without pending changes.
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

The comment incorrectly refers to 'LXC container' but this function is for QEMU VMs. It should say 'QEMU VM' instead.

Copilot uses AI. Check for mistakes.
@Tinyblargon Tinyblargon added test/done This PR has been tested and the result was succesfull and removed test/needen This PR has to be tested labels Sep 16, 2025
@Tinyblargon Tinyblargon merged commit 94a5f15 into Telmate:master Sep 16, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifies/go Pull requests that update Go code size/XL Denotes a PR that changes 500-999 lines, ignoring generated files test/done This PR has been tested and the result was succesfull type/feature Completely new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read: Pending config preferred

1 participant