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.

Sign up for GitHub

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

Jump to bottom

Improve verify_clean_boot() #5671

Merged
merged 2 commits into from
Sep 9, 2024
Merged

Improve verify_clean_boot() #5671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ba8878
test: add deprecation support to verify_clean_boot (#5671)
holmanb Sep 5, 2024
75e9ecf
test: add verify_clean_boot() calls alongside verify_clean_log() (#5671)
holmanb Sep 5, 2024
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
3 changes: 2 additions & 1 deletion tests/integration_tests/bugs/test_gh632.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log


# With some datasource hacking, we can run this on a NoCloud instance
Expand All @@ -30,6 +30,7 @@ def test_datasource_rbx_no_stacktrace(client: IntegrationInstance):

log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client)
assert "Failed to load metadata and userdata" not in log
assert (
"Getting data from <class 'cloudinit.sources.DataSourceRbxCloud."
Expand Down
3 changes: 2 additions & 1 deletion tests/integration_tests/bugs/test_gh868.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log

USERDATA = """\
#cloud-config
Expand All @@ -24,3 +24,4 @@
def test_chef_license(client: IntegrationInstance):
log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client)
2 changes: 2 additions & 0 deletions tests/integration_tests/bugs/test_lp1813396.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.util import (
verify_clean_boot,
verify_clean_log,
verify_ordered_items_in_text,
)
Expand All @@ -33,6 +34,7 @@ def test_gpg_no_tty(client: IntegrationInstance):
]
verify_ordered_items_in_text(to_verify, log)
verify_clean_log(log)
verify_clean_boot(client)
control_groups = client.execute(
"systemd-cgls -u cloud-config.service 2>/dev/null | wc -l"
).stdout
Expand Down
3 changes: 2 additions & 1 deletion tests/integration_tests/bugs/test_lp1886531.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import pytest

from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log

USER_DATA = """\
#cloud-config
Expand All @@ -26,3 +26,4 @@ class TestLp1886531:
def test_lp1886531(self, client):
log_content = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log_content)
verify_clean_boot(client)
3 changes: 2 additions & 1 deletion tests/integration_tests/bugs/test_lp1898997.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from tests.integration_tests import random_mac_address
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.releases import CURRENT_RELEASE, FOCAL
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log

MAC_ADDRESS = random_mac_address()

Expand Down Expand Up @@ -74,6 +74,7 @@ def test_ovs_member_interfaces_not_excluded(self, client):

# Confirm that the network configuration was applied successfully
verify_clean_log(cloudinit_output)
verify_clean_boot(client)
# Confirm that the applied network config created the OVS bridge
assert "ovs-br" in client.execute("ip addr")

Expand Down
17 changes: 11 additions & 6 deletions tests/integration_tests/cmd/test_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from tests.integration_tests.releases import CURRENT_RELEASE, MANTIC
from tests.integration_tests.util import (
get_feature_flag_value,
verify_clean_boot,
verify_clean_log,
)

Expand Down Expand Up @@ -70,16 +71,20 @@ def test_clean_log(self, class_client: IntegrationInstance):
version_boundary = get_feature_flag_value(
class_client, "DEPRECATION_INFO_BOUNDARY"
)
boundary_message = "Deprecated cloud-config provided:"
Copy link
Member

Choose a reason for hiding this comment

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

This feels overly cumbersome for the caller and feels slightly worse than what we had before. Ideally, we'd have some function like verify_schema_deprecations(version="22.2", messages=[...]). That could probably be folded into verify_clean_boot(), but it makes more sense as a separate function to me. I don't think it's a blocker for this PR, but I think there's still more work that can be done here.

Copy link
Member Author

@holmanb holmanb Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed. This pattern repeats itself a couple of times in tests and I'd love to make testing this easier.

That could probably be folded into verify_clean_boot(), but it makes more sense as a separate function to me.

+1 - Trying to pack that functionality into verify_clean_boot()'s signature would make the signature too messy and take on more scope than I'd like it to. In cases like this it would be cleaner / simpler to verify_clean_boot(ignore_deprecations=True) and then use a helper like you suggest.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's a blocker for this PR, but I think there's still more work that can be done here.

Agreed, this is is a rough edge that I'd love to make more ergonomic. I might try to tackle that in a follow-up PR sometime soon.

messages = [
"apt_reboot_if_required: Deprecated ",
"apt_update: Deprecated in version",
"apt_upgrade: Deprecated in version",
]
# the deprecation_version is 22.2 in schema for apt_* keys in
# user-data. Pass 22.2 in against the client's version_boundary.
if lifecycle.should_log_deprecation("22.2", version_boundary):
log_level = "DEPRECATED"
messages += boundary_message
verify_clean_boot(class_client, require_deprecations=messages)
else:
log_level = "INFO"
assert f"{log_level}]: Deprecated cloud-config provided:" in log
assert "apt_reboot_if_required: Deprecated " in log
assert "apt_update: Deprecated in version" in log
assert "apt_upgrade: Deprecated in version" in log
verify_clean_boot(class_client, require_deprecations=messages)
assert boundary_message in log

def test_network_config_schema_validation(
self, class_client: IntegrationInstance
Expand Down
7 changes: 6 additions & 1 deletion tests/integration_tests/datasources/test_lxd_discovery.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.releases import CURRENT_RELEASE, IS_UBUNTU
from tests.integration_tests.util import lxd_has_nocloud, verify_clean_log
from tests.integration_tests.util import (
lxd_has_nocloud,
verify_clean_boot,
verify_clean_log,
)


def _customize_environment(client: IntegrationInstance):
Expand Down Expand Up @@ -75,6 +79,7 @@ def test_lxd_datasource_discovery(client: IntegrationInstance):
} == netplan_cfg
log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client)
result = client.execute("cloud-id")
if result.stdout != "lxd":
raise AssertionError(
Expand Down
15 changes: 9 additions & 6 deletions tests/integration_tests/datasources/test_nocloud.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,15 +198,18 @@ def test_smbios_seed_network(self, client: IntegrationInstance):
version_boundary = get_feature_flag_value(
client, "DEPRECATION_INFO_BOUNDARY"
)
message = (
"The 'nocloud-net' datasource name is "
'deprecated" /var/log/cloud-init.log'
)
# nocloud-net deprecated in version 24.1
if lifecycle.should_log_deprecation("24.1", version_boundary):
log_level = "DEPRECATED"
verify_clean_boot(client, require_deprecations=[message])
else:
log_level = "INFO"
client.execute(
rf"grep \"{log_level}]: The 'nocloud-net' datasource name is"
' deprecated" /var/log/cloud-init.log'
).ok
client.execute(
r"grep \"INFO]: The 'nocloud-net' datasource name is"
' deprecated" /var/log/cloud-init.log'
).ok


@pytest.mark.skipif(PLATFORM != "lxd_vm", reason="Modifies grub config")
Expand Down
4 changes: 3 additions & 1 deletion tests/integration_tests/datasources/test_none.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import json

from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log

DS_NONE_BASE_CFG = """\
datasource_list: [None]
Expand All @@ -26,6 +26,7 @@ def test_datasource_none_discovery(client: IntegrationInstance):
"""
log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client)
# Limit datasource detection to DataSourceNone.
client.write_to_file(
"/etc/cloud/cloud.cfg.d/99-force-dsnone.cfg", DS_NONE_BASE_CFG
Expand Down Expand Up @@ -65,4 +66,5 @@ def test_datasource_none_discovery(client: IntegrationInstance):
)
log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client, require_warnings=expected_warnings)
assert client.execute("test -f /var/tmp/success-with-datasource-none").ok
3 changes: 2 additions & 1 deletion tests/integration_tests/datasources/test_oci_networking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from tests.integration_tests.clouds import IntegrationCloud
from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log

DS_CFG = """\
datasource:
Expand Down Expand Up @@ -52,6 +52,7 @@ def test_oci_networking_iscsi_instance(client: IntegrationInstance, tmpdir):

log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client)

assert (
"opc/v2/vnics/" not in log
Expand Down
3 changes: 2 additions & 1 deletion tests/integration_tests/datasources/test_tmp_noexec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log


def customize_client(client: IntegrationInstance):
Expand Down Expand Up @@ -30,3 +30,4 @@ def test_dhcp_tmp_noexec(client: IntegrationInstance):
not in log
)
verify_clean_log(log)
verify_clean_boot(client)
3 changes: 3 additions & 0 deletions tests/integration_tests/modules/test_ansible.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from tests.integration_tests.releases import CURRENT_RELEASE, FOCAL
from tests.integration_tests.util import (
push_and_enable_systemd_unit,
verify_clean_boot,
verify_clean_log,
)

Expand Down Expand Up @@ -267,6 +268,7 @@ def _test_ansible_pull_from_local_server(my_client):
my_client.restart()
log = my_client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(my_client)
output_log = my_client.read_from_file("/var/log/cloud-init-output.log")
assert "ok=3" in output_log
assert "SUCCESS: config-ansible ran successfully" in log
Expand Down Expand Up @@ -320,6 +322,7 @@ def test_ansible_pull_distro(client):
def test_ansible_controller(client):
log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client)
content_ansible = client.execute(
"lxc exec lxd-container-00 -- cat /home/ansible/ansible.txt"
)
Expand Down
2 changes: 2 additions & 0 deletions tests/integration_tests/modules/test_apt_functionality.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from tests.integration_tests.releases import CURRENT_RELEASE, IS_UBUNTU
from tests.integration_tests.util import (
get_feature_flag_value,
verify_clean_boot,
verify_clean_log,
)

Expand Down Expand Up @@ -489,4 +490,5 @@ def test_install_missing_deps(setup_image, session_cloud: IntegrationCloud):
) as minimal_client:
log = minimal_client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(minimal_client)
assert re.search(RE_GPG_SW_PROPERTIES_INSTALLED, log)
3 changes: 2 additions & 1 deletion tests/integration_tests/modules/test_boothook.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import pytest

from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log

USER_DATA = """\
#cloud-boothook
Expand All @@ -25,6 +25,7 @@ def test_boothook_header_runs_part_per_instance(client: IntegrationInstance):
RE_BOOTHOOK = f"BOOTHOOK: {instance_id}: is called every boot"
log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
verify_clean_boot(client)
output = client.read_from_file("/boothook.txt")
assert 1 == len(re.findall(RE_BOOTHOOK, output))
client.restart()
Expand Down
7 changes: 6 additions & 1 deletion tests/integration_tests/modules/test_ca_certs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@

from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.releases import IS_UBUNTU
from tests.integration_tests.util import get_inactive_modules, verify_clean_log
from tests.integration_tests.util import (
get_inactive_modules,
verify_clean_boot,
verify_clean_log,
)

CERT_CONTENT = """\
-----BEGIN CERTIFICATE-----
Expand Down Expand Up @@ -106,6 +110,7 @@ def test_clean_log(self, class_client: IntegrationInstance):
"""
log = class_client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log, ignore_deprecations=False)
verify_clean_boot(class_client)

expected_inactive = {
"apt_pipelining",
Expand Down
24 changes: 14 additions & 10 deletions tests/integration_tests/modules/test_combined.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
get_feature_flag_value,
get_inactive_modules,
lxd_has_nocloud,
verify_clean_boot,
verify_clean_log,
verify_ordered_items_in_text,
)
Expand Down Expand Up @@ -137,23 +138,26 @@ def test_deprecated_message(self, class_client: IntegrationInstance):
version_boundary = get_feature_flag_value(
class_client, "DEPRECATION_INFO_BOUNDARY"
)
deprecated_messages = ["users.1.sudo: Changed in version 22.2."]
boundary_message = (
"The value of 'false' in user craig's 'sudo'"
" config is deprecated"
)
# the changed_version is 22.2 in schema for user.sudo key in
# user-data. Pass 22.2 in against the client's version_boundary.
if lifecycle.should_log_deprecation("22.2", version_boundary):
log_level = "DEPRECATED"
deprecation_count = 2
deprecated_messages.append(boundary_message)
verify_clean_boot(
class_client, require_deprecations=deprecated_messages
)
else:
# Expect the distros deprecated call to be redacted.
# jsonschema still emits deprecation log due to changed_version
# instead of deprecated_version
log_level = "INFO"
deprecation_count = 1

assert (
f"[{log_level}]: The value of 'false' in user craig's 'sudo'"
" config is deprecated" in log
)
assert deprecation_count == log.count("DEPRECATE")
verify_clean_boot(
class_client, require_deprecations=deprecated_messages
)
assert f"[INFO]: {boundary_message}" in log

def test_ntp_with_apt(self, class_client: IntegrationInstance):
"""LP #1628337.
Expand Down
5 changes: 4 additions & 1 deletion tests/integration_tests/modules/test_disk_setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.releases import CURRENT_RELEASE, FOCAL, IS_UBUNTU
from tests.integration_tests.util import verify_clean_log
from tests.integration_tests.util import verify_clean_boot, verify_clean_log

DISK_PATH = "/tmp/test_disk_setup_{}".format(uuid4())

Expand Down Expand Up @@ -69,6 +69,7 @@ def test_device_alias(self, create_disk, client: IntegrationInstance):
assert "changed my_alias.1 => /dev/sdb1" in log
assert "changed my_alias.2 => /dev/sdb2" in log
verify_clean_log(log)
verify_clean_boot(client)

lsblk = json.loads(client.execute("lsblk --json"))
sdb = [x for x in lsblk["blockdevices"] if x["name"] == "sdb"][0]
Expand Down Expand Up @@ -143,6 +144,7 @@ class TestPartProbeAvailability:

def _verify_first_disk_setup(self, client, log):
verify_clean_log(log)
verify_clean_boot(client)
lsblk = json.loads(client.execute("lsblk --json"))
sdb = [x for x in lsblk["blockdevices"] if x["name"] == "sdb"][0]
assert len(sdb["children"]) == 2
Expand Down Expand Up @@ -200,6 +202,7 @@ def test_disk_setup_when_mounted(

# Assert new setup works as expected
verify_clean_log(log)
verify_clean_boot(client)

lsblk = json.loads(client.execute("lsblk --json"))
sdb = [x for x in lsblk["blockdevices"] if x["name"] == "sdb"][0]
Expand Down
Loading