From d2181a88f6b905544b6a2c9bd4e70e0bbf1da690 Mon Sep 17 00:00:00 2001 From: rojanDinc Date: Tue, 21 Oct 2025 23:11:31 +0200 Subject: [PATCH] fix: log error when instance metadata retrieval fails Added error logging in the InstanceMetadata function to capture failures when retrieving instance information, enhancing debugging capabilities. Also includes: - Added error check for metadata retrieval - Added unit tests for error handling - Updated to use errors package for error equality Signed-off-by: rojanDinc Signed-off-by: Serge Logvinov --- pkg/proxmox/instances.go | 7 +++ pkg/proxmox/instances_test.go | 106 ++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/pkg/proxmox/instances.go b/pkg/proxmox/instances.go index 49fb1ec..3ffab3f 100644 --- a/pkg/proxmox/instances.go +++ b/pkg/proxmox/instances.go @@ -18,6 +18,7 @@ package proxmox import ( "context" + "errors" "fmt" "net" "regexp" @@ -190,6 +191,8 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud info, err = i.getInstanceInfo(ctx, node) if mc.ObserveRequest(err) != nil { + klog.ErrorS(err, "instances.InstanceMetadata() failed to get instance info", "node", klog.KObj(node)) + if err == proxmoxpool.ErrInstanceNotFound { klog.V(4).InfoS("instances.InstanceMetadata() instance not found", "node", klog.KObj(node), "providerID", providerID) @@ -311,6 +314,10 @@ func (i *instances) getInstanceInfo(ctx context.Context, node *v1.Node) (*instan vmID, region, err = i.c.pxpool.FindVMByUUID(ctx, node.Status.NodeInfo.SystemUUID) if err != nil { + if errors.Is(err, proxmoxpool.ErrInstanceNotFound) { + return nil, cloudprovider.InstanceNotFound + } + return nil, fmt.Errorf("instances.getInstanceInfo() error: %v", err) } } diff --git a/pkg/proxmox/instances_test.go b/pkg/proxmox/instances_test.go index 5c82e32..c8a60a9 100644 --- a/pkg/proxmox/instances_test.go +++ b/pkg/proxmox/instances_test.go @@ -355,6 +355,112 @@ func (ts *ccmTestSuite) TestInstanceExists() { } } +func (ts *ccmTestSuite) TestInstanceExistsCAPMox() { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + // Set up a CAPMox provider instance for this test + cfg, err := providerconfig.ReadCloudConfig(strings.NewReader(` +features: + provider: 'capmox' +clusters: +- url: https://127.0.0.1:8006/api2/json + insecure: false + token_id: "user!token-id" + token_secret: "secret" + region: cluster-1 +`)) + if err != nil { + ts.T().Fatalf("failed to read config: %v", err) + } + + px, err := proxmoxpool.NewProxmoxPool(ts.T().Context(), cfg.Clusters, proxmox.WithHTTPClient(&http.Client{})) + if err != nil { + ts.T().Fatalf("failed to create cluster client: %v", err) + } + + client := &client{ + pxpool: px, + kclient: fake.NewSimpleClientset(), + } + + capmoxInstance := newInstances(client, cfg.Features) + + tests := []struct { + msg string + node *v1.Node + expectedError string + expected bool + }{ + { + msg: "NodeUUIDNotFoundCAPMox", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "talos-rqa-u7y", + }, + Spec: v1.NodeSpec{ + ProviderID: "proxmox://d290d7f2-b179-404c-b627-6e4dccb59066", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + SystemUUID: "d290d7f2-b179-404c-b627-6e4dccb59066", + }, + }, + }, + expected: false, + }, + { + msg: "NodeUUIDNotFoundCAPMoxDifferentFormat", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "talos-missing-node", + }, + Spec: v1.NodeSpec{ + ProviderID: "proxmox://00000000-0000-0000-0000-000000000000", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + SystemUUID: "00000000-0000-0000-0000-000000000000", + }, + }, + }, + expected: false, + }, + { + msg: "NodeUUIDFoundCAPMox", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1-node-1", + }, + Spec: v1.NodeSpec{ + ProviderID: "proxmox://8af7110d-bfad-407a-a663-9527d10a6583", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + SystemUUID: "8af7110d-bfad-407a-a663-9527d10a6583", + }, + }, + }, + expected: true, + }, + } + + for _, testCase := range tests { + ts.Run(fmt.Sprint(testCase.msg), func() { + exists, err := capmoxInstance.InstanceExists(ts.T().Context(), testCase.node) + + if testCase.expectedError != "" { + ts.Require().Error(err) + ts.Require().False(exists) + ts.Require().Contains(err.Error(), testCase.expectedError) + } else { + ts.Require().NoError(err) + ts.Require().Equal(testCase.expected, exists) + } + }) + } +} + // nolint:dupl func (ts *ccmTestSuite) TestInstanceShutdown() { httpmock.Activate()