Add new DV reason: ImagePullFailed

When the image pull fails, the DataVolume Running condition, will have
the Reason field of `ImagePullFailed`, to allow better error handling in
code taht uses it.

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

cmd/cdi-importer/importer.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/utils/ptr"
 
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
+
 	"kubevirt.io/containerized-data-importer/pkg/common"
 	cc "kubevirt.io/containerized-data-importer/pkg/controller/common"
 	"kubevirt.io/containerized-data-importer/pkg/image"

pkg/common/common.go

@@ -293,6 +293,10 @@ const (
 
 	// CDIControllerLeaderElectionHelperName is the name of the configmap that is used as a helper for controller leader election
 	CDIControllerLeaderElectionHelperName = "cdi-controller-leader-election-helper"
+
+	// ImagePullFailureText is the text of the ErrImagePullFailed error. We need it as a common constant because we're using
+	// both to create and to later check the error in the termination text of the importer pod.
+	ImagePullFailureText = "failed to pull image"
 )
 
 // ProxyPaths are all supported paths

pkg/controller/util.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"crypto/rsa"
 	"encoding/json"
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -35,8 +36,8 @@ import (
 	"k8s.io/klog/v2"
 
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
-	"kubevirt.io/containerized-data-importer/pkg/common"
 
+	"kubevirt.io/containerized-data-importer/pkg/common"
 	cc "kubevirt.io/containerized-data-importer/pkg/controller/common"
 	"kubevirt.io/containerized-data-importer/pkg/util"
 	"kubevirt.io/containerized-data-importer/pkg/util/cert"
@@ -60,6 +61,9 @@ const (
 	// ScratchSpaceRequiredReason is a const that defines the pod exited due to a lack of scratch space
 	ScratchSpaceRequiredReason = "Scratch space required"
 
+	// ImagePullFailedReason is a const that defines the pod exited due to failure when pulling image
+	ImagePullFailedReason = "ImagePullFailed"
+
 	// ImportCompleteMessage is a const that defines the pod completeded the import successfully
 	ImportCompleteMessage = "Import Complete"
 
@@ -294,6 +298,18 @@ func setAnnotationsFromPodWithPrefix(anno map[string]string, pod *v1.Pod, termMs
 	}
 
 	anno[cc.AnnRunningCondition] = "false"
+
+	for _, status := range pod.Status.ContainerStatuses {
+		if status.Started != nil && !(*status.Started) {
+			if status.State.Waiting != nil &&
+				(status.State.Waiting.Reason == "ImagePullBackOff" || status.State.Waiting.Reason == "ErrImagePull") {
+				anno[prefix+".message"] = fmt.Sprintf("%s: %s", common.ImagePullFailureText, status.Image)
+				anno[prefix+".reason"] = ImagePullFailedReason
+				return
+			}
+		}
+	}
+
 	if containerState.Waiting != nil && containerState.Waiting.Reason != "CrashLoopBackOff" {
 		anno[prefix+".message"] = simplifyKnownMessage(containerState.Waiting.Message)
 		anno[prefix+".reason"] = containerState.Waiting.Reason
@@ -327,6 +343,11 @@ func setAnnotationsFromPodWithPrefix(anno map[string]string, pod *v1.Pod, termMs
 			if strings.Contains(containerState.Terminated.Message, common.PreallocationApplied) {
 				anno[cc.AnnPreallocationApplied] = "true"
 			}
+
+			if strings.Contains(containerState.Terminated.Message, common.ImagePullFailureText) {
+				anno[prefix+".reason"] = ImagePullFailedReason
+				return
+			}
 		}
 		anno[prefix+".reason"] = containerState.Terminated.Reason
 	}

pkg/controller/util_test.go

@@ -17,6 +17,7 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
+
 	"kubevirt.io/containerized-data-importer/pkg/common"
 	. "kubevirt.io/containerized-data-importer/pkg/controller/common"
 	"kubevirt.io/containerized-data-importer/pkg/util/cert"
@@ -230,6 +231,55 @@ var _ = Describe("setAnnotationsFromPod", func() {
 		Expect(result[AnnRunningConditionReason]).To(Equal(ScratchSpaceRequiredReason))
 		Expect(result[AnnRequiresScratch]).To(Equal("true"))
 	})
+
+	It("Should set image pull failure message and reason", func() {
+		const errorIncludesImagePullText = `Unable to process data: ` + common.ImagePullFailureText + `: reading manifest wrong
+in quay.io/myproject/myimage: manifest unknown`
+
+		result := make(map[string]string)
+		testPod := CreateImporterTestPod(CreatePvc("test", metav1.NamespaceDefault, nil, nil), "test", nil)
+		testPod.Status = v1.PodStatus{
+			ContainerStatuses: []v1.ContainerStatus{
+				{
+					State: v1.ContainerState{
+						Terminated: &v1.ContainerStateTerminated{
+							Message: errorIncludesImagePullText,
+							Reason:  common.GenericError,
+						},
+					},
+				},
+			},
+		}
+		setAnnotationsFromPodWithPrefix(result, testPod, nil, AnnRunningCondition)
+		Expect(result[AnnRunningCondition]).To(Equal("false"))
+		Expect(result[AnnRunningConditionMessage]).To(Equal(errorIncludesImagePullText))
+		Expect(result[AnnRunningConditionReason]).To(Equal(ImagePullFailedReason))
+		Expect(result[AnnRequiresScratch]).To(BeEmpty())
+	})
+
+	It("Should set running reason as error for general errors", func() {
+		const errorMessage = `just a fake error text to check in this test`
+
+		result := make(map[string]string)
+		testPod := CreateImporterTestPod(CreatePvc("test", metav1.NamespaceDefault, nil, nil), "test", nil)
+		testPod.Status = v1.PodStatus{
+			ContainerStatuses: []v1.ContainerStatus{
+				{
+					State: v1.ContainerState{
+						Terminated: &v1.ContainerStateTerminated{
+							Message: errorMessage,
+							Reason:  common.GenericError,
+						},
+					},
+				},
+			},
+		}
+		setAnnotationsFromPodWithPrefix(result, testPod, nil, AnnRunningCondition)
+		Expect(result[AnnRunningCondition]).To(Equal("false"))
+		Expect(result[AnnRunningConditionMessage]).To(Equal(errorMessage))
+		Expect(result[AnnRunningConditionReason]).To(Equal(common.GenericError))
+		Expect(result[AnnRequiresScratch]).To(BeEmpty())
+	})
 })
 
 var _ = Describe("addLabelsFromTerminationMessage", func() {

pkg/importer/BUILD.bazel

@@ -4,6 +4,7 @@ go_library(
     name = "go_default_library",
     srcs = [
         "data-processor.go",
+        "errors.go",
         "format-readers.go",
         "gcs-datasource.go",
         "http-datasource.go",

pkg/importer/data-processor.go

@@ -17,7 +17,6 @@ limitations under the License.
 package importer
 
 import (
-	"fmt"
 	"net/url"
 	"os"
 
@@ -62,19 +61,6 @@ const (
 	ProcessingPhaseMergeDelta ProcessingPhase = "MergeDelta"
 )
 
-// ValidationSizeError is an error indication size validation failure.
-type ValidationSizeError struct {
-	err error
-}
-
-func (e ValidationSizeError) Error() string { return e.err.Error() }
-
-// ErrRequiresScratchSpace indicates that we require scratch space.
-var ErrRequiresScratchSpace = fmt.Errorf(common.ScratchSpaceRequired)
-
-// ErrInvalidPath indicates that the path is invalid.
-var ErrInvalidPath = fmt.Errorf("invalid transfer path")
-
 // may be overridden in tests
 var getAvailableSpaceBlockFunc = util.GetAvailableSpaceBlock
 var getAvailableSpaceFunc = util.GetAvailableSpace

pkg/importer/errors.go

@@ -0,0 +1,42 @@
+package importer
+
+import (
+	"fmt"
+
+	"kubevirt.io/containerized-data-importer/pkg/common"
+)
+
+// ValidationSizeError is an error indication size validation failure.
+type ValidationSizeError struct {
+	err error
+}
+
+func (e ValidationSizeError) Error() string { return e.err.Error() }
+
+// ErrRequiresScratchSpace indicates that we require scratch space.
+var ErrRequiresScratchSpace = fmt.Errorf(common.ScratchSpaceRequired)
+
+// ErrInvalidPath indicates that the path is invalid.
+var ErrInvalidPath = fmt.Errorf("invalid transfer path")
+
+// ErrImagePullFailed indicates that the importer failed to pull an image; This error type wraps the actual error.
+type ErrImagePullFailed struct {
+	err error
+}
+
+// NewErrImagePullFailed creates new ErrImagePullFailed error object, with embedded error.
+//
+// Use the err parameter fot the actual wrapped error
+func NewErrImagePullFailed(err error) *ErrImagePullFailed {
+	return &ErrImagePullFailed{
+		err: err,
+	}
+}
+
+func (err *ErrImagePullFailed) Error() string {
+	return fmt.Sprintf("%s: %s", common.ImagePullFailureText, err.err.Error())
+}
+
+func (err *ErrImagePullFailed) Unwrap() error {
+	return err.err
+}

pkg/importer/gcs-datasource.go

@@ -128,7 +128,7 @@ func (sd *GCSDataSource) Transfer(path string) (ProcessingPhase, error) {
 		return ProcessingPhaseError, ErrInvalidPath
 	}
 
-	err := util.StreamDataToFile(sd.readers.TopReader(), file)
+	err := streamDataToFile(sd.readers.TopReader(), file)
 
 	if err != nil {
 		klog.V(3).Infoln("GCS Importer: Transfer Error: ", err)
@@ -145,7 +145,7 @@ func (sd *GCSDataSource) TransferFile(fileName string) (ProcessingPhase, error)
 		return ProcessingPhaseError, err
 	}
 
-	err := util.StreamDataToFile(sd.readers.TopReader(), fileName)
+	err := streamDataToFile(sd.readers.TopReader(), fileName)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}

pkg/importer/http-datasource.go

@@ -39,6 +39,7 @@ import (
 	"k8s.io/klog/v2"
 
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
+
 	"kubevirt.io/containerized-data-importer/pkg/common"
 	"kubevirt.io/containerized-data-importer/pkg/image"
 	"kubevirt.io/containerized-data-importer/pkg/util"
@@ -160,7 +161,7 @@ func (hs *HTTPDataSource) Transfer(path string) (ProcessingPhase, error) {
 		if err != nil || size <= 0 {
 			return ProcessingPhaseError, ErrInvalidPath
 		}
-		err = util.StreamDataToFile(hs.readers.TopReader(), file)
+		err = streamDataToFile(hs.readers.TopReader(), file)
 		if err != nil {
 			return ProcessingPhaseError, err
 		}
@@ -183,7 +184,7 @@ func (hs *HTTPDataSource) TransferFile(fileName string) (ProcessingPhase, error)
 		return ProcessingPhaseError, err
 	}
 	hs.readers.StartProgressUpdate()
-	err := util.StreamDataToFile(hs.readers.TopReader(), fileName)
+	err := streamDataToFile(hs.readers.TopReader(), fileName)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}

pkg/importer/imageio-datasource.go

@@ -136,7 +136,7 @@ func (is *ImageioDataSource) Transfer(path string) (ProcessingPhase, error) {
 		return ProcessingPhaseError, ErrInvalidPath
 	}
 	is.readers.StartProgressUpdate()
-	err = util.StreamDataToFile(is.readers.TopReader(), file)
+	err = streamDataToFile(is.readers.TopReader(), file)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}
@@ -181,7 +181,7 @@ func (is *ImageioDataSource) TransferFile(fileName string) (ProcessingPhase, err
 			return ProcessingPhaseError, err
 		}
 	} else {
-		err := util.StreamDataToFile(is.readers.TopReader(), fileName)
+		err := streamDataToFile(is.readers.TopReader(), fileName)
 		if err != nil {
 			return ProcessingPhaseError, err
 		}

pkg/importer/s3-datasource.go

@@ -100,7 +100,7 @@ func (sd *S3DataSource) Transfer(path string) (ProcessingPhase, error) {
 		return ProcessingPhaseError, ErrInvalidPath
 	}
 
-	err := util.StreamDataToFile(sd.readers.TopReader(), file)
+	err := streamDataToFile(sd.readers.TopReader(), file)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}
@@ -115,7 +115,7 @@ func (sd *S3DataSource) TransferFile(fileName string) (ProcessingPhase, error) {
 		return ProcessingPhaseError, err
 	}
 
-	err := util.StreamDataToFile(sd.readers.TopReader(), fileName)
+	err := streamDataToFile(sd.readers.TopReader(), fileName)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}

pkg/importer/transport.go

@@ -34,7 +34,6 @@ import (
 	"k8s.io/klog/v2"
 
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
-	"kubevirt.io/containerized-data-importer/pkg/util"
 )
 
 const (
@@ -76,7 +75,7 @@ func readImageSource(ctx context.Context, sys *types.SystemContext, img string)
 	src, err := ref.NewImageSource(ctx, sys)
 	if err != nil {
 		klog.Errorf("Could not create image reference: %v", err)
-		return nil, errors.Wrap(err, "Could not create image reference")
+		return nil, NewErrImagePullFailed(err)
 	}
 
 	return src, nil
@@ -157,7 +156,7 @@ func processLayer(ctx context.Context,
 				return false, errors.Wrap(err, "Error creating output file's directory")
 			}
 
-			if err := util.StreamDataToFile(tarReader, filepath.Join(destDir, hdr.Name)); err != nil {
+			if err := streamDataToFile(tarReader, filepath.Join(destDir, hdr.Name)); err != nil {
 				klog.Errorf("Error copying file: %v", err)
 				return false, errors.Wrap(err, "Error copying file")
 			}

pkg/importer/upload-datasource.go

@@ -9,6 +9,7 @@ import (
 	"k8s.io/klog/v2"
 
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
+
 	"kubevirt.io/containerized-data-importer/pkg/common"
 	"kubevirt.io/containerized-data-importer/pkg/util"
 )
@@ -72,7 +73,7 @@ func (ud *UploadDataSource) Transfer(path string) (ProcessingPhase, error) {
 			//Path provided is invalid.
 			return ProcessingPhaseError, ErrInvalidPath
 		}
-		err = util.StreamDataToFile(ud.readers.TopReader(), file)
+		err = streamDataToFile(ud.readers.TopReader(), file)
 		if err != nil {
 			return ProcessingPhaseError, err
 		}
@@ -94,7 +95,7 @@ func (ud *UploadDataSource) TransferFile(fileName string) (ProcessingPhase, erro
 	if err := CleanAll(fileName); err != nil {
 		return ProcessingPhaseError, err
 	}
-	err := util.StreamDataToFile(ud.readers.TopReader(), fileName)
+	err := streamDataToFile(ud.readers.TopReader(), fileName)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}
@@ -158,7 +159,7 @@ func (aud *AsyncUploadDataSource) Transfer(path string) (ProcessingPhase, error)
 		//Path provided is invalid.
 		return ProcessingPhaseError, ErrInvalidPath
 	}
-	err = util.StreamDataToFile(aud.uploadDataSource.readers.TopReader(), file)
+	err = streamDataToFile(aud.uploadDataSource.readers.TopReader(), file)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}
@@ -173,7 +174,7 @@ func (aud *AsyncUploadDataSource) TransferFile(fileName string) (ProcessingPhase
 	if err := CleanAll(fileName); err != nil {
 		return ProcessingPhaseError, err
 	}
-	err := util.StreamDataToFile(aud.uploadDataSource.readers.TopReader(), fileName)
+	err := streamDataToFile(aud.uploadDataSource.readers.TopReader(), fileName)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}