Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 61 additions & 0 deletions test/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -728,3 +728,64 @@ def test_monitor_client(get_monitor_token_for_db_entities):
)

return client, entity_ids


@pytest.fixture(scope="session")
def create_alert_service_definition(test_linode_client):
rule_criteria = {
"rules": [
{
"aggregate_function": "min",
"dimension_filters": [
{
"dimension_label": "node_type",
"label": "Node Type",
"operator": "eq",
"value": "primary",
}
],
"label": "Memory Usage",
"metric": "memory_usage",
"operator": "eq",
"threshold": 95,
"unit": "percent",
}
]
}
trigger_conditions = {
"criteria_condition": "ALL",
"evaluation_period_seconds": 300,
"polling_interval_seconds": 900,
"trigger_occurrences": 3,
}
channels = list(test_linode_client.monitor.alert_channels())
if len(channels) == 0:
raise Exception(
"No alert channels available for testing. Please create an alert channel and try again."
)
alert = test_linode_client.monitor.create_alert_definition(
service_type="dbaas",
label=get_test_label() + "-service-definition",
severity=1,
description="description",
channel_ids=[channels[0].id],
rule_criteria=rule_criteria,
trigger_conditions=trigger_conditions,
)

yield alert

alert.delete()


def get_system_alerts(client: LinodeClient):
alerts = client.monitor.alert_definitions()
system_alerts = []
for alert in alerts.lists[0]:
if alert.type == "system":
system_alerts.append(alert)
if len(system_alerts) == 0:
raise Exception(
"No system alert definitions found. Cannot run tests dependent on system alert definitions."
)
return system_alerts
102 changes: 100 additions & 2 deletions test/integration/models/linode/test_linode.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import ipaddress
import time
from test.integration.conftest import get_region
from test.integration.conftest import get_region, get_system_alerts
from test.integration.helpers import (
get_test_label,
retry_sending_request,
Expand All @@ -10,7 +10,7 @@

import pytest

from linode_api4.errors import ApiError
from linode_api4.errors import ApiError, UnexpectedResponseError
from linode_api4.objects import (
Config,
ConfigInterface,
Expand Down Expand Up @@ -231,10 +231,37 @@ def get_status(linode: Instance, status: str):
return linode.status == status


def wait_for_clone_complete_and_delete_linode(
interval: int, timeout: int, linode: Instance
) -> object:
end_time = time.time() + timeout
while time.time() < end_time:
try:
linode.delete()
return True
except ApiError as err:
if "[400] Linode is the target of an ongoing clone" not in str(err):
raise UnexpectedResponseError(f"Unexpected delete linode error")
time.sleep(interval)
raise TimeoutError(
f"Timeout Error: not possible to delete just cloned linode in {timeout} seconds"
)


def instance_type_condition(linode: Instance, type: str):
return type in str(linode.type)


def test_get_linodes_verify_alerts(test_linode_client, create_linode):
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.

create_linode is not used in this test. Probably can be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct, but we need at least one linode to verify get linodes endpoint. It was suggested by copilot #669 (comment). What do you think about this?

Copy link
Copy Markdown
Contributor

@yec-akamai yec-akamai Mar 31, 2026

Choose a reason for hiding this comment

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

The copilot suggests to not use the listing endpoint linode.instance() and use create_linode. I agree that it's safer to not assume there is linode available by listing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I use code fix suggested by Copilot. Should I change it somehow?

linodes_list = test_linode_client.linode.instances().lists[0]
assert len(linodes_list) > 0
assert linodes_list[0].alerts.cpu >= 0
assert linodes_list[0].alerts.io >= 0
assert linodes_list[0].alerts.network_in >= 0
assert linodes_list[0].alerts.network_out >= 0
assert linodes_list[0].alerts.transfer_quota >= 0


Comment on lines +256 to +264
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

test_get_linodes_verify_alerts is non-deterministic: it creates a Linode via the create_linode fixture but then asserts against instances().lists[0][0], which may be an unrelated instance (ordering/pagination is not guaranteed). To make the test reliable, assert against the specific Linode created by the fixture (e.g., load by create_linode.id or filter the list for that ID), and avoid reaching into the internal .lists cache when plain indexing/iteration works.

Suggested change
linodes_list = test_linode_client.linode.instances().lists[0]
assert len(linodes_list) > 0
assert linodes_list[0].alerts.cpu >= 0
assert linodes_list[0].alerts.io >= 0
assert linodes_list[0].alerts.network_in >= 0
assert linodes_list[0].alerts.network_out >= 0
assert linodes_list[0].alerts.transfer_quota >= 0
linode = test_linode_client.load(Instance, create_linode.id)
assert linode.alerts.cpu >= 0
assert linode.alerts.io >= 0
assert linode.alerts.network_in >= 0
assert linode.alerts.network_out >= 0
assert linode.alerts.transfer_quota >= 0

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would implement extra filtering to find just created linode, but this test is like smoke test and in my opinion we don't need to check specific values. Do you see any benefit of this change?

def test_get_linode(test_linode_client, linode_with_volume_firewall):
linode = test_linode_client.load(Instance, linode_with_volume_firewall.id)

Expand Down Expand Up @@ -283,6 +310,8 @@ def test_linode_rebuild(test_linode_client):

assert linode.status == "rebuilding"
assert linode.image.id == "linode/debian12"
assert linode.alerts.cpu >= 0
assert linode.alerts.io >= 0

assert linode.disk_encryption == InstanceDiskEncryptionType.disabled

Expand Down Expand Up @@ -346,6 +375,75 @@ def test_linode_reboot(create_linode):
assert linode.status == "running"


def test_linode_alerts_workflow(test_linode_client, create_linode):
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.

Can we add a test case to creat linode with aclp alerts? Adding system_alerts should be good

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, please verify test just added: test_update_linode_aclp_alerts

linode = create_linode
parent_linode_id = create_linode.id
assert linode.alerts.cpu == 90
assert linode.alerts.io == 10000
assert linode.alerts.network_in == 10
assert linode.alerts.network_out == 10
assert linode.alerts.transfer_quota == 80
assert isinstance(linode.alerts.system_alerts, list)
assert isinstance(linode.alerts.user_alerts, list)

linode = test_linode_client.load(Instance, parent_linode_id)
assert linode.alerts.cpu == 90
assert linode.alerts.io == 10000
assert linode.alerts.network_in == 10
assert linode.alerts.network_out == 10
assert linode.alerts.transfer_quota == 80
assert isinstance(linode.alerts.system_alerts, list)
assert isinstance(linode.alerts.user_alerts, list)

linode.alerts = {
"cpu": 50,
"io": 6000,
"network_in": 20,
"network_out": 20,
"transfer_quota": 40,
}
linode_save_status = linode.save()
assert linode_save_status == True
assert linode.alerts["cpu"] == 50
assert linode.alerts["io"] == 6000
assert linode.alerts["network_in"] == 20
assert linode.alerts["network_out"] == 20
assert linode.alerts["transfer_quota"] == 40

wait_for_condition(10, 100, get_status, linode, "running")
new_linode = retry_sending_request(
5,
linode.clone,
region=linode.region.id,
instance_type=linode.type.id,
label=get_test_label(),
)
assert new_linode.alerts.cpu == 90
assert new_linode.alerts.io == 10000
assert new_linode.alerts.network_in == 10
assert new_linode.alerts.network_out == 10
assert new_linode.alerts.transfer_quota == 80
assert isinstance(new_linode.alerts.system_alerts, list)
assert isinstance(new_linode.alerts.user_alerts, list)
Comment on lines +398 to +427
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

In test_linode_alerts_workflow, the test updates linode.alerts (cpu/io/network_*/transfer_quota) and calls linode.save(), but none of the subsequent assertions check that the update actually took effect. As written, the test would still pass even if the save was a no-op, which weakens the intended coverage. Consider asserting the saved values after a reload, and then align the clone assertions with the intended behavior (either the clone inherits the updated alerts, or it resets to defaults—right now it asserts the same default values as before the update).

Copilot uses AI. Check for mistakes.

wait_for_clone_complete_and_delete_linode(10, 100, new_linode)


def test_update_linode_aclp_alerts(
test_linode_client, create_linode, create_alert_service_definition
):
linode = create_linode
sample_system_alert = get_system_alerts(test_linode_client)[0].id

linode.alerts = {
"user_alerts": [create_alert_service_definition.id],
"system_alerts": [sample_system_alert],
}
linode.save()
assert linode.alerts["user_alerts"] == [create_alert_service_definition.id]
assert linode.alerts["system_alerts"] == [sample_system_alert]


def test_linode_shutdown(create_linode):
linode = create_linode

Expand Down
Loading