Skip to content

config: support for including files & directories (conf.d) - #856

Merged
problame merged 18 commits into
zrepl:masterfrom
zeyadtamimi:master
Jan 19, 2026
Merged

problame merged 18 commits into
zrepl:masterfrom
zeyadtamimi:master

Conversation

@zeyadtamimi

Copy link
Copy Markdown
Contributor

**Note: This is a WIP PR that presently only adds the documentation for the features so that proposal can be discussed as per #708 **

Background

While trying to use zrepl in my ansible driven home lab deployment I ran into an interesting problem. My ZFS based servers subscribe to different sometimes overlapping roles.

Example role distribution between two servers:

serverA:
  - common
  - web
  - file

serverB:
  - common
  - git
  - file

Each role wants to create and manage a ZFS dataset with its own replication / backup policies:

  • web: pool/web
  • git: pool/git
  • file pool/file

At present the creation of a ZFS dataset from each role role is somewhat very easy, so to is the creation of the basic zrepl configuration file from the "common" role.

However, when each role tries to register it's job(s) into the singular zrepl configuration files things get tricky.

I could try adding a role at the end that hardcodes the datasets that need to be backed up but that seems a bit hacky.

I could also use ansible's lineinfile task to try to idempotently add each dataset's snapshot jobs to the zrepl configuration files but that causes a problem:

Everytime the "common" role gets run the basic zrepl configuration file gets re-created causing the web, git, and file roles to all register changes as they have to re-insert all jobs back into the singular configuration file.

Proposed Solution

The proposed solution is to allow for the distribution of zrepl job definition between multiple different YAML files that can be included from the main zrepl configuration files.

global: ...
jobs:
  include: jobs.d

This directive would be only acceptable in the main configuration file and is mutually exclusive with any other job definitions in the file.

To keep things lean there will be no conflict resolution provided to users, job names must be unique across all included job YAML files.

With this feature, the above problem becomes much simpler:

  • Common: Sets up the global zrepl configuration and the include directive
  • web/git/file: Each manage their own datasets and create their jobs.d/web.yml, jobs.d/git.yml, and jobs.d/file.yml.

**Note: This is a WIP PR that presently only adds the documentation for
the features so that proposal can be discussed.**

Background
==========

While trying to use zrepl in my ansible driven home lab deployment I ran
into an interesting problem. My ZFS based servers subscribe to different
sometimes overlapping roles.

Example role distribution between two servers:
```
serverA:
  - common
  - web
  - file

serverB:
  - common
  - git
  - file
```

Each role wants to create and manage a ZFS dataset with its own
replication / backup policies:
   - web: pool/web
   - git: pool/git
   - file pool/file

At present the creation of a ZFS dataset from each role role is somewhat
very easy, so to is the creation of the basic zrepl configuration file
from the "common" role.

However, when each role tries to register it's job(s) into the singular
zrepl configuration files things get tricky.

I could try adding a role at the end that hardcodes the datasets that
need to be backed up but that seems a bit hacky.

I could also use ansible's `lineinfile` task to try to idempotently add
each dataset's snapshot jobs to the zrepl configuration files but that
causes a problem:

Everytime the "common" role gets run the basic zrepl configuration file
gets re-created causing the web, git, and file roles to all register
changes as they have to re-insert all jobs back into the singular
configuration file.

Proposed Solution
=================

The proposed solution is to allow for the distribution of zrepl job
definition between multiple different YAML files that can be included
from the main zrepl configuration files.

```
global: ...
jobs:
  include: jobs.d
```

This directive would be only acceptable in the main configuration file
and is mutually exclusive with any other job definitions in the file.

To keep things lean there will be no conflict resolution provided to
users, job names must be unique across all included job YAML files.

With this feature, the above problem becomes much simpler:
   - Common: Sets up the global zrepl configuration and the include
     directive
   - web/git/file: Each manage their own datasets and create their
                   jobs.d/web.yml, jobs.d/git.yml, and jobs.d/file.yml.

@InsanePrawn InsanePrawn left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a typo and suggested some improvements for clarity.

Generally a good proposal IMO.

I think the format could use some discussion.
I've got some questions:

  1. Why put the includes under jobs:?
    Oh, I see you copied it from the issue. In that case @problame: I suspect it's because you wanted to emphasize the fact that only job definitions should go in other config files, but why not raise an error for now on non-job keys in extension files and allow that possibility later if we find it beneficial?
  2. Why only allow a single include?
  3. Should we allow the user to specify a single file (jobs.d/ vs jobs.d/specificfile.yml)?

Tl;dr: is there a reason why it's not

includes:
  - jobs.d/
  - /etc/zrepl/this_one_really_weird_file.yml

Comment thread docs/configuration/misc.rst Outdated
Comment thread docs/configuration/misc.rst Outdated
Comment thread docs/configuration/misc.rst Outdated
@zeyadtamimi

Copy link
Copy Markdown
Contributor Author

Thanks for the review

1. Why put the includes under `jobs:`?
   Oh, I see you copied it from the issue. In that case @problame: I suspect it's because you wanted to emphasize the fact that only job definitions should go in other config files, but why not raise an error for now on non-job keys in extension files and allow that possibility later if we find it beneficial?

Yeah I tried to preserve some of the suggestions in the original issue. I personally agree with you that having a separate key makes more sense. Though I would prefer something more explicit like job_includes.

2. Why only allow a single include?

No particular reason other than to simplify the potential implementation ( e.g. Zrepl does not need to handle the case where the file a/b.yml is included along with the directory a/

3. Should we allow the user to specify a single file (`jobs.d/` vs `jobs.d/specificfile.yml`)?

I am not opposed to the idea, It just does not really benefit the proposed use case where the entity writing the configuration file is different than the one writing the jobs file.

Aside from my preference for job_includes I don't really see an issue with your suggested improvements to the PR.

Let me update the request.

@problame problame left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for going docs first!

I like the proposal with the top-level include + only allowing jobs as a top-level key in the included files.
Keeps things open for extension later on.

Also, it makes it removes ambiguity with regards to how one parses jobs.


Please digest the feedback into an update to this PR (just new commits, no force pushes please) and ask for re-review.

I think after that implementation can start.

Comment thread docs/configuration/misc.rst Outdated
Comment thread docs/configuration/misc.rst Outdated
As per some of the discussion items I modified the proposal to have a
special top level includes key as well as added references to handling
of specific file includes.
@zeyadtamimi

Copy link
Copy Markdown
Contributor Author

Thanks for the review.

I like the proposal with the top-level include + only allowing jobs as a top-level key in the included files. Keeps things open for extension later on.

In that case should we still have jobs and the include token be mutually exclusive in the main configuration file?

@zeyadtamimi

Copy link
Copy Markdown
Contributor Author

@problame I modified the proposal based on your and @InsanePrawn's comments.

Please take a look and let me know what you think of the new usage semantics. I also opted to move the documentation into a new file rather than cramming it into misc.

dsh2dsh added a commit to dsh2dsh/zrepl that referenced this pull request Dec 11, 2024
```yaml
include_jobs: "jobs.d/*.yaml"
```

Like `include_keys`, the directory is relative to main configuration file.

`include_jobs` can be combined with `jobs`:

```yaml
jobs:
  - name: "zroot-to-zdisk"
include_jobs: "jobs.d/*.yaml"
```

See also zrepl/zrepl#856
zrepl/zrepl#856
@zeyadtamimi

Copy link
Copy Markdown
Contributor Author

@problame gentle bump on the review

@zeyadtamimi

Copy link
Copy Markdown
Contributor Author

@problame I have finished an initial implementation and added some basic tests.

Are you happy with the usage semantics? If so I will push the initial implementation into the PR.

@InsanePrawn

Copy link
Copy Markdown
Contributor

I have finished an initial implementation and added some basic tests.

Very nice! Please do push your implementation, we can always change the code.

In that case should we still have jobs and the include token be mutually exclusive in the main configuration file?

IMO no, we're gonna need to detect job name conflicts between drop-in files anyway, no need to treat the main config file differently IMO. (Am i overlooking anything?)

A relatively straight forward change that adds "include" key support in
the main configuration file.

The config parser uses the list under this key to open the included
configuration files parse them as configs and append their jobs to the
main config file.
@zeyadtamimi

Copy link
Copy Markdown
Contributor Author

Hey @InsanePrawn / @problame,

Sorry this got dropped from my radar for a while since I was struggling with getting the lint job to pass. Finally got back to it. let me know what you think

@zeyadtamimi

Copy link
Copy Markdown
Contributor Author

@InsanePrawn / @problame

Gentle bump, are you folks happy with the code.

@problame problame left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made some edits to docs & code.

Will do a follow-up PR that improves test coverage of job names not overlapping, the zrepl daemon command currently panics if that is the case (good) but stuff like zrepl configcheck gives a straight pass, before this PR and after this PR.

@problame problame changed the title Added distributed job YAML file support config: support for including files & directories (conf.d) Jan 19, 2026
@problame
problame merged commit 4d6583e into zrepl:master Jan 19, 2026
11 checks passed
@problame

Copy link
Copy Markdown
Member

problame added a commit that referenced this pull request Jan 19, 2026
Before this PR, config parsing would accept duplicate job names. `zrepl
daemon` would later fail to start with a panic. But tools like `zrepl
configcheck` would pass.

This PR adds a check to ensure job names are unique.

Similarly, internal job names were not being rejected by config parsing
Move that check to parse-time as well.

Last, drive-by change: remove `internal/config/config_include_test.go`
introduced in #856 . These aren't necessary because there is already a
wildcard test for all valid configs.
This PR adds the complimentary "invalid config" wildcard test.
dsh2dsh added a commit to dsh2dsh/zrepl that referenced this pull request Jan 19, 2026
Discard zrepl/zrepl#856. Implemented here long time ago.
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

Successfully merging this pull request may close these issues.

3 participants