From 3c765db0876f54e2c75ad1387112b821bb61fd84 Mon Sep 17 00:00:00 2001 From: ekarlso Date: Sun, 22 Sep 2024 19:37:51 +0200 Subject: [PATCH] Support toggling provisioning checks fixes #183 --- api/v1alpha1/proxmoxmachine_types.go | 9 +++ api/v1alpha1/zz_generated.deepcopy.go | 30 +++++++++ ...ture.cluster.x-k8s.io_proxmoxclusters.yaml | 9 +++ ...ster.x-k8s.io_proxmoxclustertemplates.yaml | 10 +++ ...ture.cluster.x-k8s.io_proxmoxmachines.yaml | 9 +++ ...ster.x-k8s.io_proxmoxmachinetemplates.yaml | 9 +++ internal/service/vmservice/vm.go | 41 +++++++++--- internal/service/vmservice/vm_test.go | 63 +++++++++++++++++++ pkg/proxmox/client.go | 3 +- pkg/proxmox/goproxmox/api_client.go | 8 +++ pkg/proxmox/proxmoxtest/mock_client.go | 43 +++++++++++++ 11 files changed, 225 insertions(+), 9 deletions(-) diff --git a/api/v1alpha1/proxmoxmachine_types.go b/api/v1alpha1/proxmoxmachine_types.go index 39245590..a338545e 100644 --- a/api/v1alpha1/proxmoxmachine_types.go +++ b/api/v1alpha1/proxmoxmachine_types.go @@ -50,6 +50,13 @@ const ( IPV6Format = "v6" ) +type ProxmoxMachineChecks struct { + // Skip checking CloudInit which can be very useful for specific Operating Systems like TalOS + // +optional + SkipCloudInitStatus *bool `json:"skipCloudInitStatus,omitempty"` + SkipQemuGuestAgent *bool `json:"skipQemuGuestAgent,omitempty"` +} + // ProxmoxMachineSpec defines the desired state of a ProxmoxMachine. type ProxmoxMachineSpec struct { VirtualMachineCloneSpec `json:",inline"` @@ -90,6 +97,8 @@ type ProxmoxMachineSpec struct { // Network is the network configuration for this machine's VM. // +optional Network *NetworkSpec `json:"network,omitempty"` + + Checks *ProxmoxMachineChecks `json:"checks,omitempty"` } // Storage is the physical storage on the node. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index bff863e6..fa1493f9 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -539,6 +539,31 @@ func (in *ProxmoxMachine) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ProxmoxMachineChecks) DeepCopyInto(out *ProxmoxMachineChecks) { + *out = *in + if in.SkipCloudInitStatus != nil { + in, out := &in.SkipCloudInitStatus, &out.SkipCloudInitStatus + *out = new(bool) + **out = **in + } + if in.SkipQemuGuestAgent != nil { + in, out := &in.SkipQemuGuestAgent, &out.SkipQemuGuestAgent + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxmoxMachineChecks. +func (in *ProxmoxMachineChecks) DeepCopy() *ProxmoxMachineChecks { + if in == nil { + return nil + } + out := new(ProxmoxMachineChecks) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProxmoxMachineList) DeepCopyInto(out *ProxmoxMachineList) { *out = *in @@ -595,6 +620,11 @@ func (in *ProxmoxMachineSpec) DeepCopyInto(out *ProxmoxMachineSpec) { *out = new(NetworkSpec) (*in).DeepCopyInto(*out) } + if in.Checks != nil { + in, out := &in.Checks, &out.Checks + *out = new(ProxmoxMachineChecks) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxmoxMachineSpec. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml index 9a52305b..b226e731 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml @@ -72,6 +72,15 @@ spec: description: ProxmoxMachineSpec defines the desired state of a ProxmoxMachine. properties: + checks: + properties: + skipCloudInitStatus: + description: Skip checking CloudInit which can be very + useful for specific Operating Systems like TalOS + type: boolean + skipQemuGuestAgent: + type: boolean + type: object description: description: Description for the new VM. type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml index 14b145d6..d950d601 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml @@ -90,6 +90,16 @@ spec: description: ProxmoxMachineSpec defines the desired state of a ProxmoxMachine. properties: + checks: + properties: + skipCloudInitStatus: + description: Skip checking CloudInit which can + be very useful for specific Operating Systems + like TalOS + type: boolean + skipQemuGuestAgent: + type: boolean + type: object description: description: Description for the new VM. type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml index 915d49e8..8f3a2cb8 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml @@ -65,6 +65,15 @@ spec: spec: description: ProxmoxMachineSpec defines the desired state of a ProxmoxMachine. properties: + checks: + properties: + skipCloudInitStatus: + description: Skip checking CloudInit which can be very useful + for specific Operating Systems like TalOS + type: boolean + skipQemuGuestAgent: + type: boolean + type: object description: description: Description for the new VM. type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml index fd46852f..bbf2a7d4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml @@ -72,6 +72,15 @@ spec: description: ProxmoxMachineSpec defines the desired state of a ProxmoxMachine. properties: + checks: + properties: + skipCloudInitStatus: + description: Skip checking CloudInit which can be very + useful for specific Operating Systems like TalOS + type: boolean + skipQemuGuestAgent: + type: boolean + type: object description: description: Description for the new VM. type: string diff --git a/internal/service/vmservice/vm.go b/internal/service/vmservice/vm.go index 7c72474c..a6c12066 100644 --- a/internal/service/vmservice/vm.go +++ b/internal/service/vmservice/vm.go @@ -108,22 +108,47 @@ func ReconcileVM(ctx context.Context, scope *scope.MachineScope) (infrav1alpha1. return vm, nil } +func skipQemuGuestCheck(mc *scope.MachineScope) bool { + if mc.ProxmoxMachine.Spec.Checks != nil { + return ptr.Equal(ptr.To(true), mc.ProxmoxMachine.Spec.Checks.SkipQemuGuestAgent) + + } + + return false +} + +func skipCloudInitCheck(mc *scope.MachineScope) bool { + if mc.ProxmoxMachine.Spec.Checks != nil { + return ptr.Equal(ptr.To(true), mc.ProxmoxMachine.Spec.Checks.SkipCloudInitStatus) + } + + return false +} + func checkCloudInitStatus(ctx context.Context, machineScope *scope.MachineScope) (requeue bool, err error) { if !machineScope.VirtualMachine.IsRunning() { // skip if the vm is not running. return true, nil } - if running, err := machineScope.InfraCluster.ProxmoxClient.CloudInitStatus(ctx, machineScope.VirtualMachine); err != nil || running { - if running { - return true, nil + if !skipQemuGuestCheck(machineScope) { + if err := machineScope.InfraCluster.ProxmoxClient.QemuAgentStatus(ctx, machineScope.VirtualMachine); err != nil { + return true, errors.Wrap(err, "error waiting for agent") } - if errors.Is(goproxmox.ErrCloudInitFailed, err) { - conditions.MarkFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition, infrav1alpha1.VMProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error()) - machineScope.SetFailureMessage(err) - machineScope.SetFailureReason(capierrors.MachineStatusError("BootstrapFailed")) + } + + if !skipCloudInitCheck(machineScope) { + if running, err := machineScope.InfraCluster.ProxmoxClient.CloudInitStatus(ctx, machineScope.VirtualMachine); err != nil || running { + if running { + return true, nil + } + if errors.Is(goproxmox.ErrCloudInitFailed, err) { + conditions.MarkFalse(machineScope.ProxmoxMachine, infrav1alpha1.VMProvisionedCondition, infrav1alpha1.VMProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error()) + machineScope.SetFailureMessage(err) + machineScope.SetFailureReason(capierrors.MachineStatusError("BootstrapFailed")) + } + return false, err } - return false, err } return false, nil diff --git a/internal/service/vmservice/vm_test.go b/internal/service/vmservice/vm_test.go index c03fb5c9..c016484a 100644 --- a/internal/service/vmservice/vm_test.go +++ b/internal/service/vmservice/vm_test.go @@ -42,6 +42,67 @@ func TestReconcileVM_EverythingReady(t *testing.T) { proxmoxClient.EXPECT().GetVM(context.Background(), "node1", int64(123)).Return(vm, nil).Once() proxmoxClient.EXPECT().CloudInitStatus(context.Background(), vm).Return(false, nil).Once() + proxmoxClient.EXPECT().QemuAgentStatus(context.Background(), vm).Return(nil).Once() + + result, err := ReconcileVM(context.Background(), machineScope) + require.NoError(t, err) + require.Equal(t, infrav1alpha1.VirtualMachineStateReady, result.State) + require.Equal(t, "10.10.10.10", machineScope.ProxmoxMachine.Status.Addresses[1].Address) +} + +func TestReconcileVM_QemuAgentCheckDisabled(t *testing.T) { + machineScope, proxmoxClient, _ := setupReconcilerTest(t) + vm := newRunningVM() + machineScope.SetVirtualMachineID(int64(vm.VMID)) + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + machineScope.ProxmoxMachine.Status.BootstrapDataProvided = ptr.To(true) + machineScope.ProxmoxMachine.Status.Ready = true + machineScope.ProxmoxMachine.Spec.Checks = &infrav1alpha1.ProxmoxMachineChecks{ + SkipQemuGuestAgent: ptr.To(true), + } + + proxmoxClient.EXPECT().GetVM(context.Background(), "node1", int64(123)).Return(vm, nil).Once() + proxmoxClient.EXPECT().CloudInitStatus(context.Background(), vm).Return(false, nil).Once() + + result, err := ReconcileVM(context.Background(), machineScope) + require.NoError(t, err) + require.Equal(t, infrav1alpha1.VirtualMachineStateReady, result.State) + require.Equal(t, "10.10.10.10", machineScope.ProxmoxMachine.Status.Addresses[1].Address) +} + +func TestReconcileVM_CloudInitCheckDisabled(t *testing.T) { + machineScope, proxmoxClient, _ := setupReconcilerTest(t) + vm := newRunningVM() + machineScope.SetVirtualMachineID(int64(vm.VMID)) + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + machineScope.ProxmoxMachine.Status.BootstrapDataProvided = ptr.To(true) + machineScope.ProxmoxMachine.Status.Ready = true + machineScope.ProxmoxMachine.Spec.Checks = &infrav1alpha1.ProxmoxMachineChecks{ + SkipCloudInitStatus: ptr.To(true), + } + + proxmoxClient.EXPECT().GetVM(context.Background(), "node1", int64(123)).Return(vm, nil).Once() + proxmoxClient.EXPECT().QemuAgentStatus(context.Background(), vm).Return(nil) + + result, err := ReconcileVM(context.Background(), machineScope) + require.NoError(t, err) + require.Equal(t, infrav1alpha1.VirtualMachineStateReady, result.State) + require.Equal(t, "10.10.10.10", machineScope.ProxmoxMachine.Status.Addresses[1].Address) +} + +func TestReconcileVM_InitCheckDisabled(t *testing.T) { + machineScope, proxmoxClient, _ := setupReconcilerTest(t) + vm := newRunningVM() + machineScope.SetVirtualMachineID(int64(vm.VMID)) + machineScope.ProxmoxMachine.Status.IPAddresses = map[string]infrav1alpha1.IPAddress{infrav1alpha1.DefaultNetworkDevice: {IPV4: "10.10.10.10"}} + machineScope.ProxmoxMachine.Status.BootstrapDataProvided = ptr.To(true) + machineScope.ProxmoxMachine.Status.Ready = true + machineScope.ProxmoxMachine.Spec.Checks = &infrav1alpha1.ProxmoxMachineChecks{ + SkipCloudInitStatus: ptr.To(true), + SkipQemuGuestAgent: ptr.To(true), + } + + proxmoxClient.EXPECT().GetVM(context.Background(), "node1", int64(123)).Return(vm, nil).Once() result, err := ReconcileVM(context.Background(), machineScope) require.NoError(t, err) @@ -323,6 +384,7 @@ func TestReconcileVM_CloudInitFailed(t *testing.T) { proxmoxClient.EXPECT().GetVM(context.Background(), "node1", int64(123)).Return(vm, nil).Once() proxmoxClient.EXPECT().CloudInitStatus(context.Background(), vm).Return(false, goproxmox.ErrCloudInitFailed).Once() + proxmoxClient.EXPECT().QemuAgentStatus(context.Background(), vm).Return(nil).Once() _, err := ReconcileVM(context.Background(), machineScope) require.Error(t, err, "unknown error") @@ -340,6 +402,7 @@ func TestReconcileVM_CloudInitRunning(t *testing.T) { proxmoxClient.EXPECT().GetVM(context.Background(), "node1", int64(123)).Return(vm, nil).Once() proxmoxClient.EXPECT().CloudInitStatus(context.Background(), vm).Return(true, nil).Once() + proxmoxClient.EXPECT().QemuAgentStatus(context.Background(), vm).Return(nil).Once() result, err := ReconcileVM(context.Background(), machineScope) require.NoError(t, err) diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 76187f6a..4c5f9431 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -48,6 +48,7 @@ type Client interface { TagVM(ctx context.Context, vm *proxmox.VirtualMachine, tag string) (*proxmox.Task, error) UnmountCloudInitISO(ctx context.Context, vm *proxmox.VirtualMachine, device string) error - CloudInitStatus(ctx context.Context, vm *proxmox.VirtualMachine) (bool, error) + + QemuAgentStatus(ctx context.Context, vm *proxmox.VirtualMachine) error } diff --git a/pkg/proxmox/goproxmox/api_client.go b/pkg/proxmox/goproxmox/api_client.go index 37502743..3af6e726 100644 --- a/pkg/proxmox/goproxmox/api_client.go +++ b/pkg/proxmox/goproxmox/api_client.go @@ -300,3 +300,11 @@ func (c *APIClient) CloudInitStatus(ctx context.Context, vm *proxmox.VirtualMach return false, nil } + +func (c *APIClient) QemuAgentStatus(ctx context.Context, vm *proxmox.VirtualMachine) error { + if err := vm.WaitForAgent(ctx, 5); err != nil { + return errors.Wrap(err, "error waiting for agent") + } + + return nil +} diff --git a/pkg/proxmox/proxmoxtest/mock_client.go b/pkg/proxmox/proxmoxtest/mock_client.go index 6cb0fe01..8492503f 100644 --- a/pkg/proxmox/proxmoxtest/mock_client.go +++ b/pkg/proxmox/proxmoxtest/mock_client.go @@ -477,6 +477,49 @@ func (_c *MockClient_GetVM_Call) RunAndReturn(run func(context.Context, string, return _c } +// QemuAgentStatus provides a mock function with given fields: ctx, vm +func (_m *MockClient) QemuAgentStatus(ctx context.Context, vm *go_proxmox.VirtualMachine) error { + ret := _m.Called(ctx, vm) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *go_proxmox.VirtualMachine) error); ok { + r0 = rf(ctx, vm) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockClient_QemuAgentStatus_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'QemuAgentStatus' +type MockClient_QemuAgentStatus_Call struct { + *mock.Call +} + +// QemuAgentStatus is a helper method to define mock.On call +// - ctx context.Context +// - vm *go_proxmox.VirtualMachine +func (_e *MockClient_Expecter) QemuAgentStatus(ctx interface{}, vm interface{}) *MockClient_QemuAgentStatus_Call { + return &MockClient_QemuAgentStatus_Call{Call: _e.mock.On("QemuAgentStatus", ctx, vm)} +} + +func (_c *MockClient_QemuAgentStatus_Call) Run(run func(ctx context.Context, vm *go_proxmox.VirtualMachine)) *MockClient_QemuAgentStatus_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*go_proxmox.VirtualMachine)) + }) + return _c +} + +func (_c *MockClient_QemuAgentStatus_Call) Return(_a0 error) *MockClient_QemuAgentStatus_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockClient_QemuAgentStatus_Call) RunAndReturn(run func(context.Context, *go_proxmox.VirtualMachine) error) *MockClient_QemuAgentStatus_Call { + _c.Call.Return(run) + return _c +} + // ResizeDisk provides a mock function with given fields: ctx, vm, disk, size func (_m *MockClient) ResizeDisk(ctx context.Context, vm *go_proxmox.VirtualMachine, disk string, size string) error { ret := _m.Called(ctx, vm, disk, size)