Merge pull request #1131 from vardhaman22/handle-conflict

[main] handle conflict errors while updating eksClusterConfig

controller/eks-cluster-config-handler.go

@@ -115,7 +115,13 @@ func (h *Handler) recordError(onChange func(key string, config *eksv1.EKSCluster
 			return config, err
 		}
 		if err != nil {
-			if !strings.Contains(err.Error(), "currently has update") {
+			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 eksclusterconfig: %s", err.Error())
+				return config, err
+			} else if !strings.Contains(err.Error(), "currently has update") {
 				// The update is valid in that the controller should retry but there is no actionable resolution as far
 				// as a user is concerned. An update has either been initiated by the eks-operator or another source
 				// is already in progress. It is possible an update is not being immediately reflected in the upstream
@@ -1063,6 +1069,7 @@ func (h *Handler) importCluster(ctx context.Context, config *eksv1.EKSClusterCon
 	config.Status.Subnets = clusterState.Cluster.ResourcesVpcConfig.SubnetIds
 	config.Status.SecurityGroups = clusterState.Cluster.ResourcesVpcConfig.SecurityGroupIds
 	config.Status.Phase = eksConfigActivePhase
+
 	return h.eksCC.UpdateStatus(config)
 }
 

controller/eks-cluster-config-handler_test.go

@@ -1,7 +1,9 @@
 package controller
 
 import (
+	"bytes"
 	"errors"
+	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 
@@ -12,6 +14,7 @@ import (
 	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/sirupsen/logrus"
 
 	eksv1 "github.com/rancher/eks-operator/pkg/apis/eks.cattle.io/v1"
 	"github.com/rancher/eks-operator/pkg/eks"
@@ -221,3 +224,86 @@ var _ = Describe("updateCluster", func() {
 			"the node group version may only be up to three minor versions older than the cluster version"))
 	})
 })
+
+var _ = Describe("recordError", func() {
+	var (
+		eksConfig *eksv1.EKSClusterConfig
+		handler   *Handler
+	)
+
+	BeforeEach(func() {
+		eksConfig = &eksv1.EKSClusterConfig{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "testrecorderror",
+				Namespace: "default",
+			},
+			Spec: eksv1.EKSClusterConfigSpec{
+				DisplayName:         "test",
+				Region:              "test",
+				PrivateAccess:       aws.Bool(true),
+				PublicAccess:        aws.Bool(true),
+				PublicAccessSources: []string{"test"},
+				Tags:                map[string]string{"test": "test"},
+				LoggingTypes:        []string{"test"},
+				KubernetesVersion:   aws.String("test"),
+				SecretsEncryption:   aws.Bool(true),
+				KmsKey:              aws.String("test"),
+			},
+		}
+
+		Expect(cl.Create(ctx, eksConfig)).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)
+
+		eksConfigUpdated := eksConfig.DeepCopy()
+		Expect(cl.Update(ctx, eksConfigUpdated)).To(Succeed())
+
+		var expectedErr error
+		expectedConfig := &eksv1.EKSClusterConfig{}
+		onChange := func(key string, config *eksv1.EKSClusterConfig) (*eksv1.EKSClusterConfig, error) {
+			expectedErr = cl.Update(ctx, config)
+			return expectedConfig, expectedErr
+		}
+
+		eksConfig.ResourceVersion = "1"
+		handleFunction := handler.recordError(onChange)
+		config, err := handleFunction("", eksConfig)
+
+		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)
+
+		eksConfigUpdated := eksConfig.DeepCopy()
+		Expect(cl.Update(ctx, eksConfigUpdated)).To(Succeed())
+
+		var expectedErr error
+		expectedConfig := &eksv1.EKSClusterConfig{}
+		onChange := func(key string, config *eksv1.EKSClusterConfig) (*eksv1.EKSClusterConfig, error) {
+			expectedErr = cl.Update(ctx, config)
+			return expectedConfig, expectedErr
+		}
+
+		eksConfig.ResourceVersion = "1"
+		handleFunction := handler.recordError(onChange)
+		config, err := handleFunction("", eksConfig)
+
+		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)
+	})
+})