Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Kuttl] Add test for OVN db cluster pod deletions #283

Merged

Conversation

brfrenkel
Copy link
Contributor

@brfrenkel brfrenkel commented May 12, 2024

test steps:

  1. stand up 3 replicas ovn db (nb) cluster.
  2. confirm pods are all up.
  3. confirm that the pods established the mesh.
  4. delete all pods.
  5. check that new pods are respawned.
  6. check that they re-established the mesh with the correct number of pods,
    and all connections are good.

related Jira: OSPRH-6135

@openshift-ci openshift-ci bot requested review from booxter and olliewalsh May 12, 2024 14:32
@brfrenkel brfrenkel force-pushed the add_kuttl_test branch 14 times, most recently from 9bbcfb8 to d4240a7 Compare May 15, 2024 14:47
tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
tests/kuttl/tests/ovn_db_delete/02-scale-ovndb-nb.yaml Outdated Show resolved Hide resolved
@brfrenkel brfrenkel force-pushed the add_kuttl_test branch 2 times, most recently from cec2fbe to 4a8d1f6 Compare May 20, 2024 06:02
@brfrenkel
Copy link
Contributor Author

@booxter thanks for the great points!
I made the changes and now it looks much better :)

Copy link
Contributor

@booxter booxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost ready. Just a minor cleanup of the script logic, but otherwise this is great!

tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
tests/kuttl/common/scripts/check_cluster_status.sh Outdated Show resolved Hide resolved
@brfrenkel brfrenkel force-pushed the add_kuttl_test branch 2 times, most recently from 9121329 to d097725 Compare May 21, 2024 10:46
@booxter
Copy link
Contributor

booxter commented May 21, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 21, 2024
@booxter
Copy link
Contributor

booxter commented May 21, 2024

/hold

waiting for post-beta floodgates to open

@booxter
Copy link
Contributor

booxter commented May 21, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 21, 2024
Copy link
Contributor

@slawqo slawqo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small idea, but don't need to be added in this patch, and maybe don't makes sense at all, that's also fine :) Maybe before 05-delete-pods step You could also add step to scale down to 1 replica (or 2), assert that this went fine and then do the delete-pods step. Please let me know what do You think about it.

In overall this patch looks good for me. Thx for it. Great work :)

tests/kuttl/tests/ovn_db_delete/02-scale-ovndb-nb.yaml Outdated Show resolved Hide resolved
tests/kuttl/tests/ovn_db_delete/05-delete-pods.yaml Outdated Show resolved Hide resolved
test $? -eq 0
- script: |
$OVN_KUTTL_DIR/../common/scripts/check_cluster_status.sh sb 3
test $? -eq 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before cleanup in next step we should also have scenario for scale down. 3 -> 1 and ensure cluster is healthy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this after the deletes.

@@ -0,0 +1,9 @@
apiVersion: kuttl.dev/v1beta1
kind: TestAssert
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

04-assert.yaml and 06-assert.yaml looks same so can have single copy and have a symlink for other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

tests/kuttl/tests/ovn_db_delete/04-assert.yaml Outdated Show resolved Hide resolved
@booxter
Copy link
Contributor

booxter commented May 21, 2024

@slawqo yes I think it's a good idea to validate that delete pod in a single member cluster is also working. May be included here or as a follow-up.

@booxter
Copy link
Contributor

booxter commented May 23, 2024

/hold cancel


# arguments: db-type: {nb, sb}, num-pods
# Check arguments
if [ $# -lt 2 ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@booxter what do you say about those "echo's" in the script?
I added them so if it will fail there will be some info in the logs.
But I'm not sure if it's needed.

tests/kuttl/tests/ovn_db_delete/05-delete-pods.yaml Outdated Show resolved Hide resolved
tests/kuttl/tests/ovn_db_delete/05-delete-pods.yaml Outdated Show resolved Hide resolved
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/38f2b396e8b34d198e52286d6cfc7bf1

openstack-k8s-operators-content-provider FAILURE in 12m 31s

@brfrenkel
Copy link
Contributor Author

recheck

@brfrenkel brfrenkel force-pushed the add_kuttl_test branch 2 times, most recently from d763852 to a8cbbb7 Compare May 28, 2024 08:27
test steps:

1. stand up a 3 replicas ovn db cluster (both nb & sb).
2. confirm pods are all up.
3. confirm that the pods established the mesh.
4. delete all pods.
5. check that new pods are respawned.
6. check that they re-established the mesh with the correct number of pods.

related Jira: OSPRH-6135
Copy link
Contributor

openshift-ci bot commented May 28, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brfrenkel, slawqo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 0bb142a into openstack-k8s-operators:main May 28, 2024
5 checks passed
@karelyatin
Copy link
Contributor

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants