diff --git a/controller/eks-cluster-config-handler.go b/controller/eks-cluster-config-handler.go index 2a5af07d..c9c8c434 100644 --- a/controller/eks-cluster-config-handler.go +++ b/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) } diff --git a/controller/eks-cluster-config-handler_test.go b/controller/eks-cluster-config-handler_test.go index 63a2b182..f334aa57 100644 --- a/controller/eks-cluster-config-handler_test.go +++ b/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) + }) +})