Retry install event on certain errors

src-docs/charm.py.md

@@ -12,7 +12,7 @@ Charm for creating and managing GitHub self-hosted runner instances.
 
 ---
 
-<a href="../src/charm.py#L77"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L79"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `catch_charm_errors`
 
@@ -38,7 +38,7 @@ Catch common errors in charm.
 
 ---
 
-<a href="../src/charm.py#L112"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L120"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `catch_action_errors`
 
@@ -67,7 +67,7 @@ Catch common errors in actions.
 ## <kbd>class</kbd> `GithubRunnerCharm`
 Charm for managing GitHub self-hosted runners. 
 
-<a href="../src/charm.py#L157"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L165"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>function</kbd> `__init__`
 

src-docs/charm_state.py.md

@@ -164,7 +164,7 @@ The charm state.
 
 ---
 
-<a href="../src/charm_state.py#L573"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L605"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -252,6 +252,39 @@ Return a string representing the path.
   Path to the GitHub entity. 
 
 
+---
+
+## <kbd>class</kbd> `HttpProxyEnvVar`
+HTTP proxy environment variables. 
+
+Validation is not performed. Intend for propagating the environment variable as is to services used by the charm. 
+
+Used for access the proxy env vars where the state should not be accessed. The state can throw errors based on configuration. Code that should be performed regardless of configuration issues should use this class to access the proxy, e.g., install dependencies. 
+
+
+
+**Attributes:**
+
+ - <b>`http_proxy`</b>:  HTTP proxy environment variable. 
+ - <b>`https_proxy`</b>:  HTTPS proxy environment variable. 
+ - <b>`no_proxy`</b>:  No proxy environment variable. 
+
+
+
+
+---
+
+<a href="../src/charm_state.py#L408"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+
+### <kbd>classmethod</kbd> `from_env`
+
+```python
+from_env() → HttpProxyEnvVar
+```
+
+Initialize the HTTP proxy environment variable from env. 
+
+
 ---
 
 ## <kbd>class</kbd> `ProxyConfig`
@@ -261,8 +294,8 @@ Proxy configuration.
 
 **Attributes:**
 
- - <b>`http`</b>:  HTTP proxy address. 
- - <b>`https`</b>:  HTTPS proxy address. 
+ - <b>`http_proxy`</b>:  HTTP proxy address. 
+ - <b>`https_proxy`</b>:  HTTPS proxy address. 
  - <b>`no_proxy`</b>:  Comma-separated list of hosts that should not be proxied. 
  - <b>`use_aproxy`</b>:  Whether aproxy should be used for the runners. 
 
@@ -277,7 +310,7 @@ Return the aproxy address.
 
 ---
 
-<a href="../src/charm_state.py#L440"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L472"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `check_fields`
 
@@ -300,7 +333,7 @@ Validate the proxy configuration.
 
 ---
 
-<a href="../src/charm_state.py#L402"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L433"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -413,7 +446,7 @@ SSH connection information for debug workflow.
 
 ---
 
-<a href="../src/charm_state.py#L520"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L552"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -441,7 +474,7 @@ Raised when given machine charm architecture is unsupported.
 
  - <b>`arch`</b>:  The current machine architecture. 
 
-<a href="../src/charm_state.py#L477"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L509"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>function</kbd> `__init__`
 

src-docs/errors.py.md

@@ -7,6 +7,15 @@ Errors used by the charm.
 
 
 
+---
+
+## <kbd>class</kbd> `CharmInstallError`
+Represents failure in charm installation. 
+
+
+
+
+
 ---
 
 ## <kbd>class</kbd> `ConfigurationError`

src/charm.py

@@ -38,12 +38,14 @@
     CharmConfigInvalidError,
     CharmState,
     GithubPath,
+    HttpProxyEnvVar,
     ProxyConfig,
     RunnerStorage,
     VirtualMachineResources,
     parse_github_path,
 )
 from errors import (
+    CharmInstallError,
     ConfigurationError,
     LogrotateSetupError,
     MissingRunnerBinaryError,
@@ -90,6 +92,11 @@ def func_with_catch_errors(self, event: EventT) -> None:
             func(self, event)
         except ConfigurationError as err:
             logger.exception("Issue with charm configuration")
+
+            if isinstance(event, InstallEvent):
+                # Enter error state to let juju retry the install event.
+                raise
+
             self.unit.status = BlockedStatus(str(err))
         except TokenError as err:
             logger.exception("Issue with GitHub token")
@@ -105,6 +112,7 @@ def func_with_catch_errors(self, event: EventT) -> None:
             self.unit.status = BlockedStatus(
                 "Unauthorized OpenStack connection. Check credentials."
             )
+        # Intentionally not catching `CharmInstallError` to let juju retry install related issues.
 
     return func_with_catch_errors
 
@@ -342,24 +350,23 @@ def _on_install(self, _event: InstallEvent) -> None:
         Args:
             event: Event of installing the charm.
         """
-        state = self._setup_state()
-
         self.unit.status = MaintenanceStatus("Installing packages")
         try:
-            # The `_start_services`, `_install_deps` includes retry.
             self._install_deps()
-            self._start_services(state.charm_config.token, state.proxy_config)
             metrics.setup_logrotate()
         except (LogrotateSetupError, SubprocessError) as err:
             logger.exception(err)
             if isinstance(err, LogrotateSetupError):
                 msg = "Failed to setup logrotate"
             else:
                 msg = "Failed to install dependencies"
-            self.unit.status = BlockedStatus(msg)
-            return
+            # Retry install by going into error state.
+            raise CharmInstallError(msg) from err
 
-        self._refresh_firewall(state)
+        # Above this line, retry are done by entering error state.
+        # Below this line, retry is performed by other events, e.g., config_change.
+
+        state = self._setup_state()
         runner_manager = self._get_runner_manager(state)
 
         self.unit.status = MaintenanceStatus("Building runner image")
@@ -391,6 +398,8 @@ def _on_start(self, _event: StartEvent) -> None:
         Args:
             event: Event of starting the charm.
         """
+        # Start event does not require retry on failure currently as it only perform the same
+        # actions as the reconcile event. Therefore the reconcile event serves as a retry.
         state = self._setup_state()
         runner_manager = self._get_runner_manager(state)
 
@@ -457,13 +466,9 @@ def _on_upgrade_charm(self, _event: UpgradeCharmEvent) -> None:
         Args:
             event: Event of charm upgrade.
         """
-        state = self._setup_state()
-
         logger.info("Reinstalling dependencies...")
         try:
-            # The `_start_services`, `_install_deps` includes retry.
             self._install_deps()
-            self._start_services(state.charm_config.token, state.proxy_config)
             metrics.setup_logrotate()
         except (LogrotateSetupError, SubprocessError) as err:
             logger.exception(err)
@@ -472,12 +477,14 @@ def _on_upgrade_charm(self, _event: UpgradeCharmEvent) -> None:
                 msg = "Failed to setup logrotate"
             else:
                 msg = "Failed to install dependencies"
-            self.unit.status = BlockedStatus(msg)
-            return
+            # Retry the install by going into error state.
+            raise CharmInstallError(msg) from err
+
+        # Above this line, retry are done by entering error state.
+        # Below this line, retry is performed by other events, e.g., config_change.
 
-        state = self._setup_state()
-        self._refresh_firewall(state)
         logger.info("Flushing the runners...")
+        state = self._setup_state()
         runner_manager = self._get_runner_manager(state)
         if not runner_manager:
             return
@@ -498,10 +505,12 @@ def _on_config_changed(self, _event: ConfigChangedEvent) -> None:
         """
         state = self._setup_state()
         self._set_reconcile_timer()
+        self._refresh_firewall(state)
 
         prev_config_for_flush: dict[str, str] = {}
         if state.charm_config.token != self._stored.token:
             prev_config_for_flush["token"] = str(self._stored.token)
+            # The runners should be flushed at some point after `_start_services` is called.
             self._start_services(state.charm_config.token, state.proxy_config)
             self._stored.token = None
         if self.config["path"] != self._stored.path:
@@ -516,9 +525,6 @@ def _on_config_changed(self, _event: ConfigChangedEvent) -> None:
                 # it may be the case that the prev token has expired, so we need to use force flush
                 prev_runner_manager.flush(FlushMode.FORCE_FLUSH_WAIT_REPO_CHECK)
 
-        state = self._setup_state()
-        self._refresh_firewall(state)
-
         if state.charm_config.openstack_clouds_yaml:
             # Test out openstack integration and then go
             # into BlockedStatus as it is not supported yet
@@ -585,7 +591,7 @@ def _check_and_update_dependencies(
             runner_bin_updated = True
 
         self.unit.status = MaintenanceStatus("Checking for service updates")
-        service_updated = self._install_repo_policy_compliance(proxy_config)
+        service_updated = self._install_repo_policy_compliance(HttpProxyEnvVar.from_env())
 
         if service_updated or runner_bin_updated:
             logger.info(
@@ -762,25 +768,25 @@ def _reconcile_runners(
         self.unit.status = ActiveStatus()
         return {"delta": {"virtual-machines": delta_virtual_machines}}
 
-    def _install_repo_policy_compliance(self, proxy_config: ProxyConfig) -> bool:
-        """Install latest version of repo_policy_compliance service.
+    def _install_repo_policy_compliance(self, proxies: HttpProxyEnvVar) -> bool:
+        """Install or upgrade to latest version of repo_policy_compliance service.
 
         Args:
-            proxy_config: Proxy configuration.
+            proxies: HTTP proxy environment variables.
 
         Returns:
             Whether version install is changed. Going from not installed to
             installed will return True.
         """
         # Prepare environment variables for pip subprocess
         env = {}
-        if http_proxy := proxy_config.http:
+        if http_proxy := proxies.http_proxy:
             env["HTTP_PROXY"] = http_proxy
             env["http_proxy"] = http_proxy
-        if https_proxy := proxy_config.https:
+        if https_proxy := proxies.https_proxy:
             env["HTTPS_PROXY"] = https_proxy
             env["https_proxy"] = https_proxy
-        if no_proxy := proxy_config.no_proxy:
+        if no_proxy := proxies.no_proxy:
             env["NO_PROXY"] = no_proxy
             env["no_proxy"] = no_proxy
 
@@ -828,15 +834,13 @@ def _enable_kernel_modules(self) -> None:
     @retry(tries=5, delay=5, max_delay=60, backoff=2, local_logger=logger)
     def _install_deps(self) -> None:
         """Install dependencies."""
-        state = self._setup_state()
-
         logger.info("Installing charm dependencies.")
         # Snap and Apt will use any proxies configured in the Juju model.
         # Binding for snap, apt, and lxd init commands are not available so subprocess.run used.
         # Install dependencies used by repo-policy-compliance and the firewall
         self._apt_install(["gunicorn", "python3-pip", "nftables"])
         # Install repo-policy-compliance package
-        self._install_repo_policy_compliance(state.proxy_config)
+        self._install_repo_policy_compliance(HttpProxyEnvVar.from_env())
         execute_command(
             ["/usr/bin/apt-get", "remove", "-qy", "lxd", "lxd-client"], check_exit=False
         )