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

Add systemd unit file for non docker users. #139

Merged
merged 10 commits into from Jul 28, 2022
Merged

Conversation

symgryph
Copy link
Contributor

No description provided.

YOu need to create the ddns user, and you need to create the ddns group, and obviously make sure you have the environments file.
This is a unit file, with a couple of caveats
contrib/cloudflare.service Outdated Show resolved Hide resolved
contrib/cloudflare.service Outdated Show resolved Hide resolved
@favonia
Copy link
Owner

favonia commented Apr 11, 2022

@symgryph Thanks. The systemd file looks great! I left a few comments and have two additional questions:

  1. Would you mind adding a template file for EnvironmentFile?
  2. Do you feel more comfortable assigning the copyright of your contribution to me (this is easier for me in the future in case something happens, but it is sacrificing your right and requires trust) or keeping the copyright but releasing it under the same license (the default, which is also implied by the GitHub term of use)? In either case, we should update either AUTHORS or CONTRIBUTORS to record your credit.

Again, thanks for your contribution. ❤️

@munntjlx
Copy link

This is my second account, just an update. I will include requested env and update my systemd configuration. I might even secure it a bit more.

@munntjlx
Copy link

Did you want a .md doc explaining how to use the systemd portion?

@favonia
Copy link
Owner

favonia commented Apr 13, 2022

Did you want a .md doc explaining how to use the systemd portion?

That would be helpful. Thanks. 🙏

@Macleykun
Copy link

Any update on this PR :) ?

@codecov
Copy link

codecov bot commented May 9, 2022

Codecov Report

Merging #139 (a9d24ba) into main (393f9f6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #139   +/-   ##
=======================================
  Coverage   83.31%   83.31%           
=======================================
  Files          27       27           
  Lines        1576     1576           
=======================================
  Hits         1313     1313           
  Misses        257      257           
  Partials        6        6           
Flag Coverage Δ
unittests 83.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@favonia
Copy link
Owner

favonia commented May 9, 2022

@munntjlx @symgryph Sorry I did not realize you are making a PR from your main branch. I hope my updating this PR did not disturb your production builds.

@munntjlx
Copy link

munntjlx commented May 9, 2022 via email

@munntjlx
Copy link

I will be working on this tonight. Sorry for the delay!

Copy link
Contributor Author

@symgryph symgryph left a comment

Choose a reason for hiding this comment

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

hopefully these are sufficient.....

@favonia
Copy link
Owner

favonia commented Jul 7, 2022

@symgryph Thanks. I will check this PR on Friday Saturday. In the meanwhile, I wonder if you could help me understand why we need these?

Delegate=yes
LimitNOFILE=infinity
LimitNPROC=infinity
LimitCORE=infinity

@symgryph
Copy link
Contributor Author

symgryph commented Jul 7, 2022 via email

@favonia
Copy link
Owner

favonia commented Jul 10, 2022

I'm merging this. And then, I'm going to list you as a contributor, but in a different PR so that you can "approve". (You cannot approve your own PR.)

Copy link
Contributor Author

@symgryph symgryph left a comment

Choose a reason for hiding this comment

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

Feel free to remove the 'ip4/ip6' but it does work here.

@symgryph
Copy link
Contributor Author

symgryph commented Jul 11, 2022 via email

@favonia
Copy link
Owner

favonia commented Jul 11, 2022

I played with it last night and I found a better way to specify just IPv6 or IPv4.

Out of curiosity, what is the better way? Also, does this mean we should remove IP4,IP6? In other words, what should we do about the pull request here?

Also is there a way to actually specify these environment variables in a file? I was trying to write an open RC version and it's very hard to set environment variables inside of OpenRC scripts.

There are some design difficulties---some environment variables are actually not directly read by this tool. However, I don't immediately see the difficulty of doing what you want in OpenRC. An OpenRC service script is a shell script. I believe you can create an environment file as follows:

export CF_API_TOKEN=...
export DOMAINS=...

And then, source this environment file in your OpenRC service script. source should make the new values available to the OpenRC interpreter and this DDNS tool. Did I miss something?

@favonia
Copy link
Owner

favonia commented Jul 28, 2022

I am going to merge this PR first.

@favonia favonia merged commit bbe48ae into favonia:main Jul 28, 2022
@symgryph
Copy link
Contributor Author

symgryph commented Oct 11, 2022 via email

@favonia
Copy link
Owner

favonia commented Oct 11, 2022

I'm glad it worked. Let me know if there's some problem.

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.

None yet

4 participants