feat: restructure onhost folders and naming

Author: vjripoll

.goreleaser.yml

@@ -72,31 +72,23 @@ nfpms:
         file_info:
           mode: 0700
       - src: build/package/config.yaml
-        dst: /etc/newrelic-agent-control/config.yaml
+        dst: /etc/newrelic-agent-control/local-data-agent-control/config.yaml
         type: config
         file_info:
           mode: 0600
       - src: build/package/newrelic-agent-control.service
         dst: /lib/systemd/system/newrelic-agent-control.service
       - src: build/package/newrelic-agent-control.conf
-        dst: /etc/newrelic-agent-control/newrelic-agent-control.conf
+        dst: /etc/newrelic-agent-control/systemd-env.conf
         type: config|noreplace
         file_info:
           mode: 0600
-      - dst: /etc/newrelic-agent-control/fleet/agents.d
-        type: dir
-        file_info:
-          mode: 0700
       - src: LICENSE.md
         dst: /usr/share/doc/newrelic/newrelic-agent-control/LICENSE.md
       - dst: /var/lib/newrelic-agent-control
         type: dir
         file_info:
           mode: 0700
-      - dst: /var/lib/newrelic-agent-control/fleet/agents.d
-        type: dir
-        file_info:
-          mode: 0700
       - dst: /var/lib/newrelic-agent-control/auto-generated
         type: dir
         file_info:

agent-control/src/agent_control/defaults.rs

@@ -1,4 +1,6 @@
+use crate::agent_control::agent_id::AgentID;
 use crate::agent_type::agent_type_id::AgentTypeID;
+use crate::k8s::store::StoreKey;
 use crate::opamp::remote_config::signature::SIGNATURE_CUSTOM_CAPABILITY;
 use crate::sub_agent::identity::AgentIdentity;
 use opamp_client::capabilities;
@@ -46,13 +48,18 @@ cfg_if::cfg_if! {
         pub const AGENT_CONTROL_LOG_DIR: &str = "/var/log/newrelic-agent-control";
     }
 }
-
-pub const SUB_AGENT_DIR: &str = "fleet/agents.d";
-pub const AGENT_CONTROL_CONFIG_FILENAME: &str = "config.yaml";
+/// The prefixes for the ConfigMap name and for on host directories.
+/// The cm having CM_NAME_LOCAL_DATA_PREFIX stores all the config that are "local",
+/// the SA treats those CM as read-only.
+pub const CM_NAME_LOCAL_DATA_PREFIX: &str = "local-data-";
+/// The cm having CM_NAME_OPAMP_DATA_PREFIX as prefix stores all the data related with opamp:
+/// Instance IDs, hashes, and remote configs. The Sa reads and writes those CMs.
+pub const CM_NAME_OPAMP_DATA_PREFIX: &str = "fleet-data-";
+pub const STORE_KEY_LOCAL_DATA_CONFIG: &StoreKey = "local_config";
+pub const STORE_KEY_OPAMP_DATA_CONFIG: &StoreKey = "remote_config";
+pub const STORE_KEY_INSTANCE_ID: &StoreKey = "instance_id";
 pub const DYNAMIC_AGENT_TYPE_DIR: &str = "dynamic-agent-types";
-pub const IDENTIFIERS_FILENAME: &str = "identifiers.yaml";
-pub const VALUES_DIR: &str = "values";
-pub const VALUES_FILENAME: &str = "values.yaml";
+pub const INSTANCE_ID_FILENAME: &str = "instance_id.yaml";
 pub const GENERATED_FOLDER_NAME: &str = "auto-generated";
 pub const AGENT_CONTROL_LOG_FILENAME: &str = "newrelic-agent-control.log";
 pub const STDOUT_LOG_PREFIX: &str = "stdout.log";
@@ -75,6 +82,10 @@ pub fn default_sub_agent_custom_capabilities() -> CustomCapabilities {
     }
 }
 
+pub fn build_folder_name(agent_id: &AgentID, prefix: &str) -> String {
+    format!("{prefix}{agent_id}")
+}
+
 pub(crate) fn get_custom_capabilities(agent_type_id: &AgentTypeID) -> Option<CustomCapabilities> {
     if agent_type_id.eq(&AgentIdentity::new_agent_control_identity().agent_type_id) {
         // Agent_Control does not have custom capabilities for now

agent-control/src/agent_control/run/on_host.rs

@@ -5,7 +5,7 @@ use crate::agent_control::config_repository::store::AgentControlConfigStore;
 use crate::agent_control::config_validator::RegistryDynamicConfigValidator;
 use crate::agent_control::defaults::{
     AGENT_CONTROL_VERSION, FLEET_ID_ATTRIBUTE_KEY, HOST_ID_ATTRIBUTE_KEY, HOST_NAME_ATTRIBUTE_KEY,
-    OPAMP_AGENT_VERSION_ATTRIBUTE_KEY, SUB_AGENT_DIR,
+    OPAMP_AGENT_VERSION_ATTRIBUTE_KEY,
 };
 use crate::agent_control::http_server::runner::Runner;
 use crate::agent_control::resource_cleaner::no_op::NoOpResourceCleaner;
@@ -99,7 +99,6 @@ impl AgentControlRunner {
             LocalFile,
             DirectoryManagerFs,
             self.base_paths.remote_dir.clone(),
-            self.base_paths.remote_dir.join(SUB_AGENT_DIR),
         );
         let instance_id_getter =
             InstanceIDWithIdentifiersGetter::new(instance_id_storer, identifiers);
@@ -154,8 +153,7 @@ impl AgentControlRunner {
             &self.base_paths.remote_dir,
         ));
 
-        let supervisor_builder =
-            SupervisortBuilderOnHost::new(self.base_paths.log_dir.join(SUB_AGENT_DIR));
+        let supervisor_builder = SupervisortBuilderOnHost::new(self.base_paths.log_dir);
 
         let signature_validator = Arc::new(self.signature_validator);
         let remote_config_validators = vec![

agent-control/src/bin/main_config_migrate.rs

@@ -26,7 +26,7 @@ fn main() -> Result<(), Box<dyn Error>> {
     let config_migrator = ConfigMigrator::new(
         ConfigConverter::default(),
         AgentConfigGetter::new(sa_local_config_loader),
-        ValuesPersisterFile::new(cli.local_sub_agent_data_dir()),
+        ValuesPersisterFile::new(cli.local_data_dir()),
     );
 
     let legacy_config_renamer = LegacyConfigRenamer::default();

agent-control/src/config_migrate/cli.rs

@@ -1,6 +1,6 @@
 use crate::{
     agent_control::defaults::{
-        AGENT_CONTROL_DATA_DIR, AGENT_CONTROL_LOCAL_DATA_DIR, AGENT_CONTROL_LOG_DIR, SUB_AGENT_DIR,
+        AGENT_CONTROL_DATA_DIR, AGENT_CONTROL_LOCAL_DATA_DIR, AGENT_CONTROL_LOG_DIR,
     },
     config_migrate::migration::defaults::NEWRELIC_INFRA_AGENT_TYPE_CONFIG_MAPPING,
 };
@@ -38,10 +38,6 @@ impl Cli {
         self.local_dir.to_path_buf()
     }
 
-    pub fn local_sub_agent_data_dir(&self) -> PathBuf {
-        self.local_dir.join(SUB_AGENT_DIR)
-    }
-
     pub fn remote_data_dir(&self) -> PathBuf {
         self.remote_dir.to_path_buf()
     }

agent-control/src/config_migrate/migration/persister/values_persister_file.rs

@@ -1,5 +1,5 @@
 use crate::agent_control::agent_id::AgentID;
-use crate::agent_control::defaults::{VALUES_DIR, VALUES_FILENAME};
+use crate::agent_control::defaults::STORE_KEY_LOCAL_DATA_CONFIG;
 use fs::LocalFile;
 use fs::directory_manager::{DirectoryManagementError, DirectoryManager, DirectoryManagerFs};
 use fs::writer_file::{FileWriter, WriteError};
@@ -51,11 +51,10 @@ where
         if !path.exists() {
             self.create_directory(&path)?;
         }
-        path.push(VALUES_DIR);
         if !path.exists() {
             self.create_directory(&path)?;
         }
-        path.push(VALUES_FILENAME);
+        path.push(STORE_KEY_LOCAL_DATA_CONFIG);
 
         debug!("writing to file {:?}", path.as_path());
 

agent-control/src/k8s/store.rs

@@ -3,23 +3,12 @@ use super::Error;
 use super::client::SyncK8sClient;
 use super::labels::Labels;
 use crate::agent_control::agent_id::AgentID;
+use crate::agent_control::defaults::{CM_NAME_LOCAL_DATA_PREFIX, CM_NAME_OPAMP_DATA_PREFIX};
 use std::sync::{Arc, RwLock};
 
-/// The prefixes for the ConfigMap name.
-/// The cm having CM_NAME_LOCAL_DATA_PREFIX stores all the config that are "local",
-/// the SA treats those CM as read-only.
-pub const CM_NAME_LOCAL_DATA_PREFIX: &str = "local-data-";
-/// The cm having CM_NAME_OPAMP_DATA_PREFIX as prefix stores all the data related with opamp:
-/// Instance IDs, hashes, and remote configs. The Sa reads and writes those CMs.
-pub const CM_NAME_OPAMP_DATA_PREFIX: &str = "fleet-data-";
-
 /// The key used to identify the data in the Store.
 pub type StoreKey = str;
 
-pub const STORE_KEY_LOCAL_DATA_CONFIG: &StoreKey = "local_config";
-pub const STORE_KEY_OPAMP_DATA_CONFIG: &StoreKey = "remote_config";
-pub const STORE_KEY_INSTANCE_ID: &StoreKey = "instance_id";
-
 /// Represents a Kubernetes persistent store of Agents data such as instance id and configs.
 /// The store is implemented using one ConfigMap per Agent with all the data.
 pub struct K8sStore {

agent-control/src/opamp/instance_id/k8s/storer.rs

@@ -1,16 +1,13 @@
 use crate::agent_control::agent_id::AgentID;
 
-use crate::k8s::{
-    self,
-    store::{K8sStore, STORE_KEY_INSTANCE_ID},
-};
+use super::getter::Identifiers;
+use crate::agent_control::defaults::STORE_KEY_INSTANCE_ID;
+use crate::k8s::{self, store::K8sStore};
 use crate::opamp::instance_id::getter::DataStored;
 use crate::opamp::instance_id::storer::InstanceIDStorer;
 use std::sync::Arc;
 use tracing::debug;
 
-use super::getter::Identifiers;
-
 pub struct Storer {
     k8s_store: Arc<K8sStore>,
 }

agent-control/src/opamp/instance_id/on_host/storer.rs

@@ -1,6 +1,6 @@
 use super::getter::Identifiers;
 use crate::agent_control::agent_id::AgentID;
-use crate::agent_control::defaults::IDENTIFIERS_FILENAME;
+use crate::agent_control::defaults::{CM_NAME_OPAMP_DATA_PREFIX, INSTANCE_ID_FILENAME};
 use crate::opamp::instance_id::getter::DataStored;
 use crate::opamp::instance_id::storer::InstanceIDStorer;
 use fs::LocalFile;
@@ -20,7 +20,6 @@ where
     file_rw: F,
     dir_manager: D,
     agent_control_remote_dir: PathBuf,
-    agent_remote_dir: PathBuf,
 }
 
 #[derive(thiserror::Error, Debug)]
@@ -67,17 +66,11 @@ where
     D: DirectoryManager,
     F: FileWriter + FileReader,
 {
-    pub fn new(
-        file_rw: F,
-        dir_manager: D,
-        agent_control_remote_dir: PathBuf,
-        agent_remote_dir: PathBuf,
-    ) -> Self {
+    pub fn new(file_rw: F, dir_manager: D, agent_control_remote_dir: PathBuf) -> Self {
         Self {
             file_rw,
             dir_manager,
             agent_control_remote_dir,
-            agent_remote_dir,
         }
     }
 }
@@ -128,14 +121,14 @@ where
         }
     }
 
+    pub fn build_folder_name(&self, agent_id: &AgentID, prefix: &str) -> String {
+        format!("{prefix}{agent_id}")
+    }
+
     fn get_instance_id_path(&self, agent_id: &AgentID) -> PathBuf {
-        if agent_id == &AgentID::AgentControl {
-            self.agent_control_remote_dir.join(IDENTIFIERS_FILENAME)
-        } else {
-            self.agent_remote_dir
-                .join(agent_id)
-                .join("identifiers.yaml")
-        }
+        self.agent_control_remote_dir
+            .join(self.build_folder_name(agent_id, CM_NAME_OPAMP_DATA_PREFIX))
+            .join(INSTANCE_ID_FILENAME)
     }
 }
 
@@ -156,26 +149,32 @@ mod tests {
     #[test]
     fn basic_get_uild_path() {
         let sa_dir = PathBuf::from("/super");
-        let sub_agent_dir = PathBuf::from("/sub");
         let storer = Storer::new(
             MockLocalFile::default(),
             MockDirectoryManager::default(),
             sa_dir.clone(),
-            sub_agent_dir.clone(),
         );
         let agent_id = AgentID::try_from("test").unwrap();
         let path = storer.get_instance_id_path(&agent_id);
-        assert_eq!(path, sub_agent_dir.join("test").join("identifiers.yaml"));
+        assert_eq!(
+            path,
+            sa_dir.join("fleet-data-test").join("instance_id.yaml")
+        );
 
         let agent_control_id = AgentID::AgentControl;
         let path = storer.get_instance_id_path(&agent_control_id);
-        assert_eq!(path, sa_dir.join("identifiers.yaml"));
+        assert_eq!(
+            path,
+            sa_dir
+                .join("fleet-data-agent-control")
+                .join("instance_id.yaml")
+        );
     }
 
     #[test]
     fn test_successful_write() {
         // Data
-        let (agent_id, sa_path, sub_agent_path, instance_id_path) = test_data();
+        let (agent_id, sa_path, instance_id_path) = test_data();
         let mut file_rw = MockLocalFile::default();
         let mut dir_manager = MockDirectoryManager::default();
         let instance_id = InstanceID::create();
@@ -188,14 +187,14 @@ mod tests {
         dir_manager.should_create(instance_id_path.parent().unwrap());
         file_rw.should_write(&instance_id_path, expected_file(instance_id));
 
-        let storer = Storer::new(file_rw, dir_manager, sa_path, sub_agent_path);
+        let storer = Storer::new(file_rw, dir_manager, sa_path);
         assert!(storer.set(&agent_id, &ds).is_ok());
     }
 
     #[test]
     fn test_unsuccessful_write() {
         // Data
-        let (agent_id, sa_path, sub_agent_path, instance_id_path) = test_data();
+        let (agent_id, sa_path, instance_id_path) = test_data();
         let mut file_rw = MockLocalFile::default();
         let mut dir_manager = MockDirectoryManager::default();
         let instance_id = InstanceID::create();
@@ -208,14 +207,14 @@ mod tests {
         file_rw.should_not_write(&instance_id_path, expected_file(instance_id));
         dir_manager.should_create(instance_id_path.parent().unwrap());
 
-        let storer = Storer::new(file_rw, dir_manager, sa_path, sub_agent_path);
+        let storer = Storer::new(file_rw, dir_manager, sa_path);
         assert!(storer.set(&agent_id, &ds).is_err());
     }
 
     #[test]
     fn test_successful_read() {
         // Data
-        let (agent_id, sa_path, sub_agent_path, instance_id_path) = test_data();
+        let (agent_id, sa_path, instance_id_path) = test_data();
         let mut file_rw = MockLocalFile::default();
         let dir_manager = MockDirectoryManager::default();
         let instance_id = InstanceID::create();
@@ -234,15 +233,15 @@ mod tests {
             .once()
             .return_once(|_| Ok(expected_file(instance_id)));
 
-        let storer = Storer::new(file_rw, dir_manager, sa_path, sub_agent_path);
+        let storer = Storer::new(file_rw, dir_manager, sa_path);
         let actual = storer.get(&agent_id);
         assert!(actual.is_ok());
         assert_eq!(expected, actual.unwrap());
     }
 
     #[test]
     fn test_unsuccessful_read() {
-        let (agent_id, sa_path, sub_agent_path, instance_id_path) = test_data();
+        let (agent_id, sa_path, instance_id_path) = test_data();
         let mut file_rw = MockLocalFile::default();
         let dir_manager = MockDirectoryManager::default();
 
@@ -254,7 +253,7 @@ mod tests {
             .once()
             .return_once(|_| Err(io::Error::other("some error message").into()));
 
-        let storer = Storer::new(file_rw, dir_manager, sa_path, sub_agent_path);
+        let storer = Storer::new(file_rw, dir_manager, sa_path);
         let expected = storer.get(&agent_id);
 
         // As said above, we are not generating the error variant here
@@ -270,12 +269,11 @@ mod tests {
     const HOST_ID: &str = "test-host-id";
     const FLEET_ID: &str = "test-fleet-id";
 
-    fn test_data() -> (AgentID, PathBuf, PathBuf, PathBuf) {
+    fn test_data() -> (AgentID, PathBuf, PathBuf) {
         let agent_id = AgentID::try_from("test").unwrap();
         let sa_path = PathBuf::from("/super");
-        let sub_agent_path = PathBuf::from("/sub");
-        let instance_id_path = PathBuf::from("/sub/test/identifiers.yaml");
-        (agent_id, sa_path, sub_agent_path, instance_id_path)
+        let instance_id_path = PathBuf::from("/super/fleet-data-test/instance_id.yaml");
+        (agent_id, sa_path, instance_id_path)
     }
 
     fn expected_file(instance_id: InstanceID) -> String {