Make operator config optional (metalbear-co#2100)

* Made 'config.operator' optional, ignoring operator errors when the operator is not required

* Changelog entry

* Schema updated

* Default value removed

* Progress fix

* Fixed comment format

* Fixed comments

changelog.d/+operator-bogus-errors.fixed.md

@@ -0,0 +1 @@
+Changed `operator` config to be optional. If the option is set to `true`, mirrord always uses the operator and aborts in case of failure. If the option is set to `false`, mirrord does not attempt to use the operator. If the option is not set at all, mirrord attempts to use the operator, but does not abort in case it could not be found.

mirrord-schema.json

@@ -71,7 +71,7 @@
     },
     "operator": {
       "title": "operator {#root-operator}",
-      "description": "Allow to lookup if operator is installed on cluster and use it.\n\nDefaults to `true`.",
+      "description": "Whether mirrord should use the operator. If not set, mirrord will first attempt to use the operator, but continue without it in case of failure.",
       "type": [
         "boolean",
         "null"

mirrord/cli/src/connection.rs

@@ -4,7 +4,7 @@ use mirrord_analytics::AnalyticsReporter;
 use mirrord_config::{feature::network::outgoing::OutgoingFilterConfig, LayerConfig};
 use mirrord_intproxy::agent_conn::AgentConnectInfo;
 use mirrord_kube::api::{kubernetes::KubernetesAPI, AgentManagment};
-use mirrord_operator::client::OperatorApi;
+use mirrord_operator::client::{OperatorApi, OperatorApiError};
 use mirrord_progress::Progress;
 use mirrord_protocol::{ClientMessage, DaemonMessage};
 use tokio::sync::mpsc;
@@ -16,6 +16,27 @@ pub(crate) struct AgentConnection {
     pub receiver: mpsc::Receiver<DaemonMessage>,
 }
 
+trait OperatorApiErrorExt {
+    /// Whether this error should abort the execution, even if the user did not specify whether to
+    /// use the operator or not.
+    fn should_abort_cli(&self) -> bool;
+}
+
+impl OperatorApiErrorExt for OperatorApiError {
+    fn should_abort_cli(&self) -> bool {
+        match self {
+            // Various kube errors can happen due to RBAC if the operator is not installed.
+            Self::KubeError { .. } => false,
+            // These should either never happen or can happen only if the operator is installed.
+            Self::ConcurrentStealAbort
+            | Self::ConnectRequestBuildError(..)
+            | Self::CreateApiError(..)
+            | Self::InvalidTarget { .. }
+            | Self::UnsupportedFeature { .. } => true,
+        }
+    }
+}
+
 /// Creates an agent if needed then connects to it.
 pub(crate) async fn create_and_connect<P>(
     config: &LayerConfig,
@@ -37,11 +58,11 @@ where
         }
     }
 
-    if config.operator {
+    if config.operator != Some(false) {
         let mut subtask = progress.subtask("checking operator");
 
-        match OperatorApi::create_session(config, &subtask, analytics).await? {
-            Some(session) => {
+        match OperatorApi::create_session(config, &subtask, analytics).await {
+            Ok(session) => {
                 subtask.success(Some("connected to the operator"));
                 return Ok((
                     AgentConnectInfo::Operator(session.info),
@@ -51,7 +72,10 @@ where
                     },
                 ));
             }
-            None => subtask.success(Some("no operator detected")),
+            Err(e) if config.operator == Some(true) || e.should_abort_cli() => return Err(e.into()),
+            Err(e) => {
+                subtask.failure(Some(&format!("connecting to the operator failed: {e}")));
+            }
         }
     }
 

mirrord/config/src/lib.rs

@@ -227,11 +227,11 @@ pub struct LayerConfig {
 
     /// ## operator {#root-operator}
     ///
-    /// Allow to lookup if operator is installed on cluster and use it.
-    ///
-    /// Defaults to `true`.
-    #[config(env = "MIRRORD_OPERATOR_ENABLE", default = true)]
-    pub operator: bool,
+    /// Whether mirrord should use the operator.
+    /// If not set, mirrord will first attempt to use the operator, but continue without it in case
+    /// of failure.
+    #[config(env = "MIRRORD_OPERATOR_ENABLE")]
+    pub operator: Option<bool>,
 
     /// ## kubeconfig {#root-kubeconfig}
     ///
@@ -434,7 +434,7 @@ impl LayerConfig {
         }
 
         if self.feature.copy_target.enabled {
-            if !self.operator {
+            if self.operator == Some(false) {
                 return Err(ConfigError::Conflict(
                     "The copy target feature requires a mirrord operator, \
                    please either disable this option or use the operator."

mirrord/operator/src/client.rs

@@ -2,7 +2,7 @@ use std::io;
 
 use base64::{engine::general_purpose, Engine as _};
 use futures::{SinkExt, StreamExt};
-use http::{request::Request, StatusCode};
+use http::request::Request;
 use kube::{api::PostParams, Api, Client, Resource};
 use mirrord_analytics::{AnalyticsHash, AnalyticsOperatorProperties, AnalyticsReporter};
 use mirrord_auth::{certificate::Certificate, credential_store::CredentialStoreSync};
@@ -171,21 +171,17 @@ impl OperatorApi {
     }
 
     /// Creates new [`OperatorSessionConnection`] based on the given [`LayerConfig`].
-    /// Returns [`None`] if the operator is not found.
     pub async fn create_session<P>(
         config: &LayerConfig,
         progress: &P,
         analytics: &mut AnalyticsReporter,
-    ) -> Result<Option<OperatorSessionConnection>>
+    ) -> Result<OperatorSessionConnection>
     where
         P: Progress + Send + Sync,
     {
         let operator_api = OperatorApi::new(config).await?;
 
-        let Some(operator) = operator_api.fetch_operator().await? else {
-            // No operator found.
-            return Ok(None);
-        };
+        let operator = operator_api.fetch_operator().await?;
 
         Self::check_config(config, &operator)?;
 
@@ -265,7 +261,7 @@ impl OperatorApi {
         };
         let connection = operator_api.connect_target(session_info).await?;
 
-        Ok(Some(connection))
+        Ok(connection)
     }
 
     /// Connects to exisiting operator session based on the given [`LayerConfig`] and
@@ -319,16 +315,14 @@ impl OperatorApi {
         })
     }
 
-    async fn fetch_operator(&self) -> Result<Option<MirrordOperatorCrd>> {
+    async fn fetch_operator(&self) -> Result<MirrordOperatorCrd> {
         let api: Api<MirrordOperatorCrd> = Api::all(self.client.clone());
-        match api.get(OPERATOR_STATUS_NAME).await {
-            Ok(crd) => Ok(Some(crd)),
-            Err(kube::Error::Api(e)) if e.code == StatusCode::NOT_FOUND => Ok(None),
-            Err(error) => Err(OperatorApiError::KubeError {
+        api.get(OPERATOR_STATUS_NAME)
+            .await
+            .map_err(|error| OperatorApiError::KubeError {
                 error,
                 operation: "finding operator in the cluster".into(),
-            }),
-        }
+            })
     }
 
     async fn fetch_target(&self) -> Result<TargetCrd> {