Fixing oracle integration tests

[a-dubs]

Proposed Commit Message

I should probably squash these once this is done being reviewed and ready to be merged right?

Additional Context

Test Steps

Tested locally using the following script:

declare -A tests
tests["focal"]="tests/integration_tests/modules/test_frequency_override.py::test_frequency_override \
tests/integration_tests/modules/test_ansible.py::test_ansible_pull_pip \
tests/integration_tests/modules/test_ubuntu_drivers.py::test_ubuntu_drivers_installed \
tests/integration_tests/modules/test_apt_functionality.py::test_install_missing_deps \
tests/integration_tests/modules/test_boothook.py::TestBoothook::test_boothook_header_runs_part_per_instance \
tests/integration_tests/modules/test_boothook.py::TestBoothook::test_boothook_waits_for_network"

tests["jammy"]="tests/integration_tests/modules/test_frequency_override.py::test_frequency_override \
tests/integration_tests/modules/test_ansible.py::test_ansible_pull_pip \
tests/integration_tests/modules/test_ubuntu_drivers.py::test_ubuntu_drivers_installed \
tests/integration_tests/modules/test_apt_functionality.py::test_install_missing_deps \
tests/integration_tests/modules/test_boothook.py::TestBoothook::test_boothook_header_runs_part_per_instance \
tests/integration_tests/modules/test_boothook.py::TestBoothook::test_boothook_waits_for_network"

tests["noble"]="tests/integration_tests/modules/test_ansible.py::test_ansible_pull_pip \
tests/integration_tests/modules/test_lxd.py::test_storage_lvm \
tests/integration_tests/modules/test_boothook.py::TestBoothook::test_boothook_header_runs_part_per_instance \
tests/integration_tests/modules/test_boothook.py::TestBoothook::test_boothook_waits_for_network \
tests/integration_tests/modules/test_ubuntu_drivers.py::test_ubuntu_drivers_installed \
tests/integration_tests/modules/test_ansible.py::test_ansible_pull_distro"

for suite in "${!tests[@]}"; do
    test_paths="${tests[$suite]}"
    log_file="integration-tests-logs/oracle-verification/$suite.log"
    # make sure directory exists
    mkdir -p "$(dirname "$log_file")"
    CLOUD_INIT_PLATFORM="oci" \
    CLOUD_INIT_OS_IMAGE="$suite" \
    CLOUD_INIT_CLOUD_INIT_SOURCE="ppa:cloud-init-dev/daily" \
    tox -e integration-tests-fast -- $test_paths \
    | tee "$log_file"
done

Merge type

• Squash merge using "Proposed Commit Message"
• Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

[TheRealFalcon]

Thanks for the updates here. I left a few comments inline.

[TheRealFalcon]

Two things here:

1. We don't really have a need to test older versions of cloud-init. We should generally assume we're always testing the latest version.
2. If possible, we should use pytest's skip decorator rather than skipping in the body of the test. In this case, we're spending the time launching an instance only to skip it and not really do anything.

If there's a reason a test needs to be different based on the platform we're running, we should skip like this instead.

[a-dubs]

I don't like the assumption that we are always testing the newest version tbh. Would like to discuss this further face to face. Given that I might want to test how cloud-init behaves on a stable release / publicly available image, I think it would be nice to start adding these gates where applicable moving forward. And the reason the skip must occur inside the test like this is because how else are we supposed to know what version of cloud-init is being used besides for checking on the live instance itself :P

And don't worry, for 99% of use cases this will be a no-op and the test will run anyways (like Jenkins), unless you're a freak like me and like setting CLOUD_INIT_SOURCE="NONE" hehe

[TheRealFalcon]

You should bring this one up at standup tomorrow

[a-dubs]

yeahh i forgot to today lol my b

[TheRealFalcon]

Oh also, mypy is angry. That'll need to be fixed too

[TheRealFalcon]

Just a couple more comments

[TheRealFalcon]

The problem with this approach is that it turns ANY failure into an xfail, so then the test turns out to be not very useful.

What if instead it looked something like:

@pytest.mark.skipif(PLATFORM != "oci", reason="Test is OCI specific")
def test_ubuntu_drivers_installed(session_cloud: IntegrationCloud):
    """..."""
    try:
        client = session_cloud.launch(
            launch_kwargs={"instance_type": "VM.GPU.A10.1"},
            user_data=USER_DATA,
        )
    except Exception as e:
        pytest.xfail(f"...{e}...")

    try:
        _do_test(client)
    finally:
        client.destroy()


def _do_test(client: IntegrationInstance):
    log = client.read_from_file("/var/log/cloud-init.log")
    verify_clean_log(log)
    verify_clean_boot(client)
    assert 1 == log.count(
        "Installing and activating NVIDIA drivers "
        "(nvidia/license-accepted=True, version=latest)"
    )
    result = client.execute("dpkg -l | grep nvidia")
    assert result.ok, "No nvidia packages found"
    assert re.search(r"ii\s+nvidia.*-\d+-server", result.stdout), (
        f"Did not find specific nvidia drivers packages in:"
        f" {result.stdout}"
    )

[TheRealFalcon]

I don't love introducing this fixture here just to fix some typing. Can we instead do something along the lines of assert v1_data["region"] == cast(Ec2Instance, client.cloud.cloud_instance).region for the assert?

[a-dubs]

I personally feel like that is a quite messy way to do it, and I would rather rely on using the actual underlying session_cloud fixture.

But if you disagree, I am happy to make your change.

[TheRealFalcon]

@holmanb , any strong opinions? Wanna be a tiebreaker?