From 5b9a383788ef0723ca12eb86c688cdf3b157277d Mon Sep 17 00:00:00 2001
From: Rishabh Patel <66425093+rishabh-11@users.noreply.github.com>
Date: Mon, 18 Sep 2023 10:22:30 +0530
Subject: [PATCH] Introduced `errorCode` field in `LastOperation` struct (#851)
* add errorCode field in LastOperation struct
* address review comments
* address review comments part-2
---
docs/documents/apis.md | 14 ++++
...achine.sapcloud.io_machinedeployments.yaml | 3 +
.../crds/machine.sapcloud.io_machines.yaml | 3 +
.../crds/machine.sapcloud.io_machinesets.yaml | 6 ++
pkg/apis/machine/types.go | 4 +
pkg/apis/machine/v1alpha1/machine_types.go | 4 +
.../v1alpha1/zz_generated.conversion.go | 2 +
pkg/openapi/openapi_generated.go | 7 ++
.../provider/machinecodes/status/status.go | 47 ++++++-----
.../machinecontroller/machine_test.go | 77 ++++++++++++++++---
.../machinecontroller/machine_util.go | 4 +-
11 files changed, 143 insertions(+), 28 deletions(-)
diff --git a/docs/documents/apis.md b/docs/documents/apis.md
index 393539385..6ea44ddc0 100644
--- a/docs/documents/apis.md
+++ b/docs/documents/apis.md
@@ -871,6 +871,20 @@ string
+
lastUpdateTime
|
diff --git a/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml b/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml
index efa51054d..d7196ff17 100644
--- a/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml
+++ b/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml
@@ -431,6 +431,9 @@ spec:
description:
description: Description of the current operation
type: string
+ errorCode:
+ description: ErrorCode of the current operation if any
+ type: string
lastUpdateTime:
description: Last update time of current operation
format: date-time
diff --git a/kubernetes/crds/machine.sapcloud.io_machines.yaml b/kubernetes/crds/machine.sapcloud.io_machines.yaml
index 651b2b9cf..1b800af25 100644
--- a/kubernetes/crds/machine.sapcloud.io_machines.yaml
+++ b/kubernetes/crds/machine.sapcloud.io_machines.yaml
@@ -263,6 +263,9 @@ spec:
description:
description: Description of the current operation
type: string
+ errorCode:
+ description: ErrorCode of the current operation if any
+ type: string
lastUpdateTime:
description: Last update time of current operation
format: date-time
diff --git a/kubernetes/crds/machine.sapcloud.io_machinesets.yaml b/kubernetes/crds/machine.sapcloud.io_machinesets.yaml
index 4f362a048..3611e1276 100644
--- a/kubernetes/crds/machine.sapcloud.io_machinesets.yaml
+++ b/kubernetes/crds/machine.sapcloud.io_machinesets.yaml
@@ -313,6 +313,9 @@ spec:
description:
description: Description of the current operation
type: string
+ errorCode:
+ description: ErrorCode of the current operation if any
+ type: string
lastUpdateTime:
description: Last update time of current operation
format: date-time
@@ -347,6 +350,9 @@ spec:
description:
description: Description of the current operation
type: string
+ errorCode:
+ description: ErrorCode of the current operation if any
+ type: string
lastUpdateTime:
description: Last update time of current operation
format: date-time
diff --git a/pkg/apis/machine/types.go b/pkg/apis/machine/types.go
index 44a01719d..ab83b4226 100644
--- a/pkg/apis/machine/types.go
+++ b/pkg/apis/machine/types.go
@@ -187,6 +187,10 @@ type LastOperation struct {
// Description of the current operation
Description string
+ // ErrorCode of the current operation if any
+ // +optional
+ ErrorCode string
+
// Last update time of current operation
LastUpdateTime metav1.Time
diff --git a/pkg/apis/machine/v1alpha1/machine_types.go b/pkg/apis/machine/v1alpha1/machine_types.go
index 1051f31de..0d84e76df 100644
--- a/pkg/apis/machine/v1alpha1/machine_types.go
+++ b/pkg/apis/machine/v1alpha1/machine_types.go
@@ -122,6 +122,10 @@ type LastOperation struct {
// Description of the current operation
Description string `json:"description,omitempty"`
+ // ErrorCode of the current operation if any
+ // +optional
+ ErrorCode string `json:"errorCode,omitempty"`
+
// Last update time of current operation
LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty"`
diff --git a/pkg/apis/machine/v1alpha1/zz_generated.conversion.go b/pkg/apis/machine/v1alpha1/zz_generated.conversion.go
index 5b027828d..d912fa928 100644
--- a/pkg/apis/machine/v1alpha1/zz_generated.conversion.go
+++ b/pkg/apis/machine/v1alpha1/zz_generated.conversion.go
@@ -362,6 +362,7 @@ func Convert_machine_CurrentStatus_To_v1alpha1_CurrentStatus(in *machine.Current
func autoConvert_v1alpha1_LastOperation_To_machine_LastOperation(in *LastOperation, out *machine.LastOperation, s conversion.Scope) error {
out.Description = in.Description
+ out.ErrorCode = in.ErrorCode
out.LastUpdateTime = in.LastUpdateTime
out.State = machine.MachineState(in.State)
out.Type = machine.MachineOperationType(in.Type)
@@ -375,6 +376,7 @@ func Convert_v1alpha1_LastOperation_To_machine_LastOperation(in *LastOperation,
func autoConvert_machine_LastOperation_To_v1alpha1_LastOperation(in *machine.LastOperation, out *LastOperation, s conversion.Scope) error {
out.Description = in.Description
+ out.ErrorCode = in.ErrorCode
out.LastUpdateTime = in.LastUpdateTime
out.State = MachineState(in.State)
out.Type = MachineOperationType(in.Type)
diff --git a/pkg/openapi/openapi_generated.go b/pkg/openapi/openapi_generated.go
index 0667336c2..fab3606a3 100644
--- a/pkg/openapi/openapi_generated.go
+++ b/pkg/openapi/openapi_generated.go
@@ -413,6 +413,13 @@ func schema_pkg_apis_machine_v1alpha1_LastOperation(ref common.ReferenceCallback
Format: "",
},
},
+ "errorCode": {
+ SchemaProps: spec.SchemaProps{
+ Description: "ErrorCode of the current operation if any",
+ Type: []string{"string"},
+ Format: "",
+ },
+ },
"lastUpdateTime": {
SchemaProps: spec.SchemaProps{
Description: "Last update time of current operation",
diff --git a/pkg/util/provider/machinecodes/status/status.go b/pkg/util/provider/machinecodes/status/status.go
index 33cbda2d1..98a644c39 100644
--- a/pkg/util/provider/machinecodes/status/status.go
+++ b/pkg/util/provider/machinecodes/status/status.go
@@ -32,9 +32,6 @@ package status
import (
"fmt"
- "regexp"
- "strings"
-
"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
)
@@ -105,7 +102,7 @@ func FromError(err error) (s *Status, ok bool) {
return nil, true
}
- if matches, errInFind := findInString(err.Error()); errInFind == nil {
+ if matches, errInFind := findCodeAndMessage(err.Error()); errInFind == nil {
code := codes.StringToCode(matches[0])
return &Status{
code: int32(code),
@@ -120,21 +117,35 @@ func FromError(err error) (s *Status, ok bool) {
}, false
}
-// findInString need to check if this logic can be optimized
-func findInString(input string) ([]string, error) {
- var matches []string
-
- re := regexp.MustCompile(`\[([^\[\]]*)\]`)
- submatchall := re.FindAllString(input, -1)
- if submatchall == nil || len(submatchall) != 2 {
- return nil, fmt.Errorf("Unable to decode for machine code error")
+func findCodeAndMessage(encodedMsg string) ([]string, error) {
+ var decoded []string
+ var temp []rune
+ counter := 0
+
+ for _, char := range encodedMsg {
+ switch char {
+ case '[':
+ counter++
+ temp = append(temp, char)
+ case ']':
+ if counter > 0 {
+ counter--
+ temp = append(temp, char)
+ if counter == 0 {
+ tempString := string(temp)
+ decoded = append(decoded, tempString[1:len(tempString)-1])
+ temp = nil
+ }
+ }
+ default:
+ if counter > 0 {
+ temp = append(temp, char)
+ }
+ }
}
- for _, element := range submatchall {
- element = strings.Trim(element, "[")
- element = strings.Trim(element, "]")
- matches = append(matches, element)
+ if len(decoded) != 2 {
+ return nil, fmt.Errorf("unable to decode for machine code error")
}
-
- return matches, nil
+ return decoded, nil
}
diff --git a/pkg/util/provider/machinecontroller/machine_test.go b/pkg/util/provider/machinecontroller/machine_test.go
index 9204628cb..dcb2b718f 100644
--- a/pkg/util/provider/machinecontroller/machine_test.go
+++ b/pkg/util/provider/machinecontroller/machine_test.go
@@ -503,6 +503,11 @@ var _ = Describe("machine", func() {
} else {
Expect(actual.Labels).To(Equal(data.expect.machine.Labels))
}
+ if data.expect.machine.Status.LastOperation.ErrorCode != "" {
+ Expect(actual.Status.LastOperation.ErrorCode).To(Equal(data.expect.machine.Status.LastOperation.ErrorCode))
+ } else {
+ Expect(actual.Status.LastOperation.ErrorCode).To(Equal(""))
+ }
},
Entry("Machine creation succeeds with object UPDATE", &data{
@@ -728,7 +733,7 @@ var _ = Describe("machine", func() {
retry: machineutils.LongRetry,
},
}),
- Entry("Machine creation fails with CrashLoopBackOff", &data{
+ Entry("Machine creation fails with CrashLoopBackOff due to Internal error", &data{
setup: setup{
secrets: []*corev1.Secret{
{
@@ -755,10 +760,8 @@ var _ = Describe("machine", func() {
action: action{
machine: "machine-0",
fakeDriver: &driver.FakeDriver{
- VMExists: false,
- ProviderID: "fakeID-0",
- NodeName: "fakeNode-0",
- Err: status.Error(codes.Internal, "Provider is returning error on create call"),
+ VMExists: false,
+ Err: status.Error(codes.Internal, "Provider is returning error on create call"),
},
},
expect: expect{
@@ -774,6 +777,61 @@ var _ = Describe("machine", func() {
CurrentStatus: v1alpha1.CurrentStatus{
Phase: v1alpha1.MachineCrashLoopBackOff,
},
+ LastOperation: v1alpha1.LastOperation{
+ ErrorCode: codes.Internal.String(),
+ },
+ }, nil, nil, nil, true, metav1.Now()),
+ err: nil,
+ retry: machineutils.MediumRetry,
+ },
+ }),
+ Entry("Machine creation fails with CrashLoopBackOff due to resource exhaustion", &data{
+ setup: setup{
+ secrets: []*corev1.Secret{
+ {
+ ObjectMeta: *newObjectMeta(objMeta, 0),
+ Data: map[string][]byte{"userData": []byte("test")},
+ },
+ },
+ machineClasses: []*v1alpha1.MachineClass{
+ {
+ ObjectMeta: *newObjectMeta(objMeta, 0),
+ SecretRef: newSecretReference(objMeta, 0),
+ },
+ },
+ machines: newMachines(1, &v1alpha1.MachineTemplateSpec{
+ ObjectMeta: *newObjectMeta(objMeta, 0),
+ Spec: v1alpha1.MachineSpec{
+ Class: v1alpha1.ClassSpec{
+ Kind: "MachineClass",
+ Name: "machine-0",
+ },
+ },
+ }, nil, nil, nil, nil, true, metav1.Now()),
+ },
+ action: action{
+ machine: "machine-0",
+ fakeDriver: &driver.FakeDriver{
+ VMExists: false,
+ Err: status.Error(codes.ResourceExhausted, "Provider does not have capacity to create VM"),
+ },
+ },
+ expect: expect{
+ machine: newMachine(&v1alpha1.MachineTemplateSpec{
+ ObjectMeta: *newObjectMeta(objMeta, 0),
+ Spec: v1alpha1.MachineSpec{
+ Class: v1alpha1.ClassSpec{
+ Kind: "MachineClass",
+ Name: "machineClass",
+ },
+ },
+ }, &v1alpha1.MachineStatus{
+ CurrentStatus: v1alpha1.CurrentStatus{
+ Phase: v1alpha1.MachineCrashLoopBackOff,
+ },
+ LastOperation: v1alpha1.LastOperation{
+ ErrorCode: codes.ResourceExhausted.String(),
+ },
}, nil, nil, nil, true, metav1.Now()),
err: nil,
retry: machineutils.MediumRetry,
@@ -806,10 +864,8 @@ var _ = Describe("machine", func() {
action: action{
machine: "machine-0",
fakeDriver: &driver.FakeDriver{
- VMExists: false,
- ProviderID: "fakeID-0",
- NodeName: "fakeNode-0",
- Err: status.Error(codes.Internal, "Provider is returning error on create call"),
+ VMExists: false,
+ Err: status.Error(codes.Internal, "Provider is returning error on create call"),
},
},
expect: expect{
@@ -825,6 +881,9 @@ var _ = Describe("machine", func() {
CurrentStatus: v1alpha1.CurrentStatus{
Phase: v1alpha1.MachineFailed,
},
+ LastOperation: v1alpha1.LastOperation{
+ ErrorCode: codes.Internal.String(),
+ },
}, nil, nil, nil, true, metav1.NewTime(metav1.Now().Add(-time.Hour))),
err: nil,
retry: machineutils.MediumRetry,
diff --git a/pkg/util/provider/machinecontroller/machine_util.go b/pkg/util/provider/machinecontroller/machine_util.go
index 0fb700da7..e09d64d6c 100644
--- a/pkg/util/provider/machinecontroller/machine_util.go
+++ b/pkg/util/provider/machinecontroller/machine_util.go
@@ -479,7 +479,8 @@ func (c *controller) machineCreateErrorHandler(ctx context.Context, machine *v1a
retryRequired = machineutils.MediumRetry
lastKnownState string
)
- if machineErr, ok := status.FromError(err); ok {
+ machineErr, ok := status.FromError(err)
+ if ok {
switch machineErr.Code() {
case codes.Unknown, codes.DeadlineExceeded, codes.Aborted, codes.Unavailable:
retryRequired = machineutils.ShortRetry
@@ -495,6 +496,7 @@ func (c *controller) machineCreateErrorHandler(ctx context.Context, machine *v1a
machine,
v1alpha1.LastOperation{
Description: "Cloud provider message - " + err.Error(),
+ ErrorCode: machineErr.Code().String(),
State: v1alpha1.MachineStateFailed,
Type: v1alpha1.MachineOperationCreate,
LastUpdateTime: metav1.Now(),
|