chore(bors): merge pull request #483

483: Add failure for when partial rebuild is enabled for source charts >=2.2,<=2.5 r=niladrih a=niladrih



Co-authored-by: Niladri Halder <niladri.halder26@gmail.com>

k8s/upgrade/src/bin/upgrade-job/common/constants.rs

@@ -1,3 +1,5 @@
+use semver::Version;
+
 /// This is the name of the project that is being upgraded.
 pub(crate) const PRODUCT: &str = "Mayastor";
 
@@ -30,6 +32,14 @@ pub(crate) const TO_UMBRELLA_SEMVER: &str = "4.0.0";
 pub(crate) const UMBRELLA_CHART_UPGRADE_DOCS_URL: &str =
     "https://openebs.io/docs/user-guides/upgrade#mayastor-upgrade";
 
+/// This is the limit for the number of objects we want to collect over the network from
+/// the kubernetes api.
+pub(crate) const KUBE_API_PAGE_SIZE: u32 = 500;
+
+/// The Core chart version limits for requiring partial rebuild to be disabled for upgrade.
+pub(crate) const PARTIAL_REBUILD_DISABLE_EXTENTS: (Version, Version) =
+    (Version::new(2, 2, 0), Version::new(2, 5, 0));
+
 /// Version value for the earliest possible 2.0 release.
 pub(crate) const TWO_DOT_O_RC_ONE: &str = "2.0.0-rc.1";
 

k8s/upgrade/src/bin/upgrade-job/common/error.rs

@@ -202,20 +202,6 @@ pub(crate) enum Error {
         pod_namespace: String,
     },
 
-    /// Error for when a Kubernetes API request for GET-ing a list of Pods filtered by label(s)
-    /// fails.
-    #[snafu(display(
-        "Failed to list Pods with label {} in namespace {}: {}",
-        label,
-        namespace,
-        source
-    ))]
-    ListPodsWithLabel {
-        source: kube::Error,
-        label: String,
-        namespace: String,
-    },
-
     /// Error for when a Kubernetes API request for GET-ing a list of Nodes filtered by label(s)
     /// fails.
     #[snafu(display("Failed to list Nodes with label {}: {}", label, source))]
@@ -491,23 +477,6 @@ pub(crate) enum Error {
     #[snafu(display("Failed to send Event over the channel"))]
     EventChannelSend,
 
-    /// Error for when the no value for version label is found on the helm chart.
-    #[snafu(display(
-        "Failed to get the value of the {} label in Pod {} in Namespace {}",
-        CHART_VERSION_LABEL_KEY,
-        pod_name,
-        namespace
-    ))]
-    HelmChartVersionLabelHasNoValue { pod_name: String, namespace: String },
-
-    /// Error for when a pod does not have Namespace set on it.
-    #[snafu(display(
-        "Found None when trying to get Namespace for Pod {}, context: {}",
-        pod_name,
-        context
-    ))]
-    NoNamespaceInPod { pod_name: String, context: String },
-
     /// Error for the Umbrella chart is not upgraded.
     #[snafu(display(
         "The {} helm chart is not upgraded to version {}: Upgrade for helm chart {} is not \
@@ -694,6 +663,13 @@ pub(crate) enum Error {
 
     #[snafu(display("failed to list CustomResourceDefinitions: {source}"))]
     ListCrds { source: kube::Error },
+
+    #[snafu(display("Partial rebuild must be disabled for upgrades from {chart_name} chart versions >= {lower_extent}, <= {upper_extent}"))]
+    PartialRebuildNotAllowed {
+        chart_name: String,
+        lower_extent: String,
+        upper_extent: String,
+    },
 }
 
 /// A wrapper type to remove repeated Result<T, Error> returns.

k8s/upgrade/src/bin/upgrade-job/common/kube_client.rs

@@ -1,12 +1,18 @@
-use crate::common::error::{K8sClientGeneration, Result};
+use crate::common::{
+    constants::KUBE_API_PAGE_SIZE,
+    error::{K8sClientGeneration, ListPodsWithLabelAndField, Result},
+};
 use k8s_openapi::{
     api::{
         apps::v1::Deployment,
         core::v1::{Namespace, Node, Pod},
     },
     apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition,
 };
-use kube::{api::Api, Client};
+use kube::{
+    api::{Api, ListParams},
+    Client,
+};
 use snafu::ResultExt;
 
 /// Generate a new kube::Client.
@@ -38,3 +44,46 @@ pub(crate) async fn pods_api(namespace: &str) -> Result<Api<Pod>> {
 pub(crate) async fn deployments_api(namespace: &str) -> Result<Api<Deployment>> {
     Ok(Api::namespaced(client().await?, namespace))
 }
+
+pub(crate) async fn list_pods(
+    namespace: String,
+    label_selector: Option<String>,
+    field_selector: Option<String>,
+) -> Result<Vec<Pod>> {
+    let mut pods: Vec<Pod> = Vec::with_capacity(KUBE_API_PAGE_SIZE as usize);
+
+    let mut list_params = ListParams::default().limit(KUBE_API_PAGE_SIZE);
+    if let Some(ref labels) = label_selector {
+        list_params = list_params.labels(labels.as_str());
+    }
+    if let Some(ref fields) = field_selector {
+        list_params = list_params.fields(fields.as_str());
+    }
+
+    let list_pods_error_ctx = ListPodsWithLabelAndField {
+        label: label_selector.unwrap_or_default(),
+        field: field_selector.unwrap_or_default(),
+        namespace: namespace.clone(),
+    };
+
+    loop {
+        let pod_list = pods_api(namespace.as_str())
+            .await?
+            .list(&list_params)
+            .await
+            .context(list_pods_error_ctx.clone())?;
+
+        let continue_ = pod_list.metadata.continue_.clone();
+
+        pods = pods.into_iter().chain(pod_list).collect();
+
+        match continue_ {
+            Some(token) => {
+                list_params = list_params.continue_token(token.as_str());
+            }
+            None => break,
+        }
+    }
+
+    Ok(pods)
+}

k8s/upgrade/src/bin/upgrade-job/helm/chart.rs

@@ -31,6 +31,8 @@ impl Chart {
 pub(crate) trait HelmValuesCollection {
     /// This is a getter for state of the 'ha' feature (enabled/disabled).
     fn ha_is_enabled(&self) -> bool;
+    /// This is a getter for the partial-rebuild toggle value.
+    fn partial_rebuild_is_enabled(&self) -> bool;
 }
 
 /// UmbrellaValues is used to deserialize the helm values.yaml for the Umbrella chart. The Core
@@ -56,6 +58,10 @@ impl HelmValuesCollection for UmbrellaValues {
     fn ha_is_enabled(&self) -> bool {
         self.core.ha_is_enabled()
     }
+
+    fn partial_rebuild_is_enabled(&self) -> bool {
+        self.core.partial_rebuild_is_enabled()
+    }
 }
 
 /// This is used to deserialize the values.yaml of the Core chart.
@@ -114,14 +120,13 @@ impl HelmValuesCollection for CoreValues {
     fn ha_is_enabled(&self) -> bool {
         self.agents.ha_is_enabled()
     }
-}
 
-impl CoreValues {
-    /// This is a getter for state of the 'ha' feature (enabled/disabled).
-    pub(crate) fn ha_is_enabled(&self) -> bool {
-        self.agents.ha_is_enabled()
+    fn partial_rebuild_is_enabled(&self) -> bool {
+        self.agents.partial_rebuild_is_enabled()
     }
+}
 
+impl CoreValues {
     /// This is a getter for the container image tag of the Core chart.
     pub(crate) fn image_tag(&self) -> &str {
         self.image.tag()
@@ -302,6 +307,7 @@ impl CoreValues {
 /// This is used to deserialize the yaml object agents.
 #[derive(Deserialize)]
 struct Agents {
+    core: Core,
     ha: Ha,
 }
 
@@ -310,6 +316,10 @@ impl Agents {
     fn ha_is_enabled(&self) -> bool {
         self.ha.enabled()
     }
+
+    fn partial_rebuild_is_enabled(&self) -> bool {
+        self.core.partial_rebuild_is_enabled()
+    }
 }
 
 /// This is used to deserialize the yaml object base.
@@ -326,6 +336,57 @@ impl Base {
     }
 }
 
+/// This is used to deserialize the yaml object 'agents.core'.
+#[derive(Deserialize)]
+struct Core {
+    #[serde(default)]
+    rebuild: Rebuild,
+}
+
+impl Core {
+    fn partial_rebuild_is_enabled(&self) -> bool {
+        self.rebuild.partial_is_enabled()
+    }
+}
+
+/// This is used to deserialize the yaml object 'agents.core.rebuild'.
+#[derive(Default, Deserialize)]
+struct Rebuild {
+    partial: RebuildPartial,
+}
+
+impl Rebuild {
+    fn partial_is_enabled(&self) -> bool {
+        self.partial.enabled()
+    }
+}
+
+/// This is used to deserialize the yaml object 'agents.core.rebuild.partial'.
+#[derive(Deserialize)]
+struct RebuildPartial {
+    enabled: bool,
+}
+
+impl Default for RebuildPartial {
+    /// We've never shipped with partial rebuild set to off. Also for a good while after
+    /// the feature was introduced, it was enabled without an option to disable it. So
+    /// assuming that partial rebuild is enabled, if the YAML object for Rebuild is missing.
+    /// The Rebuild type will be deserialized with a default value if it's absent from the
+    /// helm values.
+    ///
+    ///     #[serde(default)]
+    ///     rebuild: Rebuild,
+    fn default() -> Self {
+        Self { enabled: true }
+    }
+}
+
+impl RebuildPartial {
+    fn enabled(&self) -> bool {
+        self.enabled
+    }
+}
+
 /// This is used to deserialize the yaml object 'agents.ha'.
 #[derive(Deserialize)]
 struct Ha {

k8s/upgrade/src/bin/upgrade-job/helm/upgrade.rs

@@ -27,7 +27,8 @@ use tracing::info;
 
 /// HelmUpgradeRunner is returned after an upgrade is validated and dry-run-ed. Running
 /// it carries out helm upgrade.
-pub(crate) type HelmUpgradeRunner = Pin<Box<dyn Future<Output = Result<()>>>>;
+pub(crate) type HelmUpgradeRunner =
+    Pin<Box<dyn Future<Output = Result<Box<dyn HelmValuesCollection>>>>>;
 
 /// A trait object of type HelmUpgrader is either CoreHelmUpgrader or an UmbrellaHelmUpgrader.
 /// They either deal with upgrading the Core helm chart or the Umbrella helm chart respectively.
@@ -42,9 +43,6 @@ pub(crate) trait HelmUpgrader {
 
     /// Return the target helm chart version as a String.
     fn target_version(&self) -> String;
-
-    /// Returns a deserialized struct with tools to gather information about the source helm values.
-    fn source_values(&self) -> &dyn HelmValuesCollection;
 }
 
 /// This is a builder for the Helm chart upgrade.
@@ -183,14 +181,11 @@ impl HelmUpgraderBuilder {
             // Fail if the Umbrella chart isn't already upgraded.
             ensure!(already_upgraded, UmbrellaChartNotUpgraded);
 
-            // Deserialize umbrella chart values yaml.
-            let source_values = UmbrellaValues::try_from(source_values_buf.as_slice())?;
-
             Ok(Box::new(UmbrellaHelmUpgrader {
                 release_name,
+                client,
                 source_version,
                 target_version,
-                source_values,
             }))
         } else if Regex::new(core_regex.as_str())?.is_match(chart) {
             // Skip upgrade-path validation and allow all upgrades for the Core helm chart, if
@@ -292,7 +287,9 @@ impl HelmUpgrader for CoreHelmUpgrader {
 is the same as that of this upgrade-job's helm chart"
                 );
 
-                Ok(())
+                let source_values: Box<dyn HelmValuesCollection> = Box::new(self.source_values);
+
+                Ok(source_values)
             }));
         }
 
@@ -320,14 +317,20 @@ is the same as that of this upgrade-job's helm chart"
             info!("Starting helm upgrade...");
             self.client
                 .upgrade(
-                    self.release_name,
+                    self.release_name.as_str(),
                     self.chart_dir,
                     Some(self.helm_upgrade_extra_args),
                 )
                 .await?;
             info!("Helm upgrade successful!");
 
-            Ok(())
+            let final_values_buf = self
+                .client
+                .get_values_as_yaml::<&str, String>(self.release_name.as_str(), None)?;
+            let final_values: Box<dyn HelmValuesCollection> =
+                Box::new(CoreValues::try_from(final_values_buf.as_slice())?);
+
+            Ok(final_values)
         }))
     }
 
@@ -338,19 +341,15 @@ is the same as that of this upgrade-job's helm chart"
     fn target_version(&self) -> String {
         self.target_version.to_string()
     }
-
-    fn source_values(&self) -> &dyn HelmValuesCollection {
-        &self.source_values
-    }
 }
 
 /// This is a HelmUpgrader for the Umbrella chart. This gathers information, and doesn't
 /// set up a helm upgrade or a dry-run in any way.
 pub(crate) struct UmbrellaHelmUpgrader {
     release_name: String,
+    client: HelmReleaseClient,
     source_version: Version,
     target_version: Version,
-    source_values: UmbrellaValues,
 }
 
 #[async_trait]
@@ -362,7 +361,13 @@ impl HelmUpgrader for UmbrellaHelmUpgrader {
                 self.release_name.as_str()
             );
 
-            Ok(())
+            let final_values_buf = self
+                .client
+                .get_values_as_yaml::<&str, String>(self.release_name.as_str(), None)?;
+            let final_values: Box<dyn HelmValuesCollection> =
+                Box::new(UmbrellaValues::try_from(final_values_buf.as_slice())?);
+
+            Ok(final_values)
         }))
     }
 
@@ -373,8 +378,4 @@ impl HelmUpgrader for UmbrellaHelmUpgrader {
     fn target_version(&self) -> String {
         self.target_version.to_string()
     }
-
-    fn source_values(&self) -> &dyn HelmValuesCollection {
-        &self.source_values
-    }
 }