Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement files mounted to a workload #393

Open
13 tasks done
inf17101 opened this issue Oct 11, 2024 · 14 comments
Open
13 tasks done

Implement files mounted to a workload #393

inf17101 opened this issue Oct 11, 2024 · 14 comments
Assignees
Milestone

Comments

@inf17101
Copy link
Contributor

inf17101 commented Oct 11, 2024

Implement the objects in the interface (external + internal) to allow files mounted to a workload at a certain mount point. The file counted shall be taken from the desiredState.configs field were configs for workloads can be defined. Ankaios must render the files content.

This is a sub task of #267, please have a look for the detailed prototype for configs and the current implementation.
To solve this issue the template rendering functionality of #267 must be already ready and working.

Example workload supporting files:

...
  dashboard_workload:
    ...
    files:
      - mountPoint: "/some/path/to/file.json"
        data: {{c1.json_data_file_content}}
      - mountPoint: "/some/path/to/bin_data"
        binaryData: {{c1.some_data}}
   ...

I have already introduced and experimented with my current configs implementation branch how to bring in this new layout of the files field in proto format. I achieved that CLI + Server accepts the new format. It's still in draft and contains only the necessary new objects and conversion functions, the rest is draft code. It is commit to separate branch: https://github.com/eclipse-ankaios/ankaios/tree/267_config_files_impl

Tasks:

  • Takeover the branch with already brought in new objects https://github.com/eclipse-ankaios/ankaios/tree/267_config_files_impl , merge the current main branch or at least the newest configuration implementation in the configs branches of other PRs
  • Call the existing ConfigRenderer to render also the files fields
  • Implement the mount of the files in the supported runtimes (podman)
  • Add check or documentation about not supported config files for podman-kube => use existing ConfigMaps feature
  • Integrate extracted filesystem module from Prohibit multiple agent starts with the same agent name #431 PRs and enhance with needed functionality
  • Add/extend utests and stests
  • Adapt and test resume & replace logic after server and/or agent restart with config files
  • Add stest(s)
  • Adapt requirements
  • Adapt sequence diagrams
  • Add/extend existing User documentation
  • Adapt the control interface examples and user tutorials
  • Test scenario Cannot create two agents with different users on the same host #181 again because of the subfolder splitting in each workload folder and the folder permissions problems
@krucod3 krucod3 added this to the v0.6 milestone Oct 16, 2024
@inf17101 inf17101 self-assigned this Dec 2, 2024
@inf17101
Copy link
Contributor Author

The config and binary files must be mounted as read only inside the container. When the binary file has executable permissions on the host system then it is also executable inside the container and can be mounted as read only as well.

@inf17101
Copy link
Contributor Author

inf17101 commented Jan 7, 2025

@krucod3: Like discussed the Ankaios agent shall create the files under /tmp/ankaios/<agent_name>_io/<workload_name>.<hash> within a separate folder config_files. Also the control interface pipes shall be grouped within a separate folder control_interface.

@inf17101
Copy link
Contributor Author

inf17101 commented Jan 9, 2025

@krucod3: After moving the control interface fifo files into the control_interface on my local branch, I remembered that #181 had permission issues causing that a second non-root agent could not be started when first an agent with root privileges (through systemd service) was started before. However, I can see that the agent file cli.rs creates the run folder /tmp/ankaios and the subfolder of the workload including the pipes is created with subsequent commands in the ControlInterface::new(...). The current fix of #181 leads to unnecessary folder permission expansion, because it always fetches the parent folder of the path when a new directory is created and sets file mode 777 to the parent path. This means that first the /tmp/ankaios is set to 777 mode on agent startup which is correct for the bug fix of the agent root scenario. However, I can see that if afterwards control interfaces are created then again the parent path gets 777 mode assigned as well, for example when creating the directories for /tmp/ankaios/agent_A_io/<workload.hash>/input, then the agent_A_io folder permissions are also expanded to 777, which is unnecessary, because only the /tmp/ankaios/ must be writable for non-root agents. Including my subfolder changes, now if a control interface is created the <workload.hash> becomes 777 file mode instead of agent_A_io because it is the parent path when having a subfolder control_interface now in the full path. I think a solution is here to only set 777 mode when the agent creates the /tmp/ankaios in the agent's cli.rs but not for every Directory::new call.

@krucod3
Copy link
Contributor

krucod3 commented Jan 10, 2025

@inf17101, the solution makes sense. Only the Ankaios run folder (/tmp/ankaios) needs to be accessible to everyone. The folders of the agents and all their content should be readable only by the creating agent for security reasons. It could well be that different agents with different security scopes are started on the same node.

@krucod3
Copy link
Contributor

krucod3 commented Jan 10, 2025

Thinking about it further, starting an agent with same name twice one after the other under different users will also lead to problems and is probably the reason why the agent folder were also accessible by all. Even if we delete the folder when the agent stops, it could be that an unordered stop leaves the Ankaios run folder dirty. I see two options here how the agent can behave to avoid this:

  • make the agent folders unique by suffixing them with a uid (or at least some random unique value)
  • failing the start of the agent with an appropriate message

I would go with the second approach as the use-case in more like a developer one.

@inf17101
Copy link
Contributor Author

After I have expressed my concerns about using the std::fs (standard library filesystem blocking IO methods) to @krucod3 and @christoph-hamm, we analyzed together the need for using the async tokio implementation instead. We came to the conclusion to use the async implementation for writing a file, but keeping the standard library methods for creating the config files directories recursively on the host file system. This is because a filesystem might support async writes but for directory creation there is no performance impact to expect.

@inf17101
Copy link
Contributor Author

I found out that handlebars-rs lib does escaping of special sequences like double quotes by default leading to problems when rendering YAML strings of config files including such sequences. There is an option I have found out in the utests of the handlebars-rs crate to disable this behavior. Now it works without escaping.

@inf17101
Copy link
Contributor Author

inf17101 commented Jan 20, 2025

@krucod3:
I continued with the implementation and I am now able to mount config files and binary files in workloads with the podman runtime. However, I'm thinking about the feasibility of our config files manifest interface for the podman kube runtime. After the first look the podman kube runtime provides the known manifests input, which supports ConfigMaps already (config file + binary file). If we would implement the file mount feature also in podman kube, I see the following points:

  • solving the same problem for which podman kube has already a solution (ConfigMaps that can be assigned to specific containers within a pod)
  • decision about which container our config files shall be mounted (we need an extra field containing the file and container relationship for the whole pod). Then we make implicitly assumptions about the runtimeConfig content, which shall be runtime specific
  • situations like same mount point of a config file from Ankaios and inside the ConfigMap

However, currently I have tried to template at least the content of the ConfigMap inside the runtimeConfig and unfortunately, when the string replacement contains new lines (like a custom nginx.conf config) then handlebars replaces just the string inside the runtimeConfig and the indentations of the replaced configs is not oriented to the parent key/values of the yaml files. This can be prevented if we implement the Ankaios file feature for podman kube because then the config file content is not a substring replacement, it is its own field written into a file on the host system.

@krucod3
Copy link
Contributor

krucod3 commented Jan 21, 2025

Wouldn't the problem with the line breaks occur also for the file mounts if the content of the file is templated and is rendered from an Ankaios config?

@inf17101
Copy link
Contributor Author

inf17101 commented Jan 21, 2025

Wouldn't the problem with the line breaks occur also for the file mounts if the content of the file is templated and is rendered from an Ankaios config?

With a config file mount the whole field is replaced by the file content and the file is written to the host file system before it is mounted into the container.

The problem is if the template is a sub string. Then indentation is lost when replacing the template with a string containing multiple newlines. A custom handlebars-rs helper function can be implemented allowing to specify the indentation level by the user when putting in the template string.

@inf17101
Copy link
Contributor Author

@krucod3: I have tried out some possibilities to enable handlebars-rs to consider the indent level when replacing a template with a multi-line string.

Example:

For now we are not able to use the podman-kube's ConfigMap feature with allowing the user to use our templated handlebars syntax like the following:

workloads:
  kube_workload_with_mounted_text_file:
    agent: agent_B
    runtime: podman-kube
    configs:
      nginx_conf: file_data
    runtimeConfig: |
      manifest: |
        apiVersion: v1
        kind: ConfigMap
        metadata:
          name: nginx-config
        data:
          nginx.conf: |
            {{nginx_conf.web_server_config}}
        ---
        apiVersion: v1
        kind: Pod
        metadata:
          name: nginx-pod
        spec:
          restartPolicy: Never
          containers:
          - name: nginx-container
            image: ghcr.io/eclipse-ankaios/tests/nginx:alpine-slim
            ports:
            - containerPort: 80
              hostPort: 8080
            volumeMounts:
            - name: nginx-config-volume
              mountPath: /etc/nginx/nginx.conf
              subPath: nginx.conf
          volumes:
          - name: nginx-config-volume
            configMap:
              name: nginx-config

configs:
  file_data:
    web_server_config: |
      worker_processes  1;

      events {
          worker_connections  1024;
      }

      http {
          server {
              listen 80;
              server_name custom_nginx;

              location /custom {
                  default_type text/plain;
                  return 200 "The mounted custom nginx.conf is being used!\n";
              }
          }
      }

This would lead to a substitution in with wrong indentation:

Partial view to the rendered output:

manifest: |
  apiVersion: v1
  kind: ConfigMap
  metadata:
    name: nginx-config
  data:
    nginx.conf: |
      worker_processes  1;
      
events {
    worker_connections  1024;
}

http {
    server {
        listen 80;
        server_name custom_nginx;

        location /custom {
            default_type text/plain;
            return 200 "The mounted custom nginx.conf is being used!\n";
        }
    }
}
  ---
  apiVersion: v1
  kind: Pod
  ...

When trying out to resolve this with a custom helper in handlebars-rs, which is basically a method you register that can do custom rendering, then the problem is that you need to define a new helper but the indentation level must be specified by the user as argument which is ugly. It looks like the following in the runtimeConfig: {{indent 6 nginx_conf.web_server_config}} (6 means add 6 whitespaces at the front and your custom function will do that for all lines).

I have found out another solution now which might be a little bit better. You can register a partial and pass in the content dynamically.

...
handlebars
    .register_partial("indent", "{{content}}")
    .unwrap();
...

Partials keep indentation level but must be registered at compile time in handlebars-rs.

The user input to keep indentation looks like level the following:

...
        data:
          nginx.conf: |
            {{> indent content=nginx_conf.web_server_config}}
...

Now, without any further code, the indentation is kept and the rendered output looks correct:

  apiVersion: v1
  kind: ConfigMap
  metadata:
    name: nginx-config
  data:
    nginx.conf: |
      worker_processes  1;
      
      events {
          worker_connections  1024;
      }

      http {
          server {
              listen 80;
              server_name custom_nginx;

              location /custom {
                  default_type text/plain;
                  return 200 "The mounted custom nginx.conf is being used!\n";
              }
          }
      }
...

Still the user has to use an extra "indent" call in the template, but now at least this workload I trying to deploy is running now and works.

@krucod3
Copy link
Contributor

krucod3 commented Jan 22, 2025

@inf17101, the second solution is definitely better then having to specify the exact number of white spaces, but it's still something the uses must enter. It would be great if we can somehow avoid this, as probably not everybody will read the complete documentation ...
If we don't find any other option, we will need to leave it with the partial.

@inf17101
Copy link
Contributor Author

inf17101 commented Feb 3, 2025

We agreed on the partial implementation solution. It is added within an extra PR as it has nothing to do with this issue directly. It was just detected within this issue: #445

@inf17101
Copy link
Contributor Author

For adapting the SDK, the issue eclipse-ankaios/ank-sdk-python#54 was created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants