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

daemon: add drop-in config fragments for custom daemon config #7784

Closed
wants to merge 1 commit into from

Conversation

liubin
Copy link
Contributor

@liubin liubin commented Dec 8, 2022

Motivation:

Some components, such as snapshotter needs to configure the containerd config file dynamically, it's not so easy to modify the config file, especially by Linux command such as sed because the toml file has its hierarchy relations, also it has comments in the file, so simply replacing or appending is not so easy to be used to update the containerd's config file.

Components write their config fragments, and letting them be imported by the import instrument is an ideal resolution, but modifying the imports instrument has the same problem as above, it's hard to edit it, for example, add, delete or change the order of the files in the import list.

Solution:

This commit will add the drop-in config fragments like other traditional Linux packages to containerd. Users can add their configs into the config.d directory along with the main config file(normally config.toml) but need not modify the main config file.

For example, if we have a config file like this:

imports = ["import-1.toml", "import-2.toml"]

And drop-in configs in config.d:

10-plugin-1.toml
20-plugin-2.toml
12-plugin-3.toml

The drop-ins will be processed seems to be added to the imports item in the main config file. The result equals:

imports = ["import-1.toml", "import-2.toml", "10-plugin-1.toml", "12-plugin-3.toml", "20-plugin-2.toml"]

And the drop-ins will have high priority than imported files.

Signed-off-by: bin liu liubin0329@gmail.com

@k8s-ci-robot
Copy link

Hi @liubin. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Motivation:

Some components, such as snapshotter needs to configure the containerd
config file dynamically, it's not so easy to modify the config file,
especially by Linux command such as `sed` because the toml file has
its hierarchy relations, also it has comments in the file, so simply
replacing or appending is not so easy to be used to update
the containerd's config file.

Components write their config fragments, and letting them be imported
by the `import` instrument is an ideal resolution, but modifying
the `imports` instrument has the same problem as above, it's hard
to edit it, for example, add, delete or change the order of the files
in the import list.

Solution:

This commit will add the drop-in config fragments like other traditional
Linux packages to containerd. Users can add their configs into the
`config.d` directory along with the main config file(normally `config.toml`)
but need not modify the main config file.

For example, if we have a config file like this:

```toml
imports = ["import-1.toml", "import-2.toml"]
```

And drop-in configs in `config.d`:

```
10-plugin-1.toml
20-plugin-2.toml
12-plugin-3.toml
```

The drop-ins will be processed seems to be added to the `imports` item in the
main config file. The result equals:

```toml
imports = ["import-1.toml", "import-2.toml", "10-plugin-1.toml", "12-plugin-3.toml", "20-plugin-2.toml"]
```

And the drop-ins will have high priority than imported files.

Signed-off-by: bin liu <liubin0329@gmail.com>
@mxpv
Copy link
Member

mxpv commented Dec 8, 2022

I'd prefer to have one way of doing things.
As we already rely on imports, should something like this work?:

imports = ["*-import.toml"]

(just needs masks support and sorting)

@liubin
Copy link
Contributor Author

liubin commented Dec 8, 2022

Hi @mxpv, only using imports can import any config pieces, but that needs users to change the imports manually in the config.toml file, this PR aims to use cases that users may don't want to edit the config.toml even, for example using shell scripts to configure containerd. That will be somewhat user-friendly.

@mxpv
Copy link
Member

mxpv commented Dec 9, 2022

But you still define main config in some form, right?
Users don't need to change anything in main config file every time.

You can define a default directory in the main config, where containerd will look for include files, like imports = ["/abc/config-d/*.toml"]. After that you just place any number of arbitrary config files that needs to be included. Main config will always be the same.

@liubin
Copy link
Contributor Author

liubin commented Dec 9, 2022

But you still define main config in some form, right? Users don't need to change anything in main config file every time.

You can define a default directory in the main config, where containerd will look for include files, like imports = ["/abc/config-d/*.toml"]. After that you just place any number of arbitrary config files that needs to be included. Main config will always be the same.

Thanks for your advice, putting all config pieces by adding imports = ["config-d/*.toml"] is a good choice, this PR is further to add this as the default through the program. Normally, users will define their main config. But considering that the containerd is configured by many users/groups/depts, everyone maybe doesn't want to touch the main config.

@k8s-ci-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed.

@github-actions github-actions bot added the Stale label Mar 21, 2024
Copy link

This PR was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants