diff --git a/internal/controller/openstacknodeimagerelease_controller.go b/internal/controller/openstacknodeimagerelease_controller.go index 73c984d3..fe570c3d 100644 --- a/internal/controller/openstacknodeimagerelease_controller.go +++ b/internal/controller/openstacknodeimagerelease_controller.go @@ -147,7 +147,7 @@ func (r *OpenStackNodeImageReleaseReconciler) Reconcile(ctx context.Context, req conditions.MarkTrue(openstacknodeimagerelease, apiv1alpha1.OpenStackImageServiceClientAvailableCondition) - imageID, err := findImageByName(imageClient, openstacknodeimagerelease.Spec.Image.CreateOpts.Name) + imageID, err := getImageID(imageClient, openstacknodeimagerelease.Spec.Image.CreateOpts) if err != nil { conditions.MarkFalse(openstacknodeimagerelease, apiv1alpha1.OpenStackImageReadyCondition, @@ -331,27 +331,38 @@ func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Con return cloud, nil } -func findImageByName(imagesClient *gophercloud.ServiceClient, imageName string) (string, error) { - listOpts := images.ListOpts{ - Name: imageName, +func getImageID(imagesClient *gophercloud.ServiceClient, imageCreateOps *apiv1alpha1.CreateOpts) (string, error) { + var listOpts images.ListOpts + + if imageCreateOps.ID != "" { + listOpts = images.ListOpts{ + ID: imageCreateOps.ID, + } + } else { + listOpts = images.ListOpts{ + Name: imageCreateOps.Name, + Tags: imageCreateOps.Tags, + } } allPages, err := images.List(imagesClient, listOpts).AllPages() if err != nil { - return "", fmt.Errorf("failed to list images with name %s: %w", imageName, err) + return "", fmt.Errorf("failed to list images with name %s: %w", imageCreateOps.Name, err) } imageList, err := images.ExtractImages(allPages) if err != nil { - return "", fmt.Errorf("failed to extract images with name %s: %w", imageName, err) + return "", fmt.Errorf("failed to extract images with name %s: %w", imageCreateOps.Name, err) } - for i := range imageList { - if imageList[i].Name == imageName { - return imageList[i].ID, nil - } + switch len(imageList) { + case 0: + return "", nil + case 1: + return imageList[0].ID, nil + default: + return "", fmt.Errorf("too many images were found with the given image name: %s and tags: %s", imageCreateOps.Name, imageCreateOps.Tags) } - return "", nil } func createImage(imageClient *gophercloud.ServiceClient, createOpts *apiv1alpha1.CreateOpts) (*images.Image, error) { diff --git a/internal/controller/openstacknodeimagerelease_controller_test.go b/internal/controller/openstacknodeimagerelease_controller_test.go index 841faaf6..ac2be359 100644 --- a/internal/controller/openstacknodeimagerelease_controller_test.go +++ b/internal/controller/openstacknodeimagerelease_controller_test.go @@ -188,15 +188,34 @@ clouds: assert.NoError(t, err) } -func TestFindImageByName(t *testing.T) { +func TestGetImageID(t *testing.T) { th.SetupHTTP() defer th.TeardownHTTP() HandleImageListSuccessfully(t) - imageName := "test_image" + imageFilter := &apiv1alpha1.CreateOpts{ + ID: "123", + } + + imageID, err := getImageID(fakeclient.ServiceClient(), imageFilter) + + assert.NoError(t, err) + assert.Equal(t, "123", imageID) +} + +func TestGetImageIDByNameAndTags(t *testing.T) { + th.SetupHTTP() + defer th.TeardownHTTP() + + HandleImageListSuccessfully(t) + + imageFilter := &apiv1alpha1.CreateOpts{ + Name: "test_image", + Tags: []string{"v1"}, + } - imageID, err := findImageByName(fakeclient.ServiceClient(), imageName) + imageID, err := getImageID(fakeclient.ServiceClient(), imageFilter) assert.NoError(t, err) assert.Equal(t, "123", imageID) @@ -213,33 +232,103 @@ func HandleImageListSuccessfully(t *testing.T) { //nolint: gocritic w.WriteHeader(http.StatusOK) fmt.Fprintf(w, `{ "images": [ - {"id": "123", "name": "test_image"}, - {"id": "456", "name": "test_image2"}, - {"id": "789", "name": "test_image3"} + {"id": "123", "name": "test_image", "tags": ["v1"]} + ] + }`) + }) +} + +func TestGetImageIDWithTwoSameImageNames(t *testing.T) { + th.SetupHTTP() + defer th.TeardownHTTP() + + HandleImageListWithTwoImagesSuccessfully(t) + + imageFilter := &apiv1alpha1.CreateOpts{ + Name: "test_image", + Tags: []string{"v1"}, + } + + imageID, err := getImageID(fakeclient.ServiceClient(), imageFilter) + + assert.Error(t, err) // Expecting an error due to multiple images with the same name + assert.Equal(t, "", imageID) + assert.Equal(t, err.Error(), "too many images were found with the given image name: test_image and tags: [v1]") +} + +// HandleImageListWithTwoImagesSuccessfully sets up a fake response for image list request. +func HandleImageListWithTwoImagesSuccessfully(t *testing.T) { //nolint: gocritic + t.Helper() // Indicate that this is a test helper function + th.Mux.HandleFunc("/images", func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, "GET") + th.TestHeader(t, r, "X-Auth-Token", fakeclient.TokenID) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + fmt.Fprintf(w, `{ + "images": [ + {"id": "123", "name": "test_image", "tags": ["v1"]}, + {"id": "456", "name": "test_image", "tags": ["v1"]} ] }`) }) } -func TestFindImageByNameWrongImageName(t *testing.T) { +func TestGetImageIDNoImageFound(t *testing.T) { + th.SetupHTTP() + defer th.TeardownHTTP() + + HandleImageListWithNoImageFoundSuccessfully(t) + + imageFilter := &apiv1alpha1.CreateOpts{ + Name: "test_image", + Tags: []string{"v1"}, + } + + imageID, err := getImageID(fakeclient.ServiceClient(), imageFilter) + + assert.NoError(t, err) + assert.Equal(t, "", imageID) +} + +// HandleImageListWithNoImageFoundSuccessfully sets up a fake response for image list request. +func HandleImageListWithNoImageFoundSuccessfully(t *testing.T) { //nolint: gocritic + t.Helper() // Indicate that this is a test helper function + th.Mux.HandleFunc("/images", func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, "GET") + th.TestHeader(t, r, "X-Auth-Token", fakeclient.TokenID) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + fmt.Fprintf(w, `{ + "images": [] + }`) + }) +} + +func TestGetImageIDWrongImageName(t *testing.T) { th.SetupHTTP() defer th.TeardownHTTP() HandleImageListSuccessfully(t) - imageName := "test_bad_image" + imageFilter := &apiv1alpha1.CreateOpts{ + Name: "test_bad_image", + } - imageID, err := findImageByName(fakeclient.ServiceClient(), imageName) + imageID, err := getImageID(fakeclient.ServiceClient(), imageFilter) assert.NoError(t, err) assert.NotEqual(t, "231", imageID) } -func TestFindImageByNameNotFound(t *testing.T) { +func TestGetImageIDNotFound(t *testing.T) { th.SetupHTTP() defer th.TeardownHTTP() - imageName := "test_image" + imageFilter := &apiv1alpha1.CreateOpts{ + Name: "test_image", + } th.Mux.HandleFunc("/images", func(w http.ResponseWriter, r *http.Request) { th.TestMethod(t, r, http.MethodGet) @@ -253,11 +342,11 @@ func TestFindImageByNameNotFound(t *testing.T) { fakeClient := fakeclient.ServiceClient() - imageID, err := findImageByName(fakeClient, imageName) + imageID, err := getImageID(fakeClient, imageFilter) assert.Error(t, err) assert.Equal(t, "", imageID) - assert.Contains(t, err.Error(), fmt.Sprintf("failed to list images with name %s: Resource not found", imageName)) + assert.Contains(t, err.Error(), fmt.Sprintf("failed to list images with name %s: Resource not found", imageFilter.Name)) } // HandleImageCreationSuccessfully test setup.