-
Notifications
You must be signed in to change notification settings - Fork 17
Fix multi-arch smoke setup and some flaky test #387
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
Changes from 6 commits
abf07d3
1d47cbf
8dd0fb0
500eef9
71f26d5
06effa5
40124cf
bdefeda
aee5180
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1475,7 +1475,7 @@ buildvariants: | |
run_on: | ||
- rhel9-power-small | ||
- rhel9-power-large | ||
allowed_requesters: [ "patch", "github_tag" ] | ||
allowed_requesters: [ "patch", "github_tag" , "commit"] | ||
depends_on: | ||
- name: build_operator_ubi | ||
variant: init_test_run | ||
|
@@ -1498,7 +1498,7 @@ buildvariants: | |
run_on: | ||
- rhel9-zseries-small | ||
- rhel9-zseries-large | ||
allowed_requesters: [ "patch", "github_tag" ] | ||
allowed_requesters: [ "patch", "github_tag", "commit"] | ||
depends_on: | ||
- name: build_operator_ubi | ||
variant: init_test_run | ||
|
@@ -1520,7 +1520,7 @@ buildvariants: | |
tags: [ "e2e_test_suite", "e2e_smoke_release_test_suite" ] | ||
run_on: | ||
- ubuntu2204-arm64-large | ||
allowed_requesters: [ "patch", "github_tag" ] | ||
allowed_requesters: [ "patch", "github_tag", "commit"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can you please explain this a bit, just for my understanding. Do you mean, now this test will also be run when a commit is pushed to a PR branch? If yes, was not does if we just had patch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no - only direct commits to master - which means merges to mainline (mainline is master in our case) in evergreen terminology (more on the linked docs). "Yes, setting allowed_requesters: ["commit"] is correct if you want your Evergreen task to run only on mainline (master) commits—i.e., after a branch is merged into master. This will prevent the task from running on PRs, patches, or other request types." |
||
<<: *base_no_om_image_dependency | ||
tasks: | ||
- name: e2e_smoke_arm_task_group | ||
|
@@ -1530,7 +1530,7 @@ buildvariants: | |
tags: [ "e2e_test_suite", "e2e_smoke_release_test_suite", "static" ] | ||
run_on: | ||
- ubuntu2204-arm64-large | ||
allowed_requesters: [ "patch", "github_tag" ] | ||
allowed_requesters: [ "patch", "github_tag", "commit"] | ||
<<: *base_no_om_image_dependency | ||
tasks: | ||
- name: e2e_smoke_arm_task_group | ||
|
@@ -1541,7 +1541,7 @@ buildvariants: | |
run_on: | ||
- rhel9-zseries-small | ||
- rhel9-zseries-large | ||
allowed_requesters: [ "patch", "github_tag" ] | ||
allowed_requesters: [ "patch", "github_tag", "commit"] | ||
depends_on: | ||
- name: build_operator_ubi | ||
variant: init_test_run | ||
|
@@ -1564,7 +1564,7 @@ buildvariants: | |
run_on: | ||
- rhel9-power-small | ||
- rhel9-power-large | ||
allowed_requesters: [ "patch", "github_tag" ] | ||
allowed_requesters: [ "patch", "github_tag", "commit"] | ||
depends_on: | ||
- name: build_operator_ubi | ||
variant: init_test_run | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
create_secret, | ||
find_fixture, | ||
read_secret, | ||
try_load, | ||
update_secret, | ||
wait_until, | ||
) | ||
|
@@ -22,7 +23,16 @@ | |
USER_DATABASE = "admin" | ||
|
||
|
||
@fixture(scope="module") | ||
def create_password_secret(namespace: str) -> str: | ||
create_or_update_secret( | ||
namespace, | ||
PASSWORD_SECRET_NAME, | ||
{"password": USER_PASSWORD}, | ||
) | ||
return PASSWORD_SECRET_NAME | ||
|
||
|
||
@fixture(scope="function") | ||
def replica_set(namespace: str, custom_mdb_version) -> MongoDB: | ||
resource = MongoDB.from_yaml( | ||
find_fixture("replica-set-scram-sha-256.yaml"), | ||
|
@@ -36,37 +46,33 @@ def replica_set(namespace: str, custom_mdb_version) -> MongoDB: | |
"enabled": True, | ||
"modes": ["SCRAM"], | ||
} | ||
|
||
return resource.update() | ||
try_load(resource) | ||
return resource | ||
|
||
|
||
@fixture(scope="module") | ||
@fixture(scope="function") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are editing the user and secret later - module scope means we wouldn't return the "updated" user/secret... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocking: Doesn't this also mean that we will revert any change made to the user? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are right, that whole setup is bad. Let me rework that whole structure. We should have function for reads and generally split up read and write... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. adressed here: 06effa5 ugh i remember that we had the idea of reworking all tests to follow a clear pattern such that we are not running into that again.. |
||
def scram_user(namespace: str) -> MongoDBUser: | ||
resource = MongoDBUser.from_yaml(find_fixture("scram-sha-user.yaml"), namespace=namespace) | ||
|
||
create_or_update_secret( | ||
KubernetesTester.get_namespace(), | ||
resource.get_secret_name(), | ||
{"password": USER_PASSWORD}, | ||
) | ||
try_load(resource) | ||
return resource | ||
|
||
return resource.update() | ||
|
||
|
||
@fixture(scope="module") | ||
@fixture(scope="function") | ||
def standard_secret(replica_set: MongoDB): | ||
secret_name = "{}-{}-{}".format(replica_set.name, USER_NAME, USER_DATABASE) | ||
return read_secret(replica_set.namespace, secret_name) | ||
|
||
|
||
@fixture(scope="module") | ||
@fixture(scope="function") | ||
def connection_string_secret(replica_set: MongoDB): | ||
return read_secret(replica_set.namespace, CONNECTION_STRING_SECRET_NAME) | ||
|
||
|
||
@mark.e2e_replica_set_scram_sha_256_user_connectivity | ||
class TestReplicaSetCreation(KubernetesTester): | ||
def test_replica_set_created(self, replica_set: MongoDB): | ||
replica_set.create() | ||
nammn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
replica_set.assert_reaches_phase(Phase.Running, timeout=400) | ||
|
||
def test_replica_set_connectivity(self, replica_set: MongoDB): | ||
|
@@ -82,7 +88,9 @@ def test_ops_manager_state_correctly_updated(self, replica_set: MongoDB): | |
|
||
|
||
@mark.e2e_replica_set_scram_sha_256_user_connectivity | ||
def test_create_user(scram_user: MongoDBUser): | ||
def test_create_user(scram_user: MongoDBUser, namespace: str): | ||
create_password_secret(namespace) | ||
scram_user.create() | ||
scram_user.assert_reaches_phase(Phase.Updated) | ||
|
||
|
||
|
@@ -124,10 +132,15 @@ def test_user_cannot_authenticate_with_incorrect_password(self, replica_set: Mon | |
|
||
@mark.e2e_replica_set_scram_sha_256_user_connectivity | ||
class TestCanChangePassword(KubernetesTester): | ||
def test_user_can_authenticate_with_new_password(self, namespace: str, replica_set: MongoDB): | ||
update_secret(namespace, PASSWORD_SECRET_NAME, {"password": "my-new-password7"}) | ||
def test_user_can_authenticate_with_new_password( | ||
self, scram_user: MongoDBUser, namespace: str, replica_set: MongoDB | ||
): | ||
new_password = "my-new-password7" | ||
update_secret(namespace, PASSWORD_SECRET_NAME, {"password": new_password}) | ||
scram_user.assert_reaches_phase(Phase.Updated) | ||
nammn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
replica_set.tester().assert_scram_sha_authentication( | ||
password="my-new-password7", | ||
password=new_password, | ||
username="mms-user-1", | ||
auth_mechanism="SCRAM-SHA-256", | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,7 +117,6 @@ def test_user_can_authenticate_with_new_password(self): | |
password="my-new-password", | ||
username="mms-user-1", | ||
auth_mechanism="SCRAM-SHA-256", | ||
attempts=20, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missed that from prior pr where we changed the default to 50... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1m is way too close to the actual time required to pass the test. When it passed it needed 55 seconds |
||
) | ||
|
||
def test_user_cannot_authenticate_with_old_password(self): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rag bot says this is correct; also stated here: https://docs.devprod.prod.corp.mongodb.com/evergreen/Project-Configuration/Project-Configuration-Files#controlling-when-tasks-and-variants-run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by this is correct - I mean merging to master will trigger this task