Skip to content
This repository was archived by the owner on Apr 1, 2026. It is now read-only.

tests: harden instance admin samples against timeouts#452

Merged
tseaver merged 1 commit into from
Oct 20, 2021
Merged

tests: harden instance admin samples against timeouts #452

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions samples/instanceadmin/test_instanceadmin.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -97,17 +97,23 @@ def test_run_instance_operations(capsys, dispose_of):


def test_delete_instance(capsys, dispose_of):
dispose_of(INSTANCE)
from concurrent.futures import TimeoutError

# Can't delete it, it doesn't exist
instanceadmin.delete_instance(PROJECT, INSTANCE)
out = capsys.readouterr().out
assert "Deleting instance" in out
assert f"Instance {INSTANCE} does not exist" in out
@backoff.on_exception(backoff.expo, TimeoutError)
def _set_up_instance():
dispose_of(INSTANCE)

# Ok, create it then
instanceadmin.run_instance_operations(PROJECT, INSTANCE, CLUSTER1)
capsys.readouterr() # throw away output
# Can't delete it, it doesn't exist
instanceadmin.delete_instance(PROJECT, INSTANCE)
out = capsys.readouterr().out
assert "Deleting instance" in out
assert f"Instance {INSTANCE} does not exist" in out

# Ok, create it then
instanceadmin.run_instance_operations(PROJECT, INSTANCE, CLUSTER1)
capsys.readouterr() # throw away output

_set_up_instance()

# Now delete it
instanceadmin.delete_instance(PROJECT, INSTANCE)
Expand All@@ -117,22 +123,29 @@ def test_delete_instance(capsys, dispose_of):


def test_add_and_delete_cluster(capsys, dispose_of):
dispose_of(INSTANCE)
from concurrent.futures import TimeoutError

# This won't work, because the instance isn't created yet
instanceadmin.add_cluster(PROJECT, INSTANCE, CLUSTER2)
out = capsys.readouterr().out
assert f"Instance {INSTANCE} does not exist" in out
@backoff.on_exception(backoff.expo, TimeoutError)
def _set_up_instance():
dispose_of(INSTANCE)

# Get the instance created
instanceadmin.run_instance_operations(PROJECT, INSTANCE, CLUSTER1)
capsys.readouterr() # throw away output
# This won't work, because the instance isn't created yet

@crwilcoxcrwilcoxOct 19, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment took a bit to understand. Are you saying we can't add a cluster because INSTANCE doesn't exist (and you ensured that in the line above)?

"get the instance created" also feels weird. Are we getting the instance or creating it? I had similar feelings about those above. I recognize those were carried forward.

@tseavertseaverOct 19, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The instance creation gets moved down to line 138 here -- the instance does not exist at the point that we first call instanceadmin.add_cluster.

The diff display is weird -- I don't get why it breaks up. All this PR does is move the "ensure the instance is set up" code inside a nested function (and therefore indented), wrapped in a backoff.

instanceadmin.add_cluster(PROJECT, INSTANCE, CLUSTER2)
out = capsys.readouterr().out
assert f"Instance {INSTANCE} does not exist" in out

# Get the instance created
instanceadmin.run_instance_operations(PROJECT, INSTANCE, CLUSTER1)
capsys.readouterr() # throw away output

_set_up_instance()

# Add a cluster to that instance
# Avoid failing for "instance is currently being changed" by
# applying an exponential backoff
w_backoff = backoff.on_exception(backoff.expo, exceptions.ServiceUnavailable)
w_backoff(instanceadmin.add_cluster)(PROJECT, INSTANCE, CLUSTER2)
backoff_503 = backoff.on_exception(backoff.expo, exceptions.ServiceUnavailable)

backoff_503(instanceadmin.add_cluster)(PROJECT, INSTANCE, CLUSTER2)
out = capsys.readouterr().out
assert f"Adding cluster to instance {INSTANCE}" in out
assert "Listing clusters..." in out
Expand Down