From 53032f442c0aed2e20e579f41424d4dc26356c73 Mon Sep 17 00:00:00 2001 From: Ririko Nakamura Date: Fri, 19 Dec 2025 17:04:23 +0800 Subject: [PATCH 1/2] Map flavor name not found Internal error to the ResourceExhausted MCM error --- pkg/driver/executor/errors.go | 13 +++++++++++++ pkg/driver/executor/executor.go | 7 ++++++- pkg/driver/executor/executor_test.go | 28 ++++++++++++++++++++++++++++ pkg/driver/utils.go | 4 ++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/pkg/driver/executor/errors.go b/pkg/driver/executor/errors.go index 0c5b7127..fd8c1cab 100644 --- a/pkg/driver/executor/errors.go +++ b/pkg/driver/executor/errors.go @@ -24,3 +24,16 @@ var ( // OpenStack resources. In case this case, where a unique ID could not be determined an ErrMultipleFound is returned. ErrMultipleFound = fmt.Errorf("multiple resources found") ) + +// ErrFlavorNotFound is returned when there is no flavor can be matched with the specified flavor name. +// It can happen when certain flavor is not available in specified region and needs to be treated as ResourceExhausted +// to allow fallback to other flavors. +type ErrFlavorNotFound struct { + gophercloud.BaseError + Flavor string +} + +func (e ErrFlavorNotFound) string { + e.DefaultErrString = fmt.Sprintf("Unable to find flavor with name %s", e.Flavor) + return e.choseErrString() +} \ No newline at end of file diff --git a/pkg/driver/executor/executor.go b/pkg/driver/executor/executor.go index 5387cbc3..ba026501 100644 --- a/pkg/driver/executor/executor.go +++ b/pkg/driver/executor/executor.go @@ -276,7 +276,12 @@ func (ex *Executor) deployServer(ctx context.Context, machineName string, userDa } flavorRef, err := ex.Compute.FlavorIDFromName(ctx, flavorName) if err != nil { - return nil, fmt.Errorf("error resolving flavor ID from flavor name %q: %v", imageName, err) + switch v := err.(type) { + case gophercloud.ErrResourceNotFound: + return nil, ErrFlavorNotFound{Flavor: flavorName} + default: + return nil, fmt.Errorf("error resolving flavor ID from flavor name %q: %v", flavorName, err) + } } createOpts := &servers.CreateOpts{ diff --git a/pkg/driver/executor/executor_test.go b/pkg/driver/executor/executor_test.go index cc639bad..28f6965f 100644 --- a/pkg/driver/executor/executor_test.go +++ b/pkg/driver/executor/executor_test.go @@ -209,6 +209,34 @@ var _ = Describe("Executor", func() { Expect(server.ProviderID).To(Equal(encodeProviderID(region, serverID))) }) + It("should raise a ResourceExhausted error when called with a missing flavor", func() { + var ( + flavorName = "notSupportedFlavor" + ) + ex := &Executor{ + Compute: compute, + Network: network, + Config: cfg, + } + + compute.EXPECT().ListServers(ctx, &servers.ListOpts{Name: machineName}).Return([]servers.Server{}, nil) + compute.EXPECT().ImageIDFromName(ctx, imageName).Return(images.Image{ID: "imageID"}, nil) + compute.EXPECT().FlavorIDFromName(ctx, flavorName).Return(nil, ErrFlavorNotFound{Flavor: flavorName}) + compute.EXPECT().CreateServer(ctx, gomock.Any(), gomock.Any()).Return(&servers.Server{ + ID: serverID, + }, nil) + gomock.InOrder( + // we return an error to avoid waiting for the wait.Poll timeout + compute.EXPECT().GetServer(ctx, serverID).Return(nil, fmt.Errorf("error fetching server")), + compute.EXPECT().ListServers(ctx, &servers.ListOpts{Name: machineName}).Return([]servers.Server{*server}, nil), + compute.EXPECT().DeleteServer(ctx, serverID).Return(nil), + compute.EXPECT().GetServer(ctx, serverID).Do(func(_ context.Context, _ string) { server.Status = client.ServerStatusDeleted }).Return(server, nil), + ) + + _, err := ex.CreateMachine(ctx, machineName, nil) + Expect(err).To(HaveOccurred()) + }) + It("should delete the server on failure", func() { ex := &Executor{ Compute: compute, diff --git a/pkg/driver/utils.go b/pkg/driver/utils.go index 41b9c0d9..a68d77bf 100644 --- a/pkg/driver/utils.go +++ b/pkg/driver/utils.go @@ -63,6 +63,10 @@ func mapErrorToCode(err error) codes.Code { return codes.PermissionDenied } + if errors.Is(err, executor.ErrFlavorNotFound) { + return codes.ResourceExhausted + } + return mapErrorMessageToCode(err) } From 6a2dcd6420e6752d9db82ec4973191e9817f3e53 Mon Sep 17 00:00:00 2001 From: Ririko Nakamura Date: Tue, 23 Dec 2025 13:56:37 +0800 Subject: [PATCH 2/2] Fix errors and extend tests --- pkg/driver/driver_test.go | 29 ++++++++++++++++++++++++++++ pkg/driver/executor/errors.go | 6 ++---- pkg/driver/executor/executor.go | 5 +++-- pkg/driver/executor/executor_test.go | 18 +++-------------- pkg/driver/utils.go | 2 +- 5 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 pkg/driver/driver_test.go diff --git a/pkg/driver/driver_test.go b/pkg/driver/driver_test.go new file mode 100644 index 00000000..57b42292 --- /dev/null +++ b/pkg/driver/driver_test.go @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package driver + +import ( + "errors" + + "github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/status" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/gardener/machine-controller-manager-provider-openstack/pkg/driver/executor" +) + +var _ = Describe("Driver", func() { + + Context("mapErrorToCode", func() { + It("should map executor.ErrFlavorNotFound errors to ResourceExhausted error code", func() { + err1 := executor.ErrFlavorNotFound{Flavor: "flavor"} + err2 := status.Error(mapErrorToCode(err1), err1.Error()) + Expect(err1).To(HaveOccurred()) + Expect(err2).To(HaveOccurred()) + Expect(errors.Is(err2, executor.ErrNotFound)).To(BeFalse()) + }) + }) + +}) diff --git a/pkg/driver/executor/errors.go b/pkg/driver/executor/errors.go index fd8c1cab..391e63b1 100644 --- a/pkg/driver/executor/errors.go +++ b/pkg/driver/executor/errors.go @@ -29,11 +29,9 @@ var ( // It can happen when certain flavor is not available in specified region and needs to be treated as ResourceExhausted // to allow fallback to other flavors. type ErrFlavorNotFound struct { - gophercloud.BaseError Flavor string } -func (e ErrFlavorNotFound) string { - e.DefaultErrString = fmt.Sprintf("Unable to find flavor with name %s", e.Flavor) - return e.choseErrString() +func (e ErrFlavorNotFound) Error() string { + return fmt.Sprintf("Unable to find flavor with name %s", e.Flavor) } \ No newline at end of file diff --git a/pkg/driver/executor/executor.go b/pkg/driver/executor/executor.go index ba026501..3c05fc7d 100644 --- a/pkg/driver/executor/executor.go +++ b/pkg/driver/executor/executor.go @@ -10,6 +10,7 @@ import ( "fmt" "time" + "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/keypairs" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" @@ -276,9 +277,9 @@ func (ex *Executor) deployServer(ctx context.Context, machineName string, userDa } flavorRef, err := ex.Compute.FlavorIDFromName(ctx, flavorName) if err != nil { - switch v := err.(type) { + switch err.(type) { case gophercloud.ErrResourceNotFound: - return nil, ErrFlavorNotFound{Flavor: flavorName} + return nil, fmt.Errorf("error resolving flavor ID from flavor name %q: %w", flavorName, ErrFlavorNotFound{Flavor: flavorName}) default: return nil, fmt.Errorf("error resolving flavor ID from flavor name %q: %v", flavorName, err) } diff --git a/pkg/driver/executor/executor_test.go b/pkg/driver/executor/executor_test.go index 28f6965f..f668d241 100644 --- a/pkg/driver/executor/executor_test.go +++ b/pkg/driver/executor/executor_test.go @@ -209,10 +209,7 @@ var _ = Describe("Executor", func() { Expect(server.ProviderID).To(Equal(encodeProviderID(region, serverID))) }) - It("should raise a ResourceExhausted error when called with a missing flavor", func() { - var ( - flavorName = "notSupportedFlavor" - ) + It("should raise a ErrResourceNotFound error when called with a missing flavor", func() { ex := &Executor{ Compute: compute, Network: network, @@ -221,17 +218,8 @@ var _ = Describe("Executor", func() { compute.EXPECT().ListServers(ctx, &servers.ListOpts{Name: machineName}).Return([]servers.Server{}, nil) compute.EXPECT().ImageIDFromName(ctx, imageName).Return(images.Image{ID: "imageID"}, nil) - compute.EXPECT().FlavorIDFromName(ctx, flavorName).Return(nil, ErrFlavorNotFound{Flavor: flavorName}) - compute.EXPECT().CreateServer(ctx, gomock.Any(), gomock.Any()).Return(&servers.Server{ - ID: serverID, - }, nil) - gomock.InOrder( - // we return an error to avoid waiting for the wait.Poll timeout - compute.EXPECT().GetServer(ctx, serverID).Return(nil, fmt.Errorf("error fetching server")), - compute.EXPECT().ListServers(ctx, &servers.ListOpts{Name: machineName}).Return([]servers.Server{*server}, nil), - compute.EXPECT().DeleteServer(ctx, serverID).Return(nil), - compute.EXPECT().GetServer(ctx, serverID).Do(func(_ context.Context, _ string) { server.Status = client.ServerStatusDeleted }).Return(server, nil), - ) + compute.EXPECT().FlavorIDFromName(ctx, flavorName).Return(flavorName, gophercloud.ErrResourceNotFound{Name: flavorName, ResourceType: "flavor"}) + compute.EXPECT().ListServers(ctx, &servers.ListOpts{Name: machineName}).Return([]servers.Server{}, nil) _, err := ex.CreateMachine(ctx, machineName, nil) Expect(err).To(HaveOccurred()) diff --git a/pkg/driver/utils.go b/pkg/driver/utils.go index a68d77bf..1e4c21db 100644 --- a/pkg/driver/utils.go +++ b/pkg/driver/utils.go @@ -63,7 +63,7 @@ func mapErrorToCode(err error) codes.Code { return codes.PermissionDenied } - if errors.Is(err, executor.ErrFlavorNotFound) { + if errors.Is(err, executor.ErrFlavorNotFound{}) { return codes.ResourceExhausted }