refactor: remove file vartypes and ConfigurationPersister (#1743)

* refactor: remove ConfigurationPersister

* docs: replace file var types with filesystem

* chore: cleanup commented code

* refactor: remove Renderer trait

* chore: remove commented-out code

Author: DavSanchez

agent-control/src/agent_control/error.rs

@@ -5,7 +5,6 @@ use crate::agent_control::config_validator::DynamicConfigValidatorError;
 use crate::agent_control::version_updater::updater::UpdaterError;
 use crate::agent_type::agent_type_registry::AgentRepositoryError;
 use crate::agent_type::error::AgentTypeError;
-use crate::agent_type::render::persister::config_persister::PersistError;
 use crate::event::channel::EventPublisherError;
 use crate::opamp::client_builder::OpAMPClientBuilderError;
 use crate::opamp::instance_id;
@@ -54,9 +53,6 @@ pub enum AgentError {
     #[error("{0}")]
     OpAMPStartedClient(#[from] StartedClientError),
 
-    #[error("error persisting agent config: {0}")]
-    Persistence(#[from] PersistError),
-
     #[error("error getting agent instance id: {0}")]
     GetInstanceID(#[from] instance_id::getter::GetterError),
 

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

@@ -15,8 +15,7 @@ use crate::agent_control::resource_cleaner::ResourceCleanerError;
 use crate::agent_control::resource_cleaner::k8s_garbage_collector::K8sGarbageCollector;
 use crate::agent_control::run::AgentControlRunner;
 use crate::agent_control::version_updater::k8s::K8sACUpdater;
-use crate::agent_type::render::persister::config_persister_file::ConfigurationPersisterFile;
-use crate::agent_type::render::renderer::TemplateRenderer;
+use crate::agent_type::render::TemplateRenderer;
 use crate::agent_type::variable::Variable;
 use crate::agent_type::version_config::{
     AGENT_CONTROL_VERSION_CHECKER_INITIAL_DELAY, VersionCheckerInterval,
@@ -141,9 +140,8 @@ impl AgentControlRunner {
             ),
         ]);
 
-        let template_renderer: TemplateRenderer<ConfigurationPersisterFile> =
-            TemplateRenderer::default()
-                .with_agent_control_variables(agent_control_variables.clone().into_iter());
+        let template_renderer = TemplateRenderer::default()
+            .with_agent_control_variables(agent_control_variables.clone().into_iter());
 
         let mut secrets_providers = SecretsProviders::new()
             .with_env()

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

@@ -11,8 +11,7 @@ use crate::agent_control::http_server::runner::Runner;
 use crate::agent_control::resource_cleaner::no_op::NoOpResourceCleaner;
 use crate::agent_control::run::AgentControlRunner;
 use crate::agent_control::version_updater::updater::NoOpUpdater;
-use crate::agent_type::render::persister::config_persister_file::ConfigurationPersisterFile;
-use crate::agent_type::render::renderer::TemplateRenderer;
+use crate::agent_type::render::TemplateRenderer;
 use crate::agent_type::variable::Variable;
 use crate::event::channel::pub_sub;
 use crate::health::noop::NoOpHealthChecker;
@@ -136,10 +135,6 @@ impl AgentControlRunner {
         let opamp_client_builder = opamp_client_builder.map(|b| b.with_startup_check_disabled());
 
         let template_renderer = TemplateRenderer::default()
-            .with_config_persister(
-                ConfigurationPersisterFile::new(&self.base_paths.remote_dir),
-                self.base_paths.remote_dir.clone(),
-            )
             .with_agent_control_variables(agent_control_variables.clone().into_iter());
 
         let mut secrets_providers = SecretsProviders::new().with_env();

agent-control/src/agent_type/README.md

@@ -2,7 +2,7 @@
 
 Agent Type Definition is a YAML file that defines an agent's configuration and behavior. It consists of three main sections: metadata, deployment, and variables.
 
-By defining these three sections, developers can create a customizable and flexible agent type that can be used in various environments. 
+By defining these three sections, developers can create a customizable and flexible agent type that can be used in various environments.
 
 ## Metadata
 
@@ -15,31 +15,30 @@ version: 0.0.1
 ```
 
 The metadata fields can't be empty:
+
 * The name and namespace should:
-  - Start by an alphabetical character.
-  - Only encompass `alphanumeric characters`, `.`, `_` or `-`.
-  - Be in lowercase.
+  * Start by an alphabetical character.
+  * Only encompass `alphanumeric characters`, `.`, `_` or `-`.
+  * Be in lowercase.
 * The version field should adhere to [semantic versioning](https://semver.org/).
 
 ## Variables
 
-The variables section allows developers to define variables that end users can set. These variables can adjust the agent's or system's configuration.
+The `variables` section allows developers to define variables that end users can set. These variables can adjust the agent's or system's configuration.
 
 ```yaml
 variables:
   common:
     config_agent:
       description: "Newrelic infra configuration"
-      type: file
+      type: yaml
       required: false
-      default: ""
-      file_path: "newrelic-infra.yml"
+      default: {}
     config_integrations:
       description: "map of YAML configs for the OHIs"
-      type: map[string]file
+      type: map[string]yaml
       required: false
       default: {}
-      file_path: "integrations.d"
   on_host:
     backoff_delay:
       description: "seconds until next retry if agent fails to start"
@@ -57,6 +56,7 @@ variables:
 ```
 
 Variables can be classified based on their applicable environments:
+
 * `on_host`: Refers to variables utilized in on host environments.
 * `k8s`: Applies to variables used within Kubernetes clusters.
 * `common`: For variables that are environment-agnostic.
@@ -78,30 +78,24 @@ common:
 
 All variables have a few common attributes:
 
-* `description`: A brief description of the variable.This is useful for documentation purposes and can help others understand the purpose of the variable.
-* `type`: The data type of the variable. We support several data types, including `string`, `file`, `bool`, `yaml`, and more. 
+* `description`: A brief description of the variable. This is useful for documentation purposes and can help others understand the purpose of the variable.
+* `type`: The data type of the variable. We support several data types, including `string`, `file`, `bool`, `yaml`, and more.
 * `variants`: Represents a defined list of acceptable values for the variable. Only values present in the variants list are considered valid.
 * `default`: The default value for the variable if no value is provided.
 * `required`: Whether the variable is mandatory to be provided or not.
 
-Moreover, file type variables possess an additional attribute:
-* `file_path`: Indicates where the file(s) is located. 
-
-In terms of variable types, we currently support the following types listed [here](variable/kind.rs#L14):
+In terms of variable types, we currently support the following types listed in [this source file](./variable/variable_type.rs#L22):
 
 * `string`: A string value, such as "Hello, world!"
 * `number`: An numeric value, such as 42 or 0.25
 * `boolean`: A boolean value, which can be either *true* or *false*
-* `file`: It represents a file in the filesystem.
 * `map[string]string`: A dictionary of key-value pairs, associating keys(string) and values(string).
-* `map[string]file`: A dictionary of key-value pairs, associating keys(string) and values(file). Files are stored in the specified folder.
 * `yaml`: The YAML type variable is used to handle multi-line strings that will be parsed as YAML such as Helm Charts values.
-
-Note that `file` and `map[string]file` variable types can be used just for `on_host` environments. Files are not supported in Kubernetes.
+* `map[string]yaml`: Handles YAML values that guarantee their top-level fields are strings. Useful for defining file system entries for on-host.
 
 ## Deployment
 
-The deployment section indicates how the agent should be executed and its health should be checked.
+The deployment section indicates how the agent should be executed and how its health should be checked.
 
 Note you can reference the variables defined in the `variables` section using `${nr-var:variable_name}`. And this is valid for nested variables as well: following the example above, you would be able to use `${nr-var:log.info}`.
 
@@ -123,7 +117,7 @@ multi_line_string: |
     ${nr-var:yaml_variable | indent 2 }
 ```
 
-### OnHost Deployment
+### On Host Deployment
 
 For on-host deployment, use the following format:
 
@@ -158,7 +152,7 @@ In this section:
 * `health`: The measures used to check the health status of the agent.
 * `version`: The command used to check the version of the binary.
 * `executables`: This outlines the list of binaries the agent supervisor runs. Developers can define:
-  - * `id`: Unique identifier for the exec used by the health checker.
+  * * `id`: Unique identifier for the exec used by the health checker.
     * `path`: The location of the binary required.
     * `args`: The command-line arguments needed by the binary.
     * `env`: Specifies the required environment variables.
@@ -168,19 +162,19 @@ These diverse options offer extensive customization for your agent's deployment.
 
 #### Restart Policy
 
-`restart_policy` provides a set of instructions on how and when the agent process should be restarted. It's crucial for maintaining the agent's availability and reliability, particularly in case of unexpected failures or problems. 
+`restart_policy` provides a set of instructions on how and when the agent process should be restarted. It's crucial for maintaining the agent's availability and reliability, particularly in case of unexpected failures or problems.
 
 In the `backoff_strategy` we have:
 
-* `type`: This field can take several forms - _fixed_, _linear_, or _exponential_. It determines the delay timing strategy between retries.
-  * _fixed_: Constant delay interval between retries. This is the default type.
-  * _linear_: Delay interval increases linearly after each retry.
-  * _exponential_: Delay interval doubles after each retry.
-* `backoff_delay`: It defines the duration between retries when a restart is needed. This delay protects against aggressive restarts. Default is _2s_.
-* `max_retries`: This integer value defines the maximum number of retry attempts before exiting the retry mechanism and accepting the failure. Default is _0_.
-* `last_retry_interal`:  This is used to store the duration of the last delay. It can especially be relevant in case of _linear_ or _exponential_ backoff strategies where each retry level has a different delay value. Default is _600_.
+* `type`: This field can take several forms - `fixed`, `linear`, or `exponential`. It determines the delay timing strategy between retries.
+  * `fixed`: Constant delay interval between retries. This is the default type.
+  * `linear`: Delay interval increases linearly after each retry.
+  * `exponential`: Delay interval doubles after each retry.
+* `backoff_delay`: It defines the duration between retries when a restart is needed. This delay protects against aggressive restarts. Default is *2s*.
+* `max_retries`: This integer value defines the maximum number of retry attempts before exiting the retry mechanism and accepting the failure. Default is *0*.
+* `last_retry_interal`: This is used to store the duration of the last delay. It can especially be relevant in case of *linear* or *exponential* back-off strategies where each retry level has a different delay value. Default is *600*.
 
-#### OnHost Health
+#### On Host Health
 
 The `health` section in the deployment configuration is where you can specify how to monitor the health status of the agent. This is critical for maintaining the reliability of your agent and ensuring that it's functioning correctly. Here's how you can define it in the `executables` block:
 
@@ -196,10 +190,10 @@ health:
 
 In this configuration:
 
-* `interval`: This parameter specifies the frequency at which health checks should be performed. 
-* `timeout`: This is the maximum time the agent should wait for a health check response. 
+* `interval`: This parameter specifies the frequency at which health checks should be performed.
+* `timeout`: This is the maximum time the agent should wait for a health check response.
 * `http`: This section is for when agents expose their status through an HTTP endpoint. If this method is used, the `path` and `port` should be specified.
-  * `path`: This is the API endpoint for the health check. Typically it's a URI where the agent returns its current health status.
+  * `path`: This is the API endpoint for the health check. Typically, it's a URI where the agent returns its current health status.
   * `port`: This is the port on which the agent's health check endpoint is listening.
   * `healthy_codes`: This is a list of the HTTP codes the SA will consider as valid ones.
 
@@ -217,6 +211,7 @@ health:
     healthy_codes: [0] 
     unhealthy_string: ".*(unhealthy|fatal|error).*"
 ```
+
 ```yaml
 # ...
 health:
@@ -228,7 +223,7 @@ health:
     unhealthy_string: ".*(unhealthy|fatal|error).*"
 ```
 
-#### OnHost Version
+#### On Host Version
 
 The `version` section in the deployment configuration is where you can specify how to obtain the version of the binary running. Here's how you can define it block:
 
@@ -241,11 +236,10 @@ version:
 
 In this configuration:
 
-* `path`: Specifies the binary to run. 
-* `args`: Specifies the arguments passed to the binary to get the version. 
+* `path`: Specifies the binary to run.
+* `args`: Specifies the arguments passed to the binary to get the version.
 * `regex`: Optional field that specifies the regular expression to get the version from the output. When not used, the whole output will be used as the version.
 
-
 ### Kubernetes Deployment
 
 The Agent Control leverages [Flux](https://fluxcd.io/) to act as an operator running Helm commands (install, upgrade, delete) as needed based on the provided configurations.
@@ -383,65 +377,65 @@ This guideline shows how to build a custom agent type and integrate it with the
 
 1. Create a file with the agent type definition
 
-```yaml
-# namespace: newrelic, external, other
-namespace: external
-# name: reverse FQDN that uniquely identifies the agent type
-name: com.influxdata.telegraf
-# version: semver scheme
-version: 0.0.1
-
-# variables:
-#   common | on_host | k8s:
-#     my_var_1:
-#       description: "Variable description here"
-#       type: file | string
-#       required: true | false
-#       default: "default value"
-#       file_path: "persistence path"
-
-variables:
-  on_host:
-    config_file:
-      description: "Telegraf config file path"
-      type: string
-      required: false
-      default: "/path/to/telegraf.conf"
-    backoff_delay:
-      description: "seconds until next retry if agent fails to start"
-      type: string
-      required: false
-      default: 20s
-
-deployment:
-  on_host:
-    executables:
-      - id: telegraf
-        path: /usr/bin/telegraf
-        args: "--config ${nr-var:config_file}"
-        env: ""
-        restart_policy:
-          backoff_strategy:
-            type: fixed
-            backoff_delay: ${nr-var:backoff_delay}
-```
+    ```yaml
+    # namespace: newrelic, external, other
+    namespace: external
+    # name: reverse FQDN that uniquely identifies the agent type
+    name: com.influxdata.telegraf
+    # version: semver scheme
+    version: 0.0.1
+    
+    # variables:
+    #   common | on_host | k8s:
+    #     my_var_1:
+    #       description: "Variable description here"
+    #       type: string
+    #       required: false
+    #       default: "default value"
+    
+    variables:
+      on_host:
+        config_file:
+          description: "Telegraf config file path"
+          type: string
+          required: false
+          default: "/path/to/telegraf.conf"
+        backoff_delay:
+          description: "seconds until next retry if agent fails to start"
+          type: string
+          required: false
+          default: 20s
+    
+    deployment:
+      on_host:
+        executables:
+          - id: telegraf
+            path: /usr/bin/telegraf
+            args: "--config ${nr-var:config_file}"
+            env: ""
+            restart_policy:
+              backoff_strategy:
+                type: fixed
+                backoff_delay: ${nr-var:backoff_delay}
+    ```
 
 2. Copy the agent type definition to the folder `/etc/newrelic-agent-control/dynamic-agent-types`
-
 3. Use the new type in the `agents` config for the agent control:
-```yaml
-# fleet_control:
-# ...
 
-agents:
-  my-telegraf-collector:
-    agent_type: "external/com.influxdata.telegraf:0.0.1"
-```
+    ```yaml
+    # fleet_control:
+    # ...
+    
+    agents:
+      my-telegraf-collector:
+        agent_type: "external/com.influxdata.telegraf:0.0.1"
+    ```
 
 4. If any `required` variable has been defined in the type or any default value for variables needs to be customized, then define a `values.yaml` in `/etc/newrelic-agent-control/fleet/agents.d/my-telegraf/values.yaml`:
-```yaml
-config_file: /custom/path/to/file
-backoff_delay: 30s
-```
 
-5. Restart the agent control.
+    ```yaml
+    config_file: /custom/path/to/file
+    backoff_delay: 30s
+    ```
+
+5. Restart Agent Control.