Repository: scylladb/scylla-operator
Pull request #356 (2021-02-01 13:36:10)

integration tests stability fixes
Image parsing tests were relying on global value, and they were failing when any integration tests failed. 
Two replace integration tests were flaky.


Patch:
diff --git a/config/operator/operator/operator.yaml b/config/operator/operator/operator.yaml
index c2c7dffb1..c942541b9 100644
--- a/config/operator/operator/operator.yaml
+++ b/config/operator/operator/operator.yaml
@@ -41,9 +41,6 @@ spec:
         imagePullPolicy: IfNotPresent
         name: manager
         resources:
-          limits:
-            cpu: 100m
-            memory: 30Mi
           requests:
             cpu: 100m
             memory: 20Mi
diff --git a/examples/common/operator.yaml b/examples/common/operator.yaml
index 191de0ed9..4dcb733c9 100644
--- a/examples/common/operator.yaml
+++ b/examples/common/operator.yaml
@@ -1940,9 +1940,6 @@ spec:
           initialDelaySeconds: 5
           periodSeconds: 10
         resources:
-          limits:
-            cpu: 100m
-            memory: 30Mi
           requests:
             cpu: 100m
             memory: 20Mi
diff --git a/pkg/cmd/options/common.go b/pkg/cmd/options/common.go
index cb59824c7..2bbf69576 100644
--- a/pkg/cmd/options/common.go
+++ b/pkg/cmd/options/common.go
@@ -11,19 +11,19 @@ import (
 )
 
 // Singleton
-var commonOpts = &commonOptions{}
+var commonOpts = &CommonOptions{}
 
-type commonOptions struct {
+type CommonOptions struct {
 	Name      string
 	Namespace string
 	LogLevel  string
 }
 
-func GetCommonOptions() *commonOptions {
+func GetCommonOptions() *CommonOptions {
 	return commonOpts
 }
 
-func (o *commonOptions) AddFlags(cmd *cobra.Command) {
+func (o *CommonOptions) AddFlags(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&o.Name, "pod-name", os.Getenv(naming.EnvVarEnvVarPodName), "name of the pod")
 	cmd.Flags().StringVar(&o.Namespace, "pod-namespace", os.Getenv(naming.EnvVarPodNamespace), "namespace of the pod")
 	cmd.Flags().StringVar(&o.LogLevel,
@@ -34,7 +34,7 @@ func (o *commonOptions) AddFlags(cmd *cobra.Command) {
 
 }
 
-func (o *commonOptions) Validate() error {
+func (o *CommonOptions) Validate() error {
 	if o.Name == "" {
 		return errors.New("pod-name not set")
 	}
diff --git a/pkg/cmd/options/manager.go b/pkg/cmd/options/manager.go
index 9c8a7a085..834789368 100644
--- a/pkg/cmd/options/manager.go
+++ b/pkg/cmd/options/manager.go
@@ -6,11 +6,11 @@ import (
 
 // Singleton
 var managerOpts = &managerOptions{
-	commonOptions: GetCommonOptions(),
+	CommonOptions: GetCommonOptions(),
 }
 
 type managerOptions struct {
-	*commonOptions
+	*CommonOptions
 }
 
 func GetManagerOptions() *managerOptions {
@@ -18,7 +18,7 @@ func GetManagerOptions() *managerOptions {
 }
 
 func (o *managerOptions) AddFlags(cmd *cobra.Command) {
-	o.commonOptions.AddFlags(cmd)
+	o.CommonOptions.AddFlags(cmd)
 }
 
 func (o *managerOptions) Validate() error {
diff --git a/pkg/cmd/options/operator.go b/pkg/cmd/options/operator.go
index 22eeccda4..3d2578eca 100644
--- a/pkg/cmd/options/operator.go
+++ b/pkg/cmd/options/operator.go
@@ -6,29 +6,29 @@ import (
 )
 
 // Singleton
-var operatorOpts = &operatorOptions{
-	commonOptions: GetCommonOptions(),
+var operatorOpts = &OperatorOptions{
+	CommonOptions: GetCommonOptions(),
 }
 
-type operatorOptions struct {
-	*commonOptions
+type OperatorOptions struct {
+	*CommonOptions
 	Image                  string
 	EnableAdmissionWebhook bool
 }
 
-func GetOperatorOptions() *operatorOptions {
+func GetOperatorOptions() *OperatorOptions {
 	return operatorOpts
 }
 
-func (o *operatorOptions) AddFlags(cmd *cobra.Command) {
-	o.commonOptions.AddFlags(cmd)
+func (o *OperatorOptions) AddFlags(cmd *cobra.Command) {
+	o.CommonOptions.AddFlags(cmd)
 	cmd.Flags().StringVar(&o.Image, "image", "", "image of the operator used")
 	cmd.Flags().BoolVar(&o.EnableAdmissionWebhook, "enable-admission-webhook", true, "enable the admission webhook")
 }
 
-func (o *operatorOptions) Validate() error {
+func (o *OperatorOptions) Validate() error {
 
-	if o.Image == "" && o.commonOptions.Validate() != nil {
+	if o.Image == "" && o.CommonOptions.Validate() != nil {
 		return errors.New("image not set - you must set either image or namespace and name")
 	}
 	return nil
diff --git a/pkg/cmd/options/sidecar.go b/pkg/cmd/options/sidecar.go
index 196bac9c6..da5560d6c 100644
--- a/pkg/cmd/options/sidecar.go
+++ b/pkg/cmd/options/sidecar.go
@@ -10,11 +10,11 @@ import (
 
 // Singleton
 var sidecarOpts = &sidecarOptions{
-	commonOptions: GetCommonOptions(),
+	CommonOptions: GetCommonOptions(),
 }
 
 type sidecarOptions struct {
-	*commonOptions
+	*CommonOptions
 	CPU string
 }
 
@@ -23,12 +23,12 @@ func GetSidecarOptions() *sidecarOptions {
 }
 
 func (o *sidecarOptions) AddFlags(cmd *cobra.Command) {
-	o.commonOptions.AddFlags(cmd)
+	o.CommonOptions.AddFlags(cmd)
 	cmd.Flags().StringVar(&o.CPU, "cpu", os.Getenv(naming.EnvVarCPU), "number of cpus to use")
 }
 
 func (o *sidecarOptions) Validate() error {
-	if err := o.commonOptions.Validate(); err != nil {
+	if err := o.CommonOptions.Validate(); err != nil {
 		return errors.WithStack(err)
 	}
 	if o.CPU == "" {
diff --git a/pkg/controllers/cluster/cluster_controller.go b/pkg/controllers/cluster/cluster_controller.go
index 061862187..0dd336b22 100644
--- a/pkg/controllers/cluster/cluster_controller.go
+++ b/pkg/controllers/cluster/cluster_controller.go
@@ -73,7 +73,7 @@ func New(ctx context.Context, mgr ctrl.Manager, logger log.Logger) (*ClusterReco
 		return nil, errors.Wrap(err, "get dynamic uncached client")
 	}
 
-	operatorImage, err := getOperatorImage(ctx, kubeClient)
+	operatorImage, err := getOperatorImage(ctx, kubeClient, options.GetOperatorOptions())
 	if err != nil {
 		return nil, errors.Wrap(err, "get operator image")
 	}
@@ -211,8 +211,7 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return nil
 }
 
-func getOperatorImage(ctx context.Context, kubeClient kubernetes.Interface) (string, error) {
-	opts := options.GetOperatorOptions()
+func getOperatorImage(ctx context.Context, kubeClient kubernetes.Interface, opts *options.OperatorOptions) (string, error) {
 	if opts.Image != "" {
 		return opts.Image, nil
 	}


Test patch:
diff --git a/pkg/controllers/cluster/cluster_controller_test.go b/pkg/controllers/cluster/cluster_controller_test.go
index dc1c05887..043e016c2 100644
--- a/pkg/controllers/cluster/cluster_controller_test.go
+++ b/pkg/controllers/cluster/cluster_controller_test.go
@@ -21,9 +21,12 @@ var _ = Describe("Cluster controller", func() {
 			namespace = "ns"
 			name      = "name"
 		)
-		opts := options.GetOperatorOptions()
-		opts.Namespace = namespace
-		opts.Name = name
+		opts := &options.OperatorOptions{
+			CommonOptions: &options.CommonOptions{
+				Name:      name,
+				Namespace: namespace,
+			},
+		}
 
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
@@ -38,7 +41,7 @@ var _ = Describe("Cluster controller", func() {
 		}
 
 		kubeClientFake := kubefake.NewSimpleClientset(pod)
-		image, err := getOperatorImage(context.Background(), kubeClientFake)
+		image, err := getOperatorImage(context.Background(), kubeClientFake, opts)
 		if expectedImage == "" {
 			Expect(err).To(HaveOccurred())
 		} else {
diff --git a/pkg/controllers/cluster/controller_integration_test.go b/pkg/controllers/cluster/controller_integration_test.go
index ad44034e6..b2e3882de 100644
--- a/pkg/controllers/cluster/controller_integration_test.go
+++ b/pkg/controllers/cluster/controller_integration_test.go
@@ -6,6 +6,7 @@ package cluster_test
 import (
 	"context"
 	"strings"
+	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -13,6 +14,7 @@ import (
 	"github.com/scylladb/scylla-operator/pkg/naming"
 	"github.com/scylladb/scylla-operator/pkg/test/integration"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -61,7 +63,6 @@ var _ = Describe("Cluster controller", func() {
 
 	It("When PVC affinity is bound to lost node, node is replaced", func() {
 		scylla := singleNodeCluster(ns)
-		scylla.Spec.AutomaticOrphanedNodeCleanup = true
 
 		Expect(testEnv.Create(ctx, scylla)).To(Succeed())
 		defer func() {
@@ -89,6 +90,10 @@ var _ = Describe("Cluster controller", func() {
 			Expect(sstStub.CreatePVCs(ctx, scylla, pvOption)).To(Succeed())
 		}
 
+		Expect(testEnv.Refresh(ctx, scylla)).To(Succeed())
+		scylla.Spec.AutomaticOrphanedNodeCleanup = true
+		Expect(testEnv.Update(ctx, scylla)).To(Succeed())
+
 		services, err := rackMemberService(ns.Namespace, rack, scylla)
 		Expect(err).To(BeNil())
 		Expect(services).To(Not(BeEmpty()))
@@ -131,6 +136,7 @@ var _ = Describe("Cluster controller", func() {
 		})
 
 		It("replace non seed node", func() {
+			const shortWait = 2 * time.Second
 			rack := scylla.Spec.Datacenter.Racks[0]
 
 			services, err := nonSeedServices(ns.Namespace, rack, scylla)
@@ -152,19 +158,27 @@ var _ = Describe("Cluster controller", func() {
 				return scylla.Status.Racks[rack.Name].ReplaceAddressFirstBoot, nil
 			}).Should(HaveKeyWithValue(serviceToReplace.Name, replacedServiceIP))
 
+			By("Old PVC should be removed")
+			Eventually(func() []corev1.PersistentVolumeClaim {
+				pvcs := &corev1.PersistentVolumeClaimList{}
+
+				Expect(testEnv.List(ctx, pvcs, &client.ListOptions{
+					LabelSelector: naming.RackSelector(rack, scylla),
+				})).To(Succeed())
+
+				return pvcs.Items
+			}, shortWait).Should(HaveLen(int(rack.Members - 1)))
+
 			By("Old Pod should be removed")
-			Eventually(func() (bool, error) {
-				if err := sstStub.SyncStatus(ctx, scylla); err != nil {
-					return false, err
-				}
+			Eventually(func() []corev1.Pod {
+				pods := &corev1.PodList{}
 
-				if err := testEnv.Refresh(ctx, scylla); err != nil {
-					return false, err
-				}
-				readyMembers := scylla.Status.Racks[rack.Name].ReadyMembers
-				expectedMembers := scylla.Spec.Datacenter.Racks[0].Members
-				return readyMembers == expectedMembers-1, nil
-			}).Should(BeTrue())
+				Expect(testEnv.List(ctx, pods, &client.ListOptions{
+					LabelSelector: naming.RackSelector(rack, scylla),
+				})).To(Succeed())
+
+				return pods.Items
+			}, shortWait).Should(HaveLen(int(rack.Members - 1)))
 
 			By("When new pod is scheduled")
 			Expect(sstStub.CreatePods(ctx, scylla)).To(Succeed())
@@ -181,11 +195,14 @@ var _ = Describe("Cluster controller", func() {
 					Name:      serviceToReplace.Name,
 				}
 				if err := testEnv.Get(ctx, key, service); err != nil {
+					if apierrors.IsNotFound(err) {
+						return nil, nil
+					}
 					return nil, err
 				}
 
 				return service.Labels, nil
-			}).Should(HaveKeyWithValue(naming.ReplaceLabel, replacedServiceIP))
+			}, shortWait).Should(HaveKeyWithValue(naming.ReplaceLabel, replacedServiceIP))
 		})
 
 		It("replace seed node", func() {
diff --git a/pkg/test/integration/statefulset.go b/pkg/test/integration/statefulset.go
index 8eac9d82c..52e440ded 100644
--- a/pkg/test/integration/statefulset.go
+++ b/pkg/test/integration/statefulset.go
@@ -208,6 +208,7 @@ func (s *StatefulSetOperatorStub) CreatePVCs(ctx context.Context, cluster *scyll
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      naming.PVCNameForPod(pod.Name),
 					Namespace: pod.Namespace,
+					Labels:    naming.RackLabels(rack, cluster),
 				},
 				Spec: corev1.PersistentVolumeClaimSpec{
 					VolumeName:  pv.Name,
@@ -218,7 +219,7 @@ func (s *StatefulSetOperatorStub) CreatePVCs(ctx context.Context, cluster *scyll
 					},
 				},
 			}
-
+			s.logger.Info(ctx, "Creating/updating PVC", "pvc", pvc.Name, "pod", pod.Name)
 			if _, err := controllerutil.CreateOrUpdate(ctx, s.env, pvc, func() error { return nil }); err != nil {
 				return err
 			}