Skip to content

cc_modules: set default meta frequency value when no config available#1457

Merged
blackboxsw merged 4 commits into
canonical:mainfrom
blackboxsw:fix-frequency-from-meta
May 16, 2022
Merged

cc_modules: set default meta frequency value when no config available#1457
blackboxsw merged 4 commits into
canonical:mainfrom
blackboxsw:fix-frequency-from-meta

Conversation

@blackboxsw
Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw commented May 16, 2022

Proposed Commit Message

Each cloud-config module defines meta["frequency"] module attribute.
This frequency will be preferred unless overrides are given in
/etc/cloud/cloud.cfg module definitions.

Additional Context

Test Steps

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

Each cloud-config module defines meta["frequency"] module attribute.
This frequency will be preferred unless overrides are given in
/etc/cloud/cloud.cfg module definitions.
@blackboxsw blackboxsw requested a review from TheRealFalcon May 16, 2022 16:20
@TheRealFalcon
Copy link
Copy Markdown
Contributor

Additional test if you want it. It's only checking logs, but hopefully our logs aren't lying to us.

diff --git a/tests/integration_tests/modules/test_combined.py b/tests/integration_tests/modules/test_combined.py
index eb45062e..58262ce1 100644
--- a/tests/integration_tests/modules/test_combined.py
+++ b/tests/integration_tests/modules/test_combined.py
@@ -5,12 +5,16 @@ of the test would be unlikely to affect the running of another test using
 the same instance launch. Most independent module coherence tests can go
 here.
 """
+import glob
+import importlib
 import json
 import re
 import uuid
+from pathlib import Path
 
 import pytest
 
+import cloudinit.config
 from tests.integration_tests.clouds import ImageSpecification
 from tests.integration_tests.decorators import retry
 from tests.integration_tests.instances import IntegrationInstance
@@ -223,6 +227,25 @@ class TestCombined:
             class_client.execute("stat -c %N /run/cloud-init/cloud-id")
         )
 
+    def test_run_frequency(self, class_client: IntegrationInstance):
+        log = class_client.read_from_file("/var/log/cloud-init.log")
+        config_dir = Path(cloudinit.config.__file__).parent
+        module_paths = glob.glob(str(config_dir / "cc*.py"))
+        module_names = [Path(x).stem for x in module_paths]
+        found_count = 0
+        for name in module_names:
+            mod = importlib.import_module(f"cloudinit.config.{name}")
+            frequency = mod.meta["frequency"]
+            # cc_ gets replaced with config- in logs
+            log_name = name.replace("cc_", "config-")
+            # Some modules have been filtered out in /etc/cloud/cloud.cfg,
+            if f"running {log_name}" in log:
+                found_count += 1  # Ensure we're matching on the right text
+                assert f"running {log_name} with frequency {frequency}" in log
+        assert (
+            found_count > 10
+        ), "Not enough modules found in log. Did the log message change?"
+
     def _check_common_metadata(self, data):
         assert data["base64_encoded_keys"] == []
         assert data["merged_cfg"] == "redacted for non-root user"

@blackboxsw
Copy link
Copy Markdown
Collaborator Author

Additional test if you want it. It's only checking logs, but hopefully our logs aren't lying to us.

diff --git a/tests/integration_tests/modules/test_combined.py b/tests/integration_tests/modules/test_combined.py
index eb45062e..58262ce1 100644
--- a/tests/integration_tests/modules/test_combined.py
+++ b/tests/integration_tests/modules/test_combined.py
@@ -5,12 +5,16 @@ of the test would be unlikely to affect the running of another test using
 the same instance launch. Most independent module coherence tests can go
 here.
 """
+import glob
+import importlib
 import json
 import re
 import uuid
+from pathlib import Path
 
 import pytest
 
+import cloudinit.config
 from tests.integration_tests.clouds import ImageSpecification
 from tests.integration_tests.decorators import retry
 from tests.integration_tests.instances import IntegrationInstance
@@ -223,6 +227,25 @@ class TestCombined:
             class_client.execute("stat -c %N /run/cloud-init/cloud-id")
         )
 
+    def test_run_frequency(self, class_client: IntegrationInstance):
+        log = class_client.read_from_file("/var/log/cloud-init.log")
+        config_dir = Path(cloudinit.config.__file__).parent
+        module_paths = glob.glob(str(config_dir / "cc*.py"))
+        module_names = [Path(x).stem for x in module_paths]
+        found_count = 0
+        for name in module_names:
+            mod = importlib.import_module(f"cloudinit.config.{name}")
+            frequency = mod.meta["frequency"]
+            # cc_ gets replaced with config- in logs
+            log_name = name.replace("cc_", "config-")
+            # Some modules have been filtered out in /etc/cloud/cloud.cfg,
+            if f"running {log_name}" in log:
+                found_count += 1  # Ensure we're matching on the right text
+                assert f"running {log_name} with frequency {frequency}" in log
+        assert (
+            found_count > 10
+        ), "Not enough modules found in log. Did the log message change?"
+
     def _check_common_metadata(self, data):
         assert data["base64_encoded_keys"] == []
         assert data["merged_cfg"] == "redacted for non-root user"

+1 , I was adding something similar to unit tests, but also just a general check that we didn't have anything fall through the cracks with a "None" frequency.
I'll pull in yours as I'd appreciate the full integration test on this, maybe we can suppplement with a no None frequency seen.

@TheRealFalcon
Copy link
Copy Markdown
Contributor

Here's one for overriding frequency. I put it in tests/integration_tests/modules/test_frequency_override.py:

import pytest

from tests.integration_tests.instances import IntegrationInstance

USER_DATA = """\
#cloud-config
runcmd:
 - echo "hi" >> /var/tmp/hi
"""


@pytest.mark.user_data(USER_DATA)
def test_frequency_override(client: IntegrationInstance):
    # Some pre-checks
    assert (
        "running config-scripts-user with frequency once-per-instance"
        in client.read_from_file("/var/log/cloud-init.log")
    )
    assert client.read_from_file("/var/tmp/hi").strip().count("hi") == 1

    # Change frequency of scripts-user to always
    config = client.read_from_file("/etc/cloud/cloud.cfg")
    new_config = config.replace("- scripts-user", "- [ scripts-user, always ]")
    client.write_to_file("/etc/cloud/cloud.cfg", new_config)

    client.restart()

    # Ensure the script was run again
    assert (
        "running config-scripts-user with frequency always"
        in client.read_from_file("/var/log/cloud-init.log")
    )
    assert client.read_from_file("/var/tmp/hi").strip().count("hi") == 2

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me as is. I recently commented another integration test you could add for overriding frequency if you'd like.

Comment thread cloudinit/config/modules.py
@blackboxsw blackboxsw merged commit 2eb2cea into canonical:main May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants