fix: stop agent pebble svc not exist error & validate credentials err…

…or (#39)

* fix: stop agent pebble svc not exist error

* fix: don't validate agent relation credentials

* chore: check service exist before stopping

src/agent.py

@@ -207,22 +207,6 @@ def _on_agent_relation_changed(self, event: ops.RelationChangedEvent) -> None:
             logger.error("Failed to download Jenkins agent executable, %s", exc)
             raise RuntimeError("Failed to download Jenkins agent.") from exc
 
-        self.charm.unit.status = ops.MaintenanceStatus("Validating credentials.")
-        if not server.validate_credentials(
-            agent_name=self.state.agent_meta.name,
-            credentials=self.state.agent_relation_credentials,
-            container=container,
-        ):
-            # The jenkins server sets credentials one by one, hence if the current credentials are
-            # not for this particular agent, the agent operator should wait until it receives one
-            # designated for it.
-            logger.warning(
-                "Failed credential for agent %s, will wait for next credentials to be set",
-                self.state.agent_meta.name,
-            )
-            self.charm.unit.status = ops.WaitingStatus("Waiting for credentials.")
-            return
-
         self.charm.unit.status = ops.MaintenanceStatus("Starting agent pebble service.")
         self.pebble_service.reconcile(
             server_url=self.state.agent_relation_credentials.address,

src/pebble.py

@@ -90,5 +90,11 @@ def stop_agent(self, container: ops.Container) -> None:
         Args:
             container: The agent workload container.
         """
+        try:
+            # use get_service to check if service should be stopped rather than stopping and
+            # catching ops.pebble.APIError and parsing error message to determine type of error.
+            container.get_service(self.state.jenkins_agent_service_name)
+        except ops.ModelError:
+            return
         container.stop(self.state.jenkins_agent_service_name)
         container.remove_path(str(server.AGENT_READY_PATH))

tests/unit/test_agent.py

@@ -271,17 +271,9 @@ def test_agent_relation_changed_download_jenkins_agent_fail(
         assert exc.value == "Failed to download Jenkins agent executable."
 
 
-@pytest.mark.parametrize(
-    "relation",
-    [
-        pytest.param(state.AGENT_RELATION, id="agent relation"),
-        pytest.param(state.SLAVE_RELATION, id="slave relation"),
-    ],
-)
 def test_agent_relation_changed_validate_credentials_fail(
     monkeypatch: pytest.MonkeyPatch,
     harness: ops.testing.Harness,
-    relation: str,
     get_event_relation_data: typing.Callable[
         [str], typing.Tuple[unittest.mock.MagicMock, typing.Dict[str, str]]
     ],
@@ -291,11 +283,11 @@ def test_agent_relation_changed_validate_credentials_fail(
     act: when _on_agent_relation_changed is called.
     assert: the unit falls into WaitingStatus.
     """
-    (mock_event, relation_data) = get_event_relation_data(relation)
+    (mock_event, relation_data) = get_event_relation_data(state.SLAVE_RELATION)
     monkeypatch.setattr(server, "download_jenkins_agent", lambda *_args, **_kwargs: None)
     monkeypatch.setattr(server, "validate_credentials", lambda *_args, **_kwargs: False)
     harness.set_can_connect("jenkins-k8s-agent", True)
-    relation_id = harness.add_relation(relation, "jenkins")
+    relation_id = harness.add_relation(state.SLAVE_RELATION, "jenkins")
     harness.add_relation_unit(relation_id, "jenkins/0")
     harness.update_relation_data(
         relation_id=relation_id,
@@ -305,10 +297,7 @@ def test_agent_relation_changed_validate_credentials_fail(
     harness.begin()
 
     jenkins_charm = typing.cast(JenkinsAgentCharm, harness.charm)
-    if relation == state.AGENT_RELATION:
-        jenkins_charm.agent_observer._on_agent_relation_changed(mock_event)
-    else:
-        jenkins_charm.agent_observer._on_slave_relation_changed(mock_event)
+    jenkins_charm.agent_observer._on_slave_relation_changed(mock_event)
 
     assert jenkins_charm.unit.status.name == WAITING_STATUS_NAME
     assert jenkins_charm.unit.status.message == "Waiting for credentials."

tests/unit/test_pebble.py

@@ -67,6 +67,24 @@ def test_reconcile():
     mock_container.replan.assert_called_once()
 
 
+def test_stop_agent_service_not_exists():
+    """
+    arrange: given a monkeypatched container that raises pebble API service not exists error.
+    act: when stop_agent is called.
+    assert: nothing happens since the service was not started.
+    """
+    mock_state = unittest.mock.MagicMock(spec=state.State)
+    mock_state.jenkins_agent_service_name = state.State.jenkins_agent_service_name
+    mock_container = unittest.mock.MagicMock(spec=ops.Container)
+    mock_container.get_service.side_effect = [ops.ModelError()]
+    pebble_service = pebble.PebbleService(state=mock_state)
+
+    pebble_service.stop_agent(container=mock_container)
+
+    mock_container.stop.assert_not_called()
+    mock_container.remove_path.assert_not_called()
+
+
 def test_stop_agent():
     """
     arrange: given a monkeypatched _jenkins_agent_container representing non connectable container.