Merge pull request #849 from vardhaman22/add-conlict-error-debug-log

[main] added a debug log for conflict error and unit test for it

controller/aks-cluster-config-handler.go

@@ -189,6 +189,14 @@ func (h *Handler) recordError(onChange func(key string, config *aksv1.AKSCluster
 			return config, err
 		}
 		if err != nil {
+			if apierrors.IsConflict(err) {
+				// conflict error means the config is updated by rancher controller
+				// the changes which needs to be done by the operator controller will be handled in next
+				// reconcile call
+				logrus.Debugf("Error updating aksclusterconfig: %s", err.Error())
+				return config, err
+			}
+
 			message = err.Error()
 		}
 

controller/aks-cluster-config-handler_test.go

@@ -1,7 +1,9 @@
 package controller
 
 import (
+	"bytes"
 	"errors"
+	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
@@ -15,6 +17,7 @@ import (
 	"github.com/rancher/aks-operator/pkg/test"
 	"github.com/rancher/aks-operator/pkg/utils"
 	"github.com/rancher/wrangler/v3/pkg/generated/controllers/core"
+	"github.com/sirupsen/logrus"
 	"go.uber.org/mock/gomock"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -975,3 +978,78 @@ var _ = Describe("buildUpstreamClusterState", func() {
 		Expect(upstreamSpec.LogAnalyticsWorkspaceGroup).To(BeNil())
 	})
 })
+
+var _ = Describe("recordError", func() {
+	var (
+		aksConfig *aksv1.AKSClusterConfig
+		handler   *Handler
+	)
+
+	BeforeEach(func() {
+		aksConfig = &aksv1.AKSClusterConfig{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "testrecorderror",
+				Namespace: "default",
+			},
+			Spec: aksv1.AKSClusterConfigSpec{
+				ResourceGroup: "test",
+				ClusterName:   "test",
+			},
+		}
+
+		Expect(cl.Create(ctx, aksConfig)).To(Succeed())
+	})
+
+	It("should return same conflict error when onChange returns a conflict error", func() {
+		oldOutput := logrus.StandardLogger().Out
+		buf := bytes.Buffer{}
+		logrus.SetOutput(&buf)
+
+		aksConfigUpdated := aksConfig.DeepCopy()
+		Expect(cl.Update(ctx, aksConfigUpdated)).To(Succeed())
+
+		var expectedErr error
+		expectedConfig := &aksv1.AKSClusterConfig{}
+		onChange := func(key string, config *aksv1.AKSClusterConfig) (*aksv1.AKSClusterConfig, error) {
+			expectedErr = cl.Update(ctx, config)
+			return expectedConfig, expectedErr
+		}
+
+		aksConfig.ResourceVersion = "1"
+		handleFunction := handler.recordError(onChange)
+		config, err := handleFunction("", aksConfig)
+
+		Expect(config).To(Equal(expectedConfig))
+		Expect(err).To(Equal(expectedErr))
+		Expect("").To(Equal(string(buf.Bytes())))
+		logrus.SetOutput(oldOutput)
+	})
+
+	It("should return same conflict error when onChange returns a conflict error and print a debug log for the error", func() {
+		oldOutput := logrus.StandardLogger().Out
+		buf := bytes.Buffer{}
+		logrus.SetOutput(&buf)
+		logrus.SetLevel(logrus.DebugLevel)
+
+		aksConfigUpdated := aksConfig.DeepCopy()
+		Expect(cl.Update(ctx, aksConfigUpdated)).To(Succeed())
+
+		var expectedErr error
+		expectedConfig := &aksv1.AKSClusterConfig{}
+		onChange := func(key string, config *aksv1.AKSClusterConfig) (*aksv1.AKSClusterConfig, error) {
+			expectedErr = cl.Update(ctx, config)
+			return expectedConfig, expectedErr
+		}
+
+		aksConfig.ResourceVersion = "1"
+		handleFunction := handler.recordError(onChange)
+		config, err := handleFunction("", aksConfig)
+
+		Expect(config).To(Equal(expectedConfig))
+		Expect(err).To(MatchError(expectedErr))
+
+		cleanLogOutput := strings.Replace(string(buf.Bytes()), `\"`, `"`, -1)
+		Expect(strings.Contains(cleanLogOutput, err.Error())).To(BeTrue())
+		logrus.SetOutput(oldOutput)
+	})
+})