feat: enable log_level for process parents (#685)

Signed-off-by: Alex <[email protected]> Co-authored-by: Bill Wei <[email protected]> Co-authored-by: Tom Tuffin <[email protected]>
ansible · Feb 26, 2024 · c719e7a · c719e7a
1 parent 7e956b8
commit c719e7a
Show file tree

Hide file tree

Showing 14 changed files with 210 additions and 8 deletions.
diff --git a/src/aap_eda/api/serializers/activation.py b/src/aap_eda/api/serializers/activation.py
@@ -111,6 +111,7 @@ class Meta:
             "awx_token_id",
             "credentials",
             "event_streams",
+            "log_level",
         ]
         read_only_fields = [
             "id",
@@ -161,6 +162,7 @@ class Meta:
             "awx_token_id",
             "credentials",
             "event_streams",
+            "log_level",
         ]
         read_only_fields = ["id", "created_at", "modified_at"]
 
@@ -199,6 +201,7 @@ def to_representation(self, activation):
             "awx_token_id": activation.awx_token_id,
             "credentials": credentials,
             "event_streams": event_streams,
+            "log_level": activation.log_level,
         }
 
 
@@ -219,6 +222,7 @@ class Meta:
             "awx_token_id",
             "credentials",
             "event_streams",
+            "log_level",
         ]
 
     rulebook_id = serializers.IntegerField(
@@ -354,6 +358,7 @@ class Meta:
             "awx_token_id",
             "credentials",
             "event_streams",
+            "log_level",
         ]
         read_only_fields = ["id", "created_at", "modified_at", "restarted_at"]
 
@@ -432,6 +437,7 @@ def to_representation(self, activation):
             "awx_token_id": activation.awx_token_id,
             "credentials": credentials,
             "event_streams": event_streams,
+            "log_level": activation.log_level,
         }
 
 

diff --git a/src/aap_eda/api/serializers/event_stream.py b/src/aap_eda/api/serializers/event_stream.py
@@ -145,6 +145,7 @@ class Meta:
             "decision_environment_id",
             "user",
             "credentials",
+            "log_level",
             *read_only_fields,
         ]
 
@@ -197,6 +198,7 @@ class Meta:
             "user",
             "restart_policy",
             "credentials",
+            "log_level",
         ]
 
     def create(self, validated_data):

diff --git a/src/aap_eda/core/enums.py b/src/aap_eda/core/enums.py
@@ -15,6 +15,7 @@
 from enum import Enum
 
 
+# TODO(alex): migrate to django.db.models.TextChoices
 class DjangoStrEnum(str, Enum):
     @classmethod
     def choices(cls):
@@ -114,3 +115,11 @@ class ProcessParentType(DjangoStrEnum):
 
     ACTIVATION = "activation"
     EVENT_STREAM = "event_stream"
+
+
+class RulebookProcessLogLevel(DjangoStrEnum):
+    """Types of log levels for a rulebook process."""
+
+    DEBUG = "debug"
+    INFO = "info"
+    ERROR = "error"
diff --git a/src/aap_eda/core/migrations/0026_activation_log_level_eventstream_log_level.py b/src/aap_eda/core/migrations/0026_activation_log_level_eventstream_log_level.py
@@ -0,0 +1,40 @@
+# Generated by Django 4.2.7 on 2024-02-19 16:38
+
+from django.db import migrations, models
+
+import aap_eda.core.utils
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("core", "0025_rename_args_eventstream_source_args"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="activation",
+            name="log_level",
+            field=models.CharField(
+                choices=[
+                    ("debug", "debug"),
+                    ("info", "info"),
+                    ("error", "error"),
+                ],
+                default=aap_eda.core.utils.get_default_log_level,
+                max_length=20,
+            ),
+        ),
+        migrations.AddField(
+            model_name="eventstream",
+            name="log_level",
+            field=models.CharField(
+                choices=[
+                    ("debug", "debug"),
+                    ("info", "info"),
+                    ("error", "error"),
+                ],
+                default=aap_eda.core.utils.get_default_log_level,
+                max_length=20,
+            ),
+        ),
+    ]
diff --git a/src/aap_eda/core/models/activation.py b/src/aap_eda/core/models/activation.py
@@ -14,7 +14,12 @@
 
 from django.db import models
 
-from aap_eda.core.enums import ActivationStatus, RestartPolicy
+from aap_eda.core.enums import (
+    ActivationStatus,
+    RestartPolicy,
+    RulebookProcessLogLevel,
+)
+from aap_eda.core.utils import get_default_log_level
 from aap_eda.services.activation.engine.common import ContainerableMixin
 
 from .mixins import StatusHandlerModelMixin
@@ -102,3 +107,8 @@ class Meta:
         "EventStream",
         default=None,
     )
+    log_level = models.CharField(
+        max_length=20,
+        choices=RulebookProcessLogLevel.choices(),
+        default=get_default_log_level,
+    )
diff --git a/src/aap_eda/core/models/event_stream.py b/src/aap_eda/core/models/event_stream.py
@@ -16,7 +16,12 @@
 
 from django.db import models
 
-from aap_eda.core.enums import ActivationStatus, RestartPolicy
+from aap_eda.core.enums import (
+    ActivationStatus,
+    RestartPolicy,
+    RulebookProcessLogLevel,
+)
+from aap_eda.core.utils import get_default_log_level
 from aap_eda.services.activation.engine.common import ContainerableMixin
 
 from .mixins import StatusHandlerModelMixin
@@ -90,6 +95,11 @@ class EventStream(StatusHandlerModelMixin, ContainerableMixin, models.Model):
     credentials = models.ManyToManyField(
         "Credential", related_name="event_streams", default=None
     )
+    log_level = models.CharField(
+        max_length=20,
+        choices=RulebookProcessLogLevel.choices(),
+        default=get_default_log_level,
+    )
 
     class Meta:
         db_table = "core_event_stream"

diff --git a/src/aap_eda/core/utils/__init__.py b/src/aap_eda/core/utils/__init__.py
@@ -11,3 +11,11 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
+
+from django.conf import settings
+
+from aap_eda.core.enums import RulebookProcessLogLevel
+
+
+def get_default_log_level() -> RulebookProcessLogLevel:
+    return settings.ANSIBLE_RULEBOOK_LOG_LEVEL
diff --git a/src/aap_eda/services/activation/engine/common.py b/src/aap_eda/services/activation/engine/common.py
@@ -155,7 +155,7 @@ def get_command_line_parameters(self) -> dict[str, tp.Any]:
         return {
             "id": str(self.latest_instance.id),
             "ws_url": self._get_ws_url(),
-            "log_level": settings.ANSIBLE_RULEBOOK_LOG_LEVEL,
+            "log_level": self._get_log_level(),
             "ws_ssl_verify": settings.WEBSOCKET_SSL_VERIFY,
             "ws_token_url": self._get_ws_token_url(),
             "ws_access_token": access_token,
@@ -229,6 +229,15 @@ def _build_cmdline(self) -> AnsibleRulebookCmdLine:
             skip_audit_events=params["skip_audit_events"],
         )
 
+    def _get_log_level(self) -> tp.Optional[str]:
+        """Return the log level to use by ansible-rulebook."""
+        level_map = {
+            "debug": "-vv",
+            "info": "-v",
+            "error": None,
+        }
+        return level_map[self.log_level]
+
     def _get_container_name(self) -> str:
         """Return the name to use for the ContainerRequest."""
         return (

diff --git a/src/aap_eda/settings/default.py b/src/aap_eda/settings/default.py
@@ -76,6 +76,8 @@
 from django.core.exceptions import ImproperlyConfigured
 from split_settings.tools import include
 
+from aap_eda.core.enums import RulebookProcessLogLevel
+
 default_settings_file = "/etc/eda/settings.yaml"
 
 settings = dynaconf.Dynaconf(
@@ -450,7 +452,29 @@ def _get_secret_key() -> str:
 # ---------------------------------------------------------
 # RULEBOOK ENGINE LOG LEVEL
 # ---------------------------------------------------------
-ANSIBLE_RULEBOOK_LOG_LEVEL = settings.get("ANSIBLE_RULEBOOK_LOG_LEVEL", "-v")
+
+
+# For backwards compatibility, from the old value "-v" to the new value "info"
+def get_rulebook_process_log_level() -> RulebookProcessLogLevel:
+    log_level = settings.get(
+        "ANSIBLE_RULEBOOK_LOG_LEVEL",
+        "error",
+    )
+    if log_level is None:
+        return RulebookProcessLogLevel.ERROR
+    if log_level.lower() == "-v":
+        return RulebookProcessLogLevel.INFO
+    if log_level.lower() == "-vv":
+        return RulebookProcessLogLevel.DEBUG
+    if log_level not in RulebookProcessLogLevel.values():
+        raise ImproperlyConfigured(
+            f"Invalid log level '{log_level}' for ANSIBLE_RULEBOOK_LOG_LEVEL"
+            f" setting. Valid values are: {RulebookProcessLogLevel.values()}"
+        )
+    return RulebookProcessLogLevel(log_level)
+
+
+ANSIBLE_RULEBOOK_LOG_LEVEL = get_rulebook_process_log_level()
 ANSIBLE_RULEBOOK_FLUSH_AFTER = settings.get("ANSIBLE_RULEBOOK_FLUSH_AFTER", 1)
 
 # Experimental LDAP Integration https://issues.redhat.com/browse/AAP-16938

diff --git a/tests/integration/api/test_activation.py b/tests/integration/api/test_activation.py
@@ -43,6 +43,7 @@
     "restart_policy": RestartPolicy.ON_FAILURE,
     "restart_count": 0,
     "status_message": "",
+    "log_level": "debug",
 }
 
 TEST_AWX_TOKEN = {
@@ -220,6 +221,7 @@ def create_activation(fks: dict):
     activation_data["rulebook_id"] = fks["rulebook_id"]
     activation_data["extra_var_id"] = fks["extra_var_id"]
     activation_data["user_id"] = fks["user_id"]
+    activation_data["log_level"] = "debug"
     activation = models.Activation(**activation_data)
     activation.save()
 
@@ -268,6 +270,7 @@ def test_create_activation(client: APIClient):
         data,
         activation,
     )
+    assert data["log_level"] == "debug"
     if activation.project:
         assert data["project"] == {"id": activation.project.id, **TEST_PROJECT}
     else:

diff --git a/tests/integration/services/activation/test_activation.py b/tests/integration/services/activation/test_activation.py
@@ -56,7 +56,7 @@ def activation(activation_no_instance) -> models.Activation:
 def test_command_line_parameters(activation):
     params = activation.get_command_line_parameters()
     assert params["ws_url"] is not None
-    assert params["log_level"] is not None
+    assert params["log_level"] is None
     assert params["ws_ssl_verify"] is not None
     assert params["ws_token_url"] is not None
     assert params["ws_access_token"] is not None
@@ -101,3 +101,19 @@ def test_get_container_request_no_instance(activation_no_instance):
     """Test the construction of a ContainerRequest."""
     with pytest.raises(ContainerableInvalidError):
         activation_no_instance.get_container_request()
+
+
+@pytest.mark.parametrize(
+    "value,expected",
+    [
+        ("debug", "-vv"),
+        ("info", "-v"),
+        ("error", None),
+    ],
+)
+@pytest.mark.django_db
+def test_log_level_param_activation(activation, value, expected):
+    activation.log_level = value
+    activation.save(update_fields=["log_level"])
+    request = activation.get_container_request()
+    assert request.cmdline.log_level == expected
diff --git a/tests/integration/services/activation/test_event_stream.py b/tests/integration/services/activation/test_event_stream.py
@@ -56,7 +56,7 @@ def event_stream(event_stream_no_instance) -> models.EventStream:
 def test_command_line_parameters(event_stream):
     params = event_stream.get_command_line_parameters()
     assert params["ws_url"] is not None
-    assert params["log_level"] is not None
+    assert params["log_level"] is None
     assert params["ws_ssl_verify"] is not None
     assert params["ws_token_url"] is not None
     assert params["ws_access_token"] is not None
@@ -101,3 +101,19 @@ def test_get_container_request_no_instance(event_stream_no_instance):
     """Test the construction of a ContainerRequest."""
     with pytest.raises(ContainerableInvalidError):
         event_stream_no_instance.get_container_request()
+
+
+@pytest.mark.parametrize(
+    "value,expected",
+    [
+        ("debug", "-vv"),
+        ("info", "-v"),
+        ("error", None),
+    ],
+)
+@pytest.mark.django_db
+def test_log_level_param_event_stream(event_stream, value, expected):
+    event_stream.log_level = value
+    event_stream.save(update_fields=["log_level"])
+    request = event_stream.get_container_request()
+    assert request.cmdline.log_level == expected
diff --git a/tests/integration/services/activation/test_manager.py b/tests/integration/services/activation/test_manager.py
@@ -108,6 +108,7 @@ def basic_activation(
         rulebook=default_rulebook,
         # rulebook_rulesets is populated by the serializer
         rulebook_rulesets=default_rulebook.rulesets,
+        log_level="info",
     )
 
 
@@ -146,7 +147,6 @@ def test_get_container_request(
     """Test build_cmdline."""
     override_settings = {
         "WEBSOCKET_BASE_URL": "ws://localhost:8000",
-        "ANSIBLE_RULEBOOK_LOG_LEVEL": "-vv",
         "WEBSOCKET_SSL_VERIFY": "no",
         "RULEBOOK_LIVENESS_CHECK_SECONDS": 73,
     }
@@ -156,7 +156,7 @@ def test_get_container_request(
     assert isinstance(request, ContainerRequest)
     cmdline = request.cmdline
     assert cmdline.ws_url.startswith(override_settings["WEBSOCKET_BASE_URL"])
-    assert cmdline.log_level == override_settings["ANSIBLE_RULEBOOK_LOG_LEVEL"]
+    assert cmdline.log_level == "-v"
     assert cmdline.ws_ssl_verify == override_settings["WEBSOCKET_SSL_VERIFY"]
     assert (
         cmdline.heartbeat