Apply retry filter to ndkit conversions (#2584)

* Apply retry filter to ndkit conversions

Reduce the number of pod retries when there are
failures in downloading images. Instead retry
inside the pod.

Signed-off-by: Alexander Wels <awels@redhat.com>

* Address code review comments

Signed-off-by: Alexander Wels <awels@redhat.com>

---------

Signed-off-by: Alexander Wels <awels@redhat.com>

pkg/image/nbdkit.go

@@ -23,10 +23,6 @@ const (
 	defaultUserAgent      = "cdi-nbdkit-importer"
 )
 
-type nbdkitOperations struct {
-	nbdkit *Nbdkit
-}
-
 // NbdkitPlugin represents a plugin for nbdkit
 type NbdkitPlugin string
 
@@ -104,18 +100,22 @@ func NewNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraH
 	for _, header := range extraHeaders {
 		pluginArgs = append(pluginArgs, fmt.Sprintf("header=%s", header))
 	}
+	// Don't do exponential retry, the container restart will be exponential
+	pluginArgs = append(pluginArgs, "retry-exponential=no")
 	for _, header := range secretExtraHeaders {
 		redactArgs = append(redactArgs, fmt.Sprintf("header=%s", header))
 	}
-
-	return &Nbdkit{
+	n := &Nbdkit{
 		NbdPidFile: nbdkitPidFile,
 		plugin:     NbdkitCurlPlugin,
 		nbdkitArgs: args,
 		pluginArgs: pluginArgs,
 		redactArgs: redactArgs,
 		Socket:     socket,
 	}
+	// Should be last filter
+	n.AddFilter(NbdkitRetryFilter)
+	return n
 }
 
 // NewNbdkitVddk creates a new Nbdkit instance with the vddk plugin
@@ -211,9 +211,8 @@ func (n *Nbdkit) StartNbdkit(source string) error {
 		argsNbdkit = append(argsNbdkit, fmt.Sprintf("--filter=%s", f))
 	}
 	// set additional arguments
-	for _, a := range n.nbdkitArgs {
-		argsNbdkit = append(argsNbdkit, a)
-	}
+	argsNbdkit = append(argsNbdkit, n.nbdkitArgs...)
+
 	// append nbdkit plugin arguments
 	argsNbdkit = append(argsNbdkit, string(n.plugin))
 	argsNbdkit = append(argsNbdkit, n.pluginArgs...)

pkg/image/qemu.go

@@ -289,6 +289,7 @@ func (o *qemuOperations) CreateBlankImage(dest string, size resource.Quantity, p
 	err = os.Chmod(dest, 0660)
 	if err != nil {
 		err = errors.Wrap(err, "Unable to change permissions of target file")
+		return err
 	}
 
 	return nil

pkg/image/qemu_test.go

@@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
-    http://www.apache.org/licenses/LICENSE-2.0
+	http://www.apache.org/licenses/LICENSE-2.0
 
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
@@ -301,7 +301,7 @@ var _ = Describe("Create blank image", func() {
 		size := convertQuantityToQemuSize(quantity)
 		replaceExecFunction(mockExecFunction("", "", nil, "create", "-f", "raw", "image", size), func() {
 			err = CreateBlankImage("image", quantity, false)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("no such file or directory"))
 		})
 	})
 
@@ -322,7 +322,7 @@ var _ = Describe("Create blank image", func() {
 		size := convertQuantityToQemuSize(quantity)
 		replaceExecFunction(mockExecFunctionStrict("", "", nil, "create", "-f", "raw", "image", size, "-o", "preallocation=falloc"), func() {
 			err = CreateBlankImage("image", quantity, true)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("no such file or directory"))
 		})
 	})
 
@@ -332,7 +332,7 @@ var _ = Describe("Create blank image", func() {
 		size := convertQuantityToQemuSize(quantity)
 		replaceExecFunction(mockExecFunctionStrict("", "", nil, "create", "-f", "raw", "image", size), func() {
 			err = CreateBlankImage("image", quantity, false)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("no such file or directory"))
 		})
 	})
 })