Sankofa/docs/reference/CODE_INCONSISTENCIES.md

Code Inconsistencies Found

Critical Inconsistencies

1. Missing Variable Assignment in Controller

File: crossplane-provider-proxmox/pkg/controller/virtualmachine/controller.go:64

Issue: Code search shows line 64 has proxmox.NewClient( but the actual file shows it's correctly assigned. However, there's a potential issue with error handling.

Current Code:

// Line 63-72
// Create Proxmox client
proxmoxClient, err := proxmox.NewClient(
    site.Endpoint,
    creds.Username,
    creds.Password,
    site.InsecureSkipTLSVerify,
)
if err != nil {
    return ctrl.Result{}, errors.Wrap(err, "cannot create Proxmox client")
}

Status: ✅ Actually correct - variable is assigned. No fix needed.

2. Inconsistent Client Creation in VMScaleSet Controller

File: crossplane-provider-proxmox/pkg/controller/vmscaleset/controller.go:47

Issue: Creates client with empty parameters, likely a bug.

Current Code:

proxmoxClient := proxmox.NewClient("", "", "")

Problem: This will always fail or use invalid credentials.

Fix Required: Should use proper credentials from ProviderConfig, similar to virtualmachine controller.

3. Inconsistent Error Handling Patterns

Location: Multiple files

Pattern 1 (virtualmachine/controller.go):

• Credentials error → requeue after 30s
• Site error → requeue after 30s
• VM creation error → no requeue (immediate return)

Pattern 2 (Other controllers):

• Various requeue strategies

Recommendation: Standardize error handling with:

• Retryable errors → requeue with exponential backoff
• Non-retryable errors → no requeue, update status with error condition
• Partial creation errors → cleanup + requeue with longer delay

4. importdisk API Usage Without Version Check

File: crossplane-provider-proxmox/pkg/proxmox/client.go:397

Issue: Uses importdisk API without checking if it's available.

Current Code:

importPath := fmt.Sprintf("/nodes/%s/qemu/%d/importdisk", spec.Node, vmID)
if err := c.httpClient.Post(ctx, importPath, importConfig, &importResult); err != nil {
    return nil, errors.Wrapf(err, "failed to import image...")
}

Problem: No check if Proxmox version supports this API.

Fix Required: Add version check or API availability check before use.

5. Inconsistent Status Field Names

File: crossplane-provider-proxmox/apis/v1alpha1/virtualmachine_types.go:56

Issue: Status field is VMID but JSON tag is vmId (camelCase).

Current Code:

VMID int `json:"vmId,omitempty"`

Problem: Inconsistent naming (Go uses VMID, JSON uses vmId).

Impact: Minor - works but inconsistent.

6. No Cleanup on Partial VM Creation

File: crossplane-provider-proxmox/pkg/controller/virtualmachine/controller.go:142-145

Issue: When CreateVM fails, no cleanup of partially created VM.

Current Code:

createdVM, err := proxmoxClient.CreateVM(ctx, vmSpec)
if err != nil {
    return ctrl.Result{}, errors.Wrap(err, "cannot create VM")
}

Problem: If VM is created but import fails, VM remains orphaned.

Fix Required: Add cleanup logic in error path.

7. Lock File Handling Not Used in Error Recovery

File: crossplane-provider-proxmox/pkg/proxmox/client.go:803-821

Issue: UnlockVM function exists but not called during error recovery.

Current Code: UnlockVM is defined but never used in CreateVM error paths.

Fix Required: Call UnlockVM before DeleteVM in cleanup operations.

---

Summary of Required Fixes

1. ✅ virtualmachine/controller.go:64 - Actually correct, no fix needed
2. ❌ vmscaleset/controller.go:47 - Fix client creation
3. ⚠️ Error handling - Standardize across all controllers
4. ❌ importdisk API - Add version/availability check
5. ⚠️ Status field naming - Consider consistency (low priority)
6. ❌ Partial VM cleanup - Add cleanup in error paths
7. ❌ Lock file handling - Use UnlockVM in cleanup

---

Last Updated: 2025-12-12