chore(bors): merge pull request #580

580: refactor(upgrade): expose modules to pub scope r=niladrih a=niladrih

This is a refactor which moves most of the upgrade-job 'bin' code to the upgrade lib, and expands its visibility scope from 'pub(crate)' to 'pub'.

Co-authored-by: Niladri Halder <[email protected]>

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

@@ -1,24 +1,21 @@
 use crate::{
-    common::{constants::product_train, error::Result},
     opts::validators::{
         validate_helm_chart_dir, validate_helm_release, validate_helmv3_in_path,
         validate_namespace, validate_rest_endpoint,
     },
-    upgrade::upgrade,
+    product_upgrade::upgrade,
 };
 use clap::Parser;
 use opts::CliArgs;
 use tracing::{error, info};
+use upgrade::common::{constants::product_train, error::Result};
 use utils::{
     print_package_info, raw_version_str,
     tracing_telemetry::{default_tracing_tags, flush_traces, TracingTelemetry},
 };
 
-mod common;
-mod events;
-mod helm;
 mod opts;
-mod upgrade;
+mod product_upgrade;
 
 #[tokio::main]
 async fn main() -> Result<()> {

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

@@ -1,6 +1,6 @@
-use crate::common::constants::product_train;
 use clap::Parser;
 use std::path::PathBuf;
+use upgrade::common::constants::product_train;
 use utils::{package_description, tracing_telemetry::FmtStyle, version_info_str};
 
 /// Validate input whose validation depends on other inputs.

k8s/upgrade/src/bin/upgrade-job/opts/validators.rs

@@ -1,4 +1,8 @@
-use crate::{
+use regex::bytes::Regex;
+use snafu::{ensure, ResultExt};
+use std::{fs, path::PathBuf, process::Command, str};
+use tracing::debug;
+use upgrade::{
     common::{
         constants::CORE_CHART_NAME,
         error::{
@@ -8,15 +12,11 @@ use crate::{
             YamlParseFromFile,
         },
         kube::client as KubeClient,
+        macros::vec_to_strings,
         rest_client::RestClientSet,
     },
     helm::chart::Chart,
-    vec_to_strings,
 };
-use regex::bytes::Regex;
-use snafu::{ensure, ResultExt};
-use std::{fs, path::PathBuf, process::Command, str};
-use tracing::debug;
 
 /// Validate that the helm release specified in the CLI options exists in the namespace,
 /// which is also specified in the CLI options.

k8s/upgrade/src/bin/upgrade-job/upgrade.rs → k8s/upgrade/src/bin/upgrade-job/product_upgrade.rs

@@ -1,4 +1,8 @@
-use crate::{
+use crate::opts::CliArgs;
+use constants::DS_CONTROLLER_REVISION_HASH_LABEL_KEY;
+use semver::Version;
+use tracing::error;
+use upgrade::{
     common::{
         constants::{
             product_train, CORE_CHART_NAME, IO_ENGINE_LABEL, PARTIAL_REBUILD_DISABLE_EXTENTS,
@@ -7,23 +11,11 @@ use crate::{
         kube::client as KubeClient,
     },
     events::event_recorder::{EventAction, EventRecorder},
+};
+use upgrade::{
     helm::upgrade::{HelmUpgradeRunner, HelmUpgraderBuilder},
-    opts::CliArgs,
+    upgrade_data_plane::upgrade_data_plane,
 };
-use constants::DS_CONTROLLER_REVISION_HASH_LABEL_KEY;
-use data_plane::upgrade_data_plane;
-
-use semver::Version;
-use tracing::error;
-
-/// Contains the data-plane upgrade logic.
-pub(crate) mod data_plane;
-
-/// Contains upgrade utilities.
-pub(crate) mod utils;
-
-/// Tools to validate upgrade path.
-pub(crate) mod path;
 
 /// This function starts and sees upgrade through to the end.
 pub(crate) async fn upgrade(opts: &CliArgs) -> Result<()> {
@@ -190,7 +182,7 @@ fn partial_rebuild_check(source_version: &Version, partial_rebuild_is_enabled: b
 mod tests {
     #[test]
     fn test_partial_rebuild_check() {
-        use crate::upgrade::partial_rebuild_check;
+        use crate::product_upgrade::partial_rebuild_check;
         use semver::Version;
 
         let source = Version::new(2, 1, 0);

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

@@ -1,20 +1,20 @@
 /// Contains constant values which are used as arguments to functions and in log messages.
-pub(crate) mod constants;
+pub mod constants;
 
 /// Contains the error handling tooling.
-pub(crate) mod error;
+pub mod error;
 
 /// Contains tools to work with Kubernetes APIs.
-pub(crate) mod kube;
+pub mod kube;
 
 /// Contains macros.
-pub(crate) mod macros;
+pub mod macros;
 
 /// Contains tools to create storage API clients.
-pub(crate) mod rest_client;
+pub mod rest_client;
 
 /// Contains tools for working with files.
-pub(crate) mod file;
+pub mod file;
 
 /// Contains a wrapper around regex::Regex.
-pub(crate) mod regex;
+pub mod regex;

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

@@ -5,24 +5,24 @@ pub use constants::product_train;
 
 /// This is the name of the Helm chart which included the core chart as a sub-chart.
 /// Under the hood, this installs the Core Helm chart (see below).
-pub(crate) const UMBRELLA_CHART_NAME: &str = constants::UMBRELLA_CHART_NAME;
+pub const UMBRELLA_CHART_NAME: &str = constants::UMBRELLA_CHART_NAME;
 
 /// This is the name of the Helm chart of this project.
-pub(crate) const CORE_CHART_NAME: &str = constants::PRODUCT_NAME;
+pub const CORE_CHART_NAME: &str = constants::PRODUCT_NAME;
 
 /// This is the shared Pod label of the <helm-release>-io-engine DaemonSet.
-pub(crate) const IO_ENGINE_LABEL: &str = "app=io-engine";
+pub const IO_ENGINE_LABEL: &str = "app=io-engine";
 
 /// This is the shared Pod label of the <helm-release>-agent-core Deployment.
-pub(crate) const AGENT_CORE_LABEL: &str = "app=agent-core";
+pub const AGENT_CORE_LABEL: &str = "app=agent-core";
 
 /// This is the label set on a storage API Node resource when a 'Node Drain' is issued.
-pub fn drain_for_upgrade() -> String {
+pub(crate) fn drain_for_upgrade() -> String {
     format!("{CORE_CHART_NAME}-upgrade")
 }
 
 /// This is the label set on a storage API Node resource when a 'Node Drain' is issued.
-pub fn cordon_ana_check() -> String {
+pub(crate) fn cordon_ana_check() -> String {
     format!("{CORE_CHART_NAME}-upgrade-nvme-ana-check")
 }
 
@@ -34,7 +34,7 @@ pub(crate) const UMBRELLA_CHART_UPGRADE_DOCS_URL: &str = constants::UMBRELLA_CHA
 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) =
+pub 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.

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

@@ -14,10 +14,10 @@ use url::Url;
 /// defined withing the same scope and must return to the outer scope (calling scope) using
 /// the try operator -- '?'.
 #[derive(Debug, Snafu)]
-#[snafu(visibility(pub(crate)))]
+#[snafu(visibility(pub))]
 #[snafu(context(suffix(false)))]
 #[allow(unused)]
-pub(crate) enum Error {
+pub enum Error {
     /// Error for when the storage REST API URL is parsed.
     #[snafu(display(
         "Failed to parse {} REST API URL {rest_endpoint}: {source}",
@@ -666,4 +666,4 @@ pub(crate) enum Error {
 }
 
 /// A wrapper type to remove repeated Result<T, Error> returns.
-pub(crate) type Result<T, E = Error> = std::result::Result<T, E>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;

k8s/upgrade/src/bin/upgrade-job/common/file.rs → k8s/upgrade/src/common/file.rs

@@ -5,7 +5,7 @@ use tempfile::NamedTempFile as TempFile;
 
 /// Write buffer to an existing temporary file if a path is provided as an argument, else
 /// create a new temporary file and write to it. Returns the file handle.
-pub(crate) fn write_to_tempfile<P>(file_dir: Option<P>, buf: &[u8]) -> Result<TempFile>
+pub fn write_to_tempfile<P>(file_dir: Option<P>, buf: &[u8]) -> Result<TempFile>
 where
     P: AsRef<Path>,
 {

k8s/upgrade/src/bin/upgrade-job/common/kube.rs → k8s/upgrade/src/common/kube.rs

@@ -1,2 +1,2 @@
 /// This contains tools for working with the Kubernetes REST API.
-pub(crate) mod client;
+pub mod client;

k8s/upgrade/src/bin/upgrade-job/common/kube/client.rs → k8s/upgrade/src/common/kube/client.rs

@@ -23,46 +23,46 @@ use serde::de::DeserializeOwned;
 use snafu::{ensure, ErrorCompat, IntoError, ResultExt};
 
 /// Generate a new kube::Client.
-pub(crate) async fn client() -> Result<Client> {
+pub async fn client() -> Result<Client> {
     Client::try_default().await.context(K8sClientGeneration)
 }
 
 /// Generate the Node api client.
-pub(crate) async fn nodes_api() -> Result<Api<Node>> {
+pub async fn nodes_api() -> Result<Api<Node>> {
     Ok(Api::all(client().await?))
 }
 
 /// Generate the Namespace api client.
-pub(crate) async fn namespaces_api() -> Result<Api<Namespace>> {
+pub async fn namespaces_api() -> Result<Api<Namespace>> {
     Ok(Api::all(client().await?))
 }
 
 /// Generate the CustomResourceDefinition api client.
-pub(crate) async fn crds_api() -> Result<Api<CustomResourceDefinition>> {
+pub async fn crds_api() -> Result<Api<CustomResourceDefinition>> {
     Ok(Api::all(client().await?))
 }
 
 /// Generate ControllerRevision api client.
-pub(crate) async fn controller_revisions_api(namespace: &str) -> Result<Api<ControllerRevision>> {
+pub async fn controller_revisions_api(namespace: &str) -> Result<Api<ControllerRevision>> {
     Ok(Api::namespaced(client().await?, namespace))
 }
 
 /// Generate the Pod api client.
-pub(crate) async fn pods_api(namespace: &str) -> Result<Api<Pod>> {
+pub async fn pods_api(namespace: &str) -> Result<Api<Pod>> {
     Ok(Api::namespaced(client().await?, namespace))
 }
 
 /// Generate the Secret api client.
-pub(crate) async fn secrets_api(namespace: &str) -> Result<Api<Secret>> {
+pub async fn secrets_api(namespace: &str) -> Result<Api<Secret>> {
     Ok(Api::namespaced(client().await?, namespace))
 }
 
 /// Generate the Configmap api client.
-pub(crate) async fn configmaps_api(namespace: &str) -> Result<Api<ConfigMap>> {
+pub async fn configmaps_api(namespace: &str) -> Result<Api<ConfigMap>> {
     Ok(Api::namespaced(client().await?, namespace))
 }
 
-pub(crate) async fn list_pods(
+pub async fn list_pods(
     namespace: String,
     label_selector: Option<String>,
     field_selector: Option<String>,
@@ -91,7 +91,7 @@ pub(crate) async fn list_pods(
 }
 
 /// List Nodes metadata in the kubernetes cluster.
-pub(crate) async fn list_nodes_metadata(
+pub async fn list_nodes_metadata(
     label_selector: Option<String>,
     field_selector: Option<String>,
 ) -> Result<Vec<PartialObjectMeta<Node>>> {
@@ -124,7 +124,7 @@ pub(crate) async fn list_nodes_metadata(
 }
 
 /// List ControllerRevisions in a Kubernetes namespace.
-pub(crate) async fn list_controller_revisions(
+pub async fn list_controller_revisions(
     namespace: String,
     label_selector: Option<String>,
     field_selector: Option<String>,
@@ -159,7 +159,7 @@ pub(crate) async fn list_controller_revisions(
 }
 
 /// Returns the controller-revision-hash of the latest revision of a resource's ControllerRevisions.
-pub(crate) async fn latest_controller_revision_hash(
+pub async fn latest_controller_revision_hash(
     namespace: String,
     label_selector: Option<String>,
     field_selector: Option<String>,
@@ -199,7 +199,7 @@ pub(crate) async fn latest_controller_revision_hash(
 }
 
 /// This returns a list of Secrets based on filtering criteria. Returns all if criteria is absent.
-pub(crate) async fn list_secrets(
+pub async fn list_secrets(
     namespace: String,
     label_selector: Option<String>,
     field_selector: Option<String>,
@@ -235,7 +235,7 @@ pub(crate) async fn list_secrets(
 
 /// This returns a list of ConfigMaps based on filtering criteria. Returns all if criteria is
 /// absent.
-pub(crate) async fn list_configmaps(
+pub async fn list_configmaps(
     namespace: String,
     label_selector: Option<String>,
     field_selector: Option<String>,
@@ -270,10 +270,7 @@ pub(crate) async fn list_configmaps(
 }
 
 /// GET the helm release secret for a helm release in a namespace.
-pub(crate) async fn get_helm_release_secret(
-    release_name: String,
-    namespace: String,
-) -> Result<Secret> {
+pub async fn get_helm_release_secret(release_name: String, namespace: String) -> Result<Secret> {
     let secrets = list_secrets(
         namespace.clone(),
         Some(format!("name={release_name},status=deployed")),
@@ -294,7 +291,7 @@ pub(crate) async fn get_helm_release_secret(
 }
 
 /// GET the helm release configmap for a helm release in a namespace.
-pub(crate) async fn get_helm_release_configmap(
+pub async fn get_helm_release_configmap(
     release_name: String,
     namespace: String,
 ) -> Result<ConfigMap> {

k8s/upgrade/src/bin/upgrade-job/common/macros.rs → k8s/upgrade/src/common/macros.rs

@@ -3,3 +3,5 @@
 macro_rules! vec_to_strings {
     ($($x:expr),*) => (vec![$($x.to_string()),*]);
 }
+
+pub use vec_to_strings;

k8s/upgrade/src/bin/upgrade-job/common/regex.rs → k8s/upgrade/src/common/regex.rs

@@ -3,18 +3,19 @@ use regex::Regex as BackendRegex;
 use snafu::ResultExt;
 
 /// This is a wrapper around regex::Regex.
-pub(crate) struct Regex {
+pub struct Regex {
     inner: BackendRegex,
 }
 
 impl Regex {
     /// This is a wrapper around regex::Regex::new(). It handles errors, so that crate-wide
     /// if statements can look prettier:
     ///
+    /// use upgrade::common::regex::Regex
     ///     if Regex::new(r"^yay$")?.is_match("yay") {
     ///         todo!();
     ///     }
-    pub(crate) fn new(expr: &str) -> Result<Regex> {
+    pub fn new(expr: &str) -> Result<Regex> {
         let regex = BackendRegex::new(expr).context(RegexCompile {
             expression: expr.to_string(),
         })?;
@@ -23,7 +24,7 @@ impl Regex {
     }
 
     /// This is a wrapper around regex::Regex::is_match().
-    pub(crate) fn is_match(&self, haystack: &str) -> bool {
+    pub fn is_match(&self, haystack: &str) -> bool {
         self.inner.is_match(haystack)
     }
 }

k8s/upgrade/src/bin/upgrade-job/common/rest_client.rs → k8s/upgrade/src/common/rest_client.rs

@@ -5,13 +5,13 @@ use std::time::Duration;
 use url::Url;
 
 /// This is a wrapper for the openapi::tower::client::ApiClient.
-pub(crate) struct RestClientSet {
+pub struct RestClientSet {
     client: ApiClient,
 }
 
 impl RestClientSet {
     /// Build the RestConfig, and the eventually the ApiClient. Fails if configuration is invalid.
-    pub(crate) fn new_with_url(rest_endpoint: String) -> Result<Self> {
+    pub fn new_with_url(rest_endpoint: String) -> Result<Self> {
         let rest_url =
             Url::try_from(rest_endpoint.as_str()).context(RestUrlParse { rest_endpoint })?;
 
@@ -31,11 +31,11 @@ impl RestClientSet {
         Ok(RestClientSet { client })
     }
 
-    pub(crate) fn nodes_api(&self) -> &dyn openapi::apis::nodes_api::tower::client::Nodes {
+    pub fn nodes_api(&self) -> &dyn openapi::apis::nodes_api::tower::client::Nodes {
         self.client.nodes_api()
     }
 
-    pub(crate) fn volumes_api(&self) -> &dyn openapi::apis::volumes_api::tower::client::Volumes {
+    pub fn volumes_api(&self) -> &dyn openapi::apis::volumes_api::tower::client::Volumes {
         self.client.volumes_api()
     }
 }