chore(bors): merge pull request #692

692: feat(drain): disallow node draining if ha is disabled r=Abhinandan-Purkait a=Abhinandan-Purkait

Node Drain relies on the HA feature to move and reconnect the nexus. So if HA is disabled node drain will not work and can leave the volumes in an inconsistent state. 

This PR restricts node draining if the HA is not enabled.

Co-authored-by: Abhinandan Purkait <purkaitabhinandan@gmail.com>

control-plane/agents/src/bin/core/controller/reconciler/node/nexus.rs

@@ -163,6 +163,11 @@ async fn check_and_drain_node(
     context: &PollContext,
     node_spec: &mut OperationGuardArc<NodeSpec>,
 ) -> PollResult {
+    // Drain should not be allowed if HA is not enabled.
+    if context.registry().ha_disabled() {
+        return PollResult::Err(SvcError::DrainNotAllowedWhenHAisDisabled {});
+    }
+
     if !node_spec.as_ref().is_draining() {
         return PollResult::Ok(PollerState::Idle);
     }

control-plane/agents/src/bin/core/controller/registry.rs

@@ -98,6 +98,8 @@ pub(crate) struct RegistryInner<S: Store> {
     legacy_prefix_present: bool,
     /// Thin provisioning parameters.
     thin_args: ThinArgs,
+    /// Check if the HA feature is enabled.
+    ha_disabled: bool,
 }
 
 impl Registry {
@@ -118,6 +120,7 @@ impl Registry {
         create_volume_limit: usize,
         host_acl: Vec<HostAccessControl>,
         thin_args: ThinArgs,
+        ha_enabled: bool,
     ) -> Result<Self, SvcError> {
         let store_endpoint = Self::format_store_endpoint(&store_url);
         tracing::info!("Connecting to persistent store at {}", store_endpoint);
@@ -167,6 +170,7 @@ impl Registry {
                 host_acl,
                 legacy_prefix_present,
                 thin_args,
+                ha_disabled: ha_enabled,
             }),
         };
         registry.init().await?;
@@ -193,6 +197,11 @@ impl Registry {
         Ok(registry)
     }
 
+    /// Check if the HA feature is enabled.
+    pub(crate) fn ha_disabled(&self) -> bool {
+        self.ha_disabled
+    }
+
     /// Formats the store endpoint with a default port if one isn't supplied.
     fn format_store_endpoint(endpoint: &str) -> String {
         match endpoint.contains(':') {

control-plane/agents/src/bin/core/main.rs

@@ -103,6 +103,11 @@ pub(crate) struct CliArgs {
     /// Events message-bus endpoint url.
     #[clap(long, short)]
     events_url: Option<url::Url>,
+
+    /// Disable the HA/Failover feature.
+    /// This is useful when the frontend nodes do not support the NVMe ANA feature.
+    #[clap(long, env = "HA_DISABLED")]
+    pub(crate) disable_ha: bool,
 }
 impl CliArgs {
     fn args() -> Self {
@@ -172,6 +177,7 @@ async fn server(cli_args: CliArgs) -> anyhow::Result<()> {
             cli_args.hosts_acl.clone()
         },
         cli_args.thin_args,
+        cli_args.disable_ha,
     )
     .await?;
 

control-plane/agents/src/bin/core/node/service.rs

@@ -361,6 +361,11 @@ impl Service {
 
     /// Apply a drain label to the specified node. The reconciler will perform the drain.
     async fn drain(&self, id: NodeId, label: String) -> Result<Node, SvcError> {
+        // Don't allow draining if HA_ENABLED is false. If it is undefined we treat it as true.
+        if self.registry.ha_disabled() {
+            return Err(SvcError::DrainNotAllowedWhenHAisDisabled {});
+        }
+
         let mut guarded_node = self.specs().guarded_node(&id).await?;
 
         let spec = guarded_node.drain(&self.registry, label.clone()).await?;

control-plane/agents/src/bin/core/volume/operations.rs

@@ -379,6 +379,10 @@ impl ResourcePublishing for OperationGuardArc<VolumeSpec> {
         registry: &Registry,
         request: &Self::Republish,
     ) -> Result<Self::PublishOutput, SvcError> {
+        // If HA is disabled there is no point in switchover.
+        if registry.ha_disabled() {
+            return Err(SvcError::SwitchoverNotAllowedWhenHAisDisabled {});
+        }
         let specs = registry.specs();
         let spec = self.as_ref().clone();
         let state = registry.volume_state(&request.uuid).await?;

control-plane/agents/src/bin/core/volume/service.rs

@@ -362,6 +362,10 @@ impl Service {
         &self,
         request: &RepublishVolume,
     ) -> Result<Volume, SvcError> {
+        // If HA is disabled there is no point in switchover.
+        if self.registry.ha_disabled() {
+            return Err(SvcError::SwitchoverNotAllowedWhenHAisDisabled {});
+        }
         let mut volume = self.specs().volume(&request.uuid).await?;
         volume.republish(&self.registry, request).await
     }

control-plane/agents/src/common/errors.rs

@@ -330,6 +330,10 @@ pub enum SvcError {
     ClonedSnapshotVolumeRepl {},
     #[snafu(display("The source snapshot is not created"))]
     SnapshotNotCreated {},
+    #[snafu(display("Draining is not allowed without HA"))]
+    DrainNotAllowedWhenHAisDisabled {},
+    #[snafu(display("Target switchover is not allowed without HA"))]
+    SwitchoverNotAllowedWhenHAisDisabled {},
 }
 
 impl SvcError {
@@ -918,6 +922,18 @@ impl From<SvcError> for ReplyError {
                 source,
                 extra,
             },
+            SvcError::DrainNotAllowedWhenHAisDisabled {} => ReplyError {
+                kind: ReplyErrorKind::FailedPrecondition,
+                resource: ResourceKind::Node,
+                source,
+                extra,
+            },
+            SvcError::SwitchoverNotAllowedWhenHAisDisabled {} => ReplyError {
+                kind: ReplyErrorKind::FailedPrecondition,
+                resource: ResourceKind::Nexus,
+                source,
+                extra,
+            },
         }
     }
 }