From 9b849adf36d9e893dca2dd056b063f72767743c2 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 21 Oct 2024 19:33:03 +0300 Subject: [PATCH 1/6] Rename test script There is nothing about regional DR in the script, it is just a helper to run without a timeout and enabling verbose mode. Signed-off-by: Nir Soffer --- Makefile | 2 +- e2e/{e2e-rdr.sh => run.sh} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename e2e/{e2e-rdr.sh => run.sh} (100%) diff --git a/Makefile b/Makefile index e307f3445..0098717f8 100644 --- a/Makefile +++ b/Makefile @@ -195,7 +195,7 @@ test-ramenctl: ## Run ramenctl tests. $(MAKE) -C ramenctl e2e-rdr: generate manifests ## Run rdr-e2e tests. - cd e2e && ./e2e-rdr.sh + cd e2e && ./run.sh coverage: go tool cover -html=cover.out diff --git a/e2e/e2e-rdr.sh b/e2e/run.sh similarity index 100% rename from e2e/e2e-rdr.sh rename to e2e/run.sh From dace2c1f1c1615ddcb502455f9dcd704833b74d7 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 21 Oct 2024 19:45:32 +0300 Subject: [PATCH 2/6] Rename e2e -configFile to -config It easier to type and more consistent. Nobody uses camelCase in option names. Signed-off-by: Nir Soffer --- e2e/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/main_test.go b/e2e/main_test.go index 5a9be639a..4000a0fbe 100644 --- a/e2e/main_test.go +++ b/e2e/main_test.go @@ -15,7 +15,7 @@ import ( ) func init() { - flag.StringVar(&util.ConfigFile, "configfile", "", "Path to the config file") + flag.StringVar(&util.ConfigFile, "config", "", "Path to the config file") } func TestMain(m *testing.M) { From b54eaeca1025cf16b45109fce8d9861a0a00f68e Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 21 Oct 2024 20:45:24 +0300 Subject: [PATCH 3/6] Add lint-e2e and lint-api targets When working on e2e or api we need a way to run lint quickly without running complex golangci-lint command manually. Signed-off-by: Nir Soffer --- Makefile | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 0098717f8..e9bd192bc 100644 --- a/Makefile +++ b/Makefile @@ -114,16 +114,21 @@ generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and $(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..." -.PHONY: lint -lint: golangci-bin ## Run configured golangci-lint and pre-commit.sh linters against the code. # golangci-lint has a limitation that it doesn't lint subdirectories if # they are a different module. # see https://github.com/golangci/golangci-lint/issues/828 + +.PHONY: lint +lint: golangci-bin lint-e2e lint-api ## Run configured golangci-lint and pre-commit.sh linters against the code. testbin/golangci-lint run ./... --config=./.golangci.yaml - cd api && ../testbin/golangci-lint run ./... --config=../.golangci.yaml - cd e2e && ../testbin/golangci-lint run ./... --config=../.golangci.yaml hack/pre-commit.sh +lint-e2e: golangci-bin ## Run configured golangci-lint for e2e module + cd e2e && ../testbin/golangci-lint run ./... --config=../.golangci.yaml + +lint-api: golangci-bin ## Run configured golangci-lint for api module + cd api && ../testbin/golangci-lint run ./... --config=../.golangci.yaml + .PHONY: create-rdr-env create-rdr-env: drenv-prereqs ## Create a new rdr environment. ./hack/dev-env.sh create From f2ba011bb937c1edc76b915312a37e8ea210756c Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 21 Oct 2024 20:55:38 +0300 Subject: [PATCH 4/6] Handle conflicts when updating placement Fix random test failures if the placement was modified after we got it: actions_test.go:31: Operation cannot be fulfilled on placements.cluster.open-cluster-management.io "appset-deploy-cephfs-busybox": the object has been modified; please apply your changes to the latest version and try again Fixes: #1593 Signed-off-by: Nir Soffer --- e2e/dractions/actions.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/e2e/dractions/actions.go b/e2e/dractions/actions.go index b0f04c80d..c08677996 100644 --- a/e2e/dractions/actions.go +++ b/e2e/dractions/actions.go @@ -11,6 +11,7 @@ import ( "github.com/ramendr/ramen/e2e/deployers" "github.com/ramendr/ramen/e2e/util" "github.com/ramendr/ramen/e2e/workloads" + "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -55,18 +56,22 @@ func EnableProtection(w workloads.Workload, d deployers.Deployer) error { clusterName := placementDecision.Status.Decisions[0].ClusterName util.Ctx.Log.Info("got clusterName " + clusterName + " from " + placementDecisionName) - // move update placement annotation after placement has been handled - // otherwise if we first add ocm disable annotation then it might not - // yet be handled by ocm and thus PlacementSatisfied=false - if placement.Annotations == nil { - placement.Annotations = make(map[string]string) - } + util.Ctx.Log.Info("update placement " + placementName + " annotation") - placement.Annotations[OcmSchedulingDisable] = "true" + err = retry.RetryOnConflict(retry.DefaultBackoff, func() error { + placement, err := getPlacement(util.Ctx.Hub.CtrlClient, namespace, placementName) + if err != nil { + return err + } - util.Ctx.Log.Info("update placement " + placementName + " annotation") + if placement.Annotations == nil { + placement.Annotations = make(map[string]string) + } + placement.Annotations[OcmSchedulingDisable] = "true" - if err = updatePlacement(util.Ctx.Hub.CtrlClient, placement); err != nil { + return updatePlacement(util.Ctx.Hub.CtrlClient, placement) + }) + if err != nil { return err } From ec7c087ae1b8bfc47125f7b3e9bfb24b0855e2ad Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 21 Oct 2024 21:00:33 +0300 Subject: [PATCH 5/6] Simplify waitPlacementDecision It was returning both placement and placementDecsision name which is not helpful since we have a helper to return a placement. Signed-off-by: Nir Soffer --- e2e/dractions/actions.go | 2 +- e2e/dractions/retry.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/e2e/dractions/actions.go b/e2e/dractions/actions.go index c08677996..406fc819a 100644 --- a/e2e/dractions/actions.go +++ b/e2e/dractions/actions.go @@ -43,7 +43,7 @@ func EnableProtection(w workloads.Workload, d deployers.Deployer) error { placementName := name drpcName := name - placement, placementDecisionName, err := waitPlacementDecision(util.Ctx.Hub.CtrlClient, namespace, placementName) + placementDecisionName, err := waitPlacementDecision(util.Ctx.Hub.CtrlClient, namespace, placementName) if err != nil { return err } diff --git a/e2e/dractions/retry.go b/e2e/dractions/retry.go index 751b6c8ca..b85af0be0 100644 --- a/e2e/dractions/retry.go +++ b/e2e/dractions/retry.go @@ -16,23 +16,23 @@ import ( ) // nolint:gocognit -// return placement object, placementDecisionName, error +// return placementDecisionName, error func waitPlacementDecision(client client.Client, namespace string, placementName string, -) (*v1beta1.Placement, string, error) { +) (string, error) { startTime := time.Now() placementDecisionName := "" for { placement, err := getPlacement(client, namespace, placementName) if err != nil { - return nil, "", err + return "", err } for _, cond := range placement.Status.Conditions { if cond.Type == "PlacementSatisfied" && cond.Status == "True" { placementDecisionName = placement.Status.DecisionGroups[0].Decisions[0] if placementDecisionName != "" { - return placement, placementDecisionName, nil + return placementDecisionName, nil } } } @@ -41,11 +41,11 @@ func waitPlacementDecision(client client.Client, namespace string, placementName // so need query placementdecision by label placementDecision, err := getPlacementDecisionFromPlacement(client, placement) if err == nil && placementDecision != nil { - return placement, placementDecision.Name, nil + return placementDecision.Name, nil } if time.Since(startTime) > time.Second*time.Duration(util.Timeout) { - return nil, "", fmt.Errorf( + return "", fmt.Errorf( "could not get placement decision for " + placementName + " before timeout, fail") } @@ -136,7 +136,7 @@ func waitDRPCPhase(client client.Client, namespace, name string, phase ramen.DRS } func getCurrentCluster(client client.Client, namespace string, placementName string) (string, error) { - _, placementDecisionName, err := waitPlacementDecision(client, namespace, placementName) + placementDecisionName, err := waitPlacementDecision(client, namespace, placementName) if err != nil { return "", err } From 797475c53ff04b72e1d85a1601010bad7b89bfa4 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 21 Oct 2024 23:38:05 +0300 Subject: [PATCH 6/6] Handle conflicts when starting failover or relocate Fix random failures like: actions_test.go:53: Operation cannot be fulfilled on drplacementcontrols.ramendr.openshift.io "subscr-deploy-rbd-busybox": the object has been modified; please apply your changes to the latest version and try again Signed-off-by: Nir Soffer --- e2e/dractions/actions.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/e2e/dractions/actions.go b/e2e/dractions/actions.go index 406fc819a..970166508 100644 --- a/e2e/dractions/actions.go +++ b/e2e/dractions/actions.go @@ -165,11 +165,6 @@ func waitAndUpdateDRPC(client client.Client, namespace, drpcName string, action return err } - drpc, err := getDRPC(client, namespace, drpcName) - if err != nil { - return err - } - drPolicyName := util.DefaultDRPolicyName drpolicy, err := util.GetDRPolicy(client, drPolicyName) @@ -182,16 +177,23 @@ func waitAndUpdateDRPC(client client.Client, namespace, drpcName string, action return err } - drpc.Spec.Action = action - if action == ramen.ActionFailover { - drpc.Spec.FailoverCluster = targetCluster - } else { - drpc.Spec.PreferredCluster = targetCluster - } - util.Ctx.Log.Info("update drpc " + drpcName + " " + strings.ToLower(string(action)) + " to " + targetCluster) - return updateDRPC(client, drpc) + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + drpc, err := getDRPC(client, namespace, drpcName) + if err != nil { + return err + } + + drpc.Spec.Action = action + if action == ramen.ActionFailover { + drpc.Spec.FailoverCluster = targetCluster + } else { + drpc.Spec.PreferredCluster = targetCluster + } + + return updateDRPC(client, drpc) + }) } func GetNamespace(d deployers.Deployer, w workloads.Workload) string {