[bump_20.10 backport ]Fix panic in agent

[thaJeztah]

backport of #3024 for 20.10 (already back ported to 19.03 through #3031)

Previously, there was an issue where the agent could panic while
attempting to determine if an error was temporary.

Before the change, in agent/exec/errors.go, the function IsTemporary
attempted to drill down to a root cause by iterating through the causes
of an error by calling errors.Cause. If an error has no cause, then
errors.Cause returns that same error.

The issue is that somewhere in the depths of some code, it was posssible
for the error to have an underlying type that was non-comparable; for
example, maps and slices are uncomparable types. This would cause a
panic, as the uncomparable type cannot be compared even to itself.

However, one can see that errors.Cause has its own loop, and drills
down to the root cause in its own way. There is no need for us to
iterate here.

Instead, we can just take a look at the error itself, and then take a
look at its cause once. If neither is temporary, the error is not
temporary, and we have nothing to worry about.

Signed-off-by: Drew Erny [email protected]
(cherry picked from commit 39a4233)
Signed-off-by: Sebastiaan van Stijn [email protected]

- What I did

- How I did it

- How to test it

- Description for the changelog

[thaJeztah]

@dperny PTAL

[codecov]

Codecov Report

Merging #3032 (2a87c2b) into bump_20.10 (286f457) will increase coverage by 0.01%.
The diff coverage is 20.00%.

@@              Coverage Diff               @@
##           bump_20.10    #3032      +/-   ##
==============================================
+ Coverage       61.89%   61.91%   +0.01%     
==============================================
  Files             142      142              
  Lines           23031    23029       -2     
==============================================
+ Hits            14256    14258       +2     
+ Misses           7284     7277       -7     
- Partials         1491     1494       +3     

[thaJeztah]

Actually; wondering if we should replace this function to use errors.As(). which should be compatible with both errors.Wrap() (github.com/pkg/errors) and native go 1.13 %w wrapping;

// IsTemporary returns true if the error or a recursive cause returns true for
// temporary.
func IsTemporary(err error) bool {
	var tmp Temporary
	if errors.As(err, &tmp) {
		return tmp.Temporary()
	}
	return false
}

See https://play.golang.org/p/tG1klGjjS8R

package main

import (
	"fmt"

	"github.com/pkg/errors"
)

func main() {
	err := fmt.Errorf("hello")
	err1 := temporary{fmt.Errorf("hello1: %w", err)}
	err2 := errors.Wrap(err1, "hello2")
	err3 := errors.Wrap(err2, "hello3")
	err4 := fmt.Errorf("hello4: %w", err3)
	err5 := fmt.Errorf("hello5: %w", err4)

	yeah := IsTemporary(nil)
	fmt.Println(yeah)

	yeah = IsTemporary(err)
	fmt.Println(yeah)

	yeah = IsTemporary(err5)
	fmt.Println(yeah)
}

// Temporary indicates whether or not the error condition is temporary.
//
// If this is encountered in the controller, the failing operation will be
// retried when this returns true. Otherwise, the operation is considered
// fatal.
type Temporary interface {
	Temporary() bool
}

type temporary struct {
	error
}

func (t temporary) Cause() error    { return t.error }
func (t temporary) Temporary() bool { return true }

// IsTemporary returns true if the error or a recursive cause returns true for
// temporary.
func IsTemporary(err error) bool {
	var tmp Temporary
	if errors.As(err, &tmp) {
		return tmp.Temporary()
	}
	return false
}