Add instructions for creating systemd units#5
Open
jmeekhof wants to merge 4 commits intojaxxstorm:mainfrom
Open
Add instructions for creating systemd units#5jmeekhof wants to merge 4 commits intojaxxstorm:mainfrom
jmeekhof wants to merge 4 commits intojaxxstorm:mainfrom
Conversation
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 7efa8ee in 2 minutes and 28 seconds. Click for details.
- Reviewed
288lines of code in6files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. systemd/README.md:31
- Draft comment:
The README example passes '--config=%h/dnsscale.yaml' on ExecStart but the provided unit file (systemd/dnsscale.service) omits it. Please align the examples to avoid confusion. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. systemd/podman/Containerfile:9
- Draft comment:
Similar to the main Containerfile, consider explicitly providing an output flag (e.g. -o dnsscale) in the 'go build' command and ensure the ENTRYPOINT format is consistent. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The current code works fine without the explicit -o flag since Go will use the package name by default. The COPY command on line 12 shows it's already finding the binary correctly. The ENTRYPOINT format looks standard and correct. This seems like a style preference rather than a necessary change. Maybe there's a good reason to be explicit about the output location that I'm missing? The reference to "main Containerfile" suggests there might be consistency concerns across files. Even if there's another Containerfile with different conventions, this file works correctly as is. Being explicit about the output location doesn't add meaningful value here. The comment suggests a style change that doesn't improve functionality or clarity, and makes an unclear reference to ENTRYPOINT format issues that don't appear to exist.
3. systemd/podman/README.md:142
- Draft comment:
Minor grammatical correction: change 'allows' to 'allow' in the instructions regarding .bashrc. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and suggests a minor grammatical correction. It doesn't provide a code suggestion or address a potential issue in the code. According to the rules, purely informative comments should be removed.
4. systemd/podman/README.md:29
- Draft comment:
Consider standardizing configuration file paths between the systemd and Podman examples. The README for systemd uses '%h/dnsscale.yaml' while the Podman example mounts '%h/dnsscale/conf/dnsscale.yaml' to '/root/.dnsscale.yaml'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The inconsistency is real and could be confusing to users. However, there may be valid reasons for the different paths - Podman containers often need different file organization than native systemd services. The comment doesn't indicate a clear problem that needs fixing, just points out a difference. Per the rules, we should not make purely informative comments without a clear action needed. The paths being different might actually be intentional due to the different needs of containerized vs native services. Am I being too quick to assume this is a real issue? Good point - without understanding the architectural reasons for the different paths, this comment may be premature and potentially misleading. The comment should be deleted as it only points out a difference without clear evidence that standardization is needed or beneficial.
5. systemd/podman/README.md:3
- Draft comment:
Typo: In "using Quadlet's to manage services.", the apostrophe is likely unintentional. Suggest changing "Quadlet's" to "Quadlets". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment is technically correct about the grammar, it's a very minor issue in documentation. The meaning is still clear despite the typo. The rules state we should not make purely informative comments or unimportant ones. This seems to fall into that category. The incorrect apostrophe could be seen as affecting readability and professional appearance of the documentation. Documentation quality is important. While documentation quality matters, this is an extremely minor issue that doesn't impact understanding. The rules specifically say not to make comments about obvious or unimportant issues. This comment should be deleted as it addresses a trivial grammatical issue that doesn't materially impact the documentation's usefulness.
6. systemd/podman/README.md:143
- Draft comment:
Typo: The phrase "This will allows the user" contains an unnecessary 's'. Consider changing it to "This will allow the user". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the grammar error is real, our rules emphasize focusing on code logic and important issues. Grammar fixes in documentation, while correct, are very minor and could be seen as noise in the review process. This is especially true in a README file where the meaning is still perfectly clear despite the minor grammatical error. The comment is technically correct, but am I being too strict by suggesting we remove a valid correction just because it's minor? The rules explicitly state to avoid obvious or unimportant comments. This grammatical fix, while correct, doesn't impact understanding or functionality. We should remove this comment as it's too minor and doesn't affect code functionality or documentation clarity.
Workflow ID: wflow_CBkUApvA00uK4XfW
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| COPY go.mod go.sum ./ | ||
| RUN go mod download | ||
| COPY . . | ||
| RUN GOARCH=$TARGETARCH go build |
There was a problem hiding this comment.
Consider specifying the output binary name (using -o) and using the exec array form for ENTRYPOINT (as in the Podman variant) for consistency and to avoid shell issues.
Suggested change
| RUN GOARCH=$TARGETARCH go build | |
| RUN GOARCH=$TARGETARCH go build -o dnsscale |
| [Service] | ||
| Environment="PATH=/bin:/usr/bin:/usr/local/bin:%h/.local/bin:%h/go/bin" | ||
| EnvironmentFile=%h/dnsscale.env | ||
| ExecStart=%h/go/bin/dnsscale |
There was a problem hiding this comment.
Consider including a restart policy (e.g. Restart=on-failure) in the service file to improve resilience.
| @@ -0,0 +1,162 @@ | |||
| ## Podman Quadlet Instructions | |||
|
|
|||
| If you're running Podman, you also have the option of using Quadlet's to manage services. | |||
There was a problem hiding this comment.
Typo: change "Quadlet's" to "Quadlets" to use the correct plural form.
Suggested change
| If you're running Podman, you also have the option of using Quadlet's to manage services. | |
| If you're running Podman, you also have the option of using Quadlets to manage services. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces new containerization and systemd integration options for the
dnsscaleproject, along with comprehensive documentation for running the service under both standard systemd and Podman Quadlet environments. The changes provide example unit files, environment setup instructions, and scripts to streamline installation and deployment.Containerization and Build Improvements:
Containerfilefor building and packaging thednsscaleGo binary using Alpine Linux, enabling easier container-based deployments.Containerfilethat clones the repository, caches Go modules, and builds the binary for containerized environments.Systemd Integration and Documentation:
dnsscale.serviceunit file for user-level systemd, supporting environment files and custom paths for the executable.install_service.sh) to automate installation of the systemd unit file into the correct user configuration directory.systemd/README.mdfor configuring and runningdnsscaleas a user service, including environment and configuration file management.Podman Quadlet Support and Documentation:
systemd/podman/README.mdwith step-by-step instructions for runningdnsscaleusing Podman Quadlet, including example Quadlet unit files, environment setup, and advanced user management tips.Important
Adds systemd and Podman Quadlet support for
dnsscale, including containerization improvements and detailed documentation.Containerfilefor buildingdnsscaleGo binary using Alpine Linux.Containerfilefor cloning, caching, and building the binary.dnsscale.serviceunit file for user-level systemd with environment file support.install_service.shscript for automating systemd unit file installation.systemd/README.md.systemd/podman/README.mdwith instructions for Podman Quadlet setup.This description was created by
for 7efa8ee. You can customize this summary. It will automatically update as commits are pushed.