From 117beff05c4507112c6f4aadbff055994306fa43 Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Mon, 8 May 2023 12:04:29 +0200
Subject: vm: separate boot time and infrastructure errors

It's not correct to mix them since they point to fundamentally different
issues:
1) Boot time errors are caused by a problematic kernel image and can
   only be resolved by using another kernel version or config.
2) Infrastructure errors are temporary, so we can just try again some
   time later.

Reserve the existing BootError for (1) errors and let all other VM
handling errors refer to (2).

To make it possible to attach more output to the infra error, introduce
the VerboseInfraError type.
---
 pkg/instance/instance.go |  7 +++++++
 vm/vm.go                 | 11 ++++++++---
 vm/vmimpl/vmimpl.go      | 17 +++++++++++++++++
 vm/vmm/vmm.go            | 10 ++++++++--
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go
index 882d678d4..73cef6931 100644
--- a/pkg/instance/instance.go
+++ b/pkg/instance/instance.go
@@ -216,6 +216,7 @@ func OverrideVMCount(cfg *mgrconfig.Config, n int) error {
 
 type TestError struct {
 	Boot   bool // says if the error happened during booting or during instance testing
+	Infra  bool // whether the problem is related to some infrastructure problems
 	Title  string
 	Output []byte
 	Report *report.Report
@@ -323,6 +324,12 @@ func (inst *inst) test() EnvTestResult {
 			}
 			testErr.Report = rep
 			testErr.Title = rep.Title
+		} else {
+			testErr.Infra = true
+			if infraErr, ok := err.(vm.InfraErrorer); ok {
+				// In case there's more info available.
+				testErr.Title, testErr.Output = infraErr.InfraError()
+			}
 		}
 		return ret
 	}
diff --git a/vm/vm.go b/vm/vm.go
index 033e297db..e1f9cda60 100644
--- a/vm/vm.go
+++ b/vm/vm.go
@@ -57,15 +57,20 @@ type Instance struct {
 }
 
 var (
-	Shutdown               = vmimpl.Shutdown
-	ErrTimeout             = vmimpl.ErrTimeout
-	_          BootErrorer = vmimpl.BootError{}
+	Shutdown                = vmimpl.Shutdown
+	ErrTimeout              = vmimpl.ErrTimeout
+	_          BootErrorer  = vmimpl.BootError{}
+	_          InfraErrorer = vmimpl.InfraError{}
 )
 
 type BootErrorer interface {
 	BootError() (string, []byte)
 }
 
+type InfraErrorer interface {
+	InfraError() (string, []byte)
+}
+
 // vmType splits the VM type from any suffix (separated by ":"). This is mostly
 // useful for the "proxyapp" type, where pkg/build needs to specify/handle
 // sub-types.
diff --git a/vm/vmimpl/vmimpl.go b/vm/vmimpl/vmimpl.go
index 10de2ffed..9dbd61a92 100644
--- a/vm/vmimpl/vmimpl.go
+++ b/vm/vmimpl/vmimpl.go
@@ -85,6 +85,8 @@ type Env struct {
 }
 
 // BootError is returned by Pool.Create when VM does not boot.
+// It should not be used for VMM intfrastructure errors, i.e. for problems not related
+// to the tested kernel itself.
 type BootError struct {
 	Title  string
 	Output []byte
@@ -107,6 +109,21 @@ func (err BootError) BootError() (string, []byte) {
 	return err.Title, err.Output
 }
 
+// By default, all Pool.Create() errors are related to infrastructure problems.
+// InfraError is to be used when we want to also attach output to the title.
+type InfraError struct {
+	Title  string
+	Output []byte
+}
+
+func (err InfraError) Error() string {
+	return fmt.Sprintf("%v\n%s", err.Title, err.Output)
+}
+
+func (err InfraError) InfraError() (string, []byte) {
+	return err.Title, err.Output
+}
+
 // Register registers a new VM type within the package.
 func Register(typ string, ctor ctorFunc, allowsOvercommit bool) {
 	Types[typ] = Type{
diff --git a/vm/vmm/vmm.go b/vm/vmm/vmm.go
index 3cee24d53..1f9b911f7 100644
--- a/vm/vmm/vmm.go
+++ b/vm/vmm/vmm.go
@@ -201,11 +201,17 @@ func (inst *instance) lookupSSHAddress() (string, error) {
 	}
 	lines := strings.Split(out, "\n")
 	if len(lines) < 2 {
-		return "", vmimpl.BootError{Title: "Unexpected vmctl status output", Output: []byte(out)}
+		return "", vmimpl.InfraError{
+			Title:  "unexpected vmctl status output",
+			Output: []byte(out),
+		}
 	}
 	matches := vmctlStatusRegex.FindStringSubmatch(lines[1])
 	if len(matches) < 2 {
-		return "", vmimpl.BootError{Title: "Unexpected vmctl status output", Output: []byte(out)}
+		return "", vmimpl.InfraError{
+			Title:  "unexpected vmctl status output",
+			Output: []byte(out),
+		}
 	}
 	return fmt.Sprintf("100.64.%s.3", matches[1]), nil
 }
-- 
cgit mrf-deployment