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

dietpi-ddns/update.sh does not log message #5954

Closed
jtmoon79 opened this issue Dec 4, 2022 · 6 comments · Fixed by #7309
Closed

dietpi-ddns/update.sh does not log message #5954

jtmoon79 opened this issue Dec 4, 2022 · 6 comments · Fixed by #7309
Labels
Enhancement 💨 Solution available 🥂 Definite solution has been done
Milestone

Comments

@jtmoon79
Copy link

jtmoon79 commented Dec 4, 2022

Creating a bug report/issue

Required Information

  • DietPi version |
    G_DIETPI_VERSION_CORE=8
    G_DIETPI_VERSION_SUB=11
    G_DIETPI_VERSION_RC=2
    G_GITBRANCH='master'
    G_GITOWNER='MichaIng'
    G_LIVE_PATCH_STATUS[0]='not applicable'
    
  • Distro version | bullseye

Additional Information (if applicable)

  • Software title | dietpi-ddns
  • Was the software title installed freshly or updated/migrated? Yes.
  • Can this issue be replicated on a fresh installation of DietPi? Yes.

Steps to reproduce

  1. run dietpi-ddns
  2. add an entry for a duckdns name
  3. this creates /var/lib/dietpi/dietpi-ddns/update.sh
curl -4 -sSfL 'https://www.duckdns.org/update?domains=my-domain.duckdns.org&token=ec793bf0-f379-470d-9635-cca24e2e3f7e' 2>&1 > /dev/null | logger -t dietpi-ddns -p 3
  1. tail systemctl logs
    journalctl -xf
  2. manually run /var/lib/dietpi/dietpi-ddns/update.sh

Expected behaviour

Should see logger message.

Actual behaviour

There is no logger message.

Extra details

In generated script /var/lib/dietpi/dietpi-ddns/update.sh, remove redirect to /dev/null/

#!/bin/dash
curl -4 -sSfL 'https://www.duckdns.org/update?domains=my-domain.duckdns.org&token=ec793bf0-f379-470d-9635-cca24e2e3f7e' 2>&1 | logger -t dietpi-ddns -p 3

Feature Creep

While you're at it, I recommend changing the script /var/lib/dietpi/dietpi-ddns/update.sh

  • adding a logger message about which domain will be updated
  • passing logger the CLI option -s (print stderr), so manual calls to the script will print something
#!/bin/dash
logger -t dietpi-ddns -s -- "update domain my-domain.duckdns.org"
curl -4 -sSfL 'https://www.duckdns.org/update?domains=my-domain.duckdns.org&token=ec793bf0-f379-470d-9635-cca24e2e3f7e' 2>&1 | logger -t dietpi-ddns -p 3 -s
@MichaIng MichaIng added this to the v8.12 milestone Dec 4, 2022
@MichaIng MichaIng removed the Bug 🐞 label Dec 4, 2022
@MichaIng MichaIng removed this from the v8.12 milestone Dec 4, 2022
@MichaIng
Copy link
Owner

MichaIng commented Dec 4, 2022

Thanks for reporting.

I was confused now as well, but it actually makes sense as it is: I didn't want to spam the system log with some cryptic or redundant successful HTTP response, but only when the request failed, implying error HTTP responses thanks to -f flag. Since STDERR is redirected to STDOUT first, it is passed to the logger correctly.

And since Cron cannot handle outputs, as long as no MTA is configured, I do not want anything on STDOUT or STDERR, which is the reason why stuff is send to system logger in the first place. As since this is also done when you manually call the script, having error messages duplicated to STDERR aren't such a significant benefit, is it?

The problem with adding the domain is, aside of that it again is the syslog spam I want to avoid (its trivial to check back in dietpi-ddns if you really forgot which domain you did configure), is that with some providers you do only send a token, no domain at all, e.g. FreeDNS.

@jtmoon79
Copy link
Author

jtmoon79 commented Dec 4, 2022

I didn't want to spam the system log with some cryptic or redundant successful HTTP response

I prefer to know when such updates have run. The per 30 minute rate wasn't too much IMO.

But it's up to you. This Issue could be closed as "WillNotFix" if you prefer.

@MichaIng
Copy link
Owner

MichaIng commented Dec 4, 2022

You see the executions by cron already:

journalctl -u cron

@MichaIng
Copy link
Owner

MichaIng commented Dec 6, 2022

Best to view everything DietPi-DDNS related:

journalctl --grep dietpi-ddns

Would it be sufficient to have this documented properly? I agree that it's important to have ways to see not only errors, but that it executed at all, as well. I just want to avoid redundancy to not spam the system log too much.

@jtmoon79
Copy link
Author

jtmoon79 commented Dec 6, 2022

Would it be sufficient to have this documented properly?

Perhaps a user hint describing that command journalctl --grep dietpi-ddns would be sufficient. The hint could be placed somewhere in the dietpi-ddns program, or at the tail-end of the stdout output.

$ dietpi-ddns
...
[ INFO ] DietPi-DDNS | Applying DietPi-DDNS Cron job...
[ INFO ] DietPi-DDNS activity can be reviewed with command:
         journalctl --grep dietpi-ddns

I agree that it's important to have ways to see not only errors, but that it executed at all, as well. I just want to avoid redundancy to not spam the system log too much.

Would a checkbox option for verbose logging be worthwhile? i.e. another line-item in dietpi-ddns main dialog that adjusts the script /var/lib/dietpi/dietpi-ddns/update.sh. When verbose is checked/selected within dietpi-ddns then the update.sh has the above-recommended logger statements. Otherwise, the update.sh script retains it's current behavior.
This is just an off-hand consideration.

Seeing that the tagged cron messages regarding dietpi-ddns are logged, this bug is low priority or WontFix.

@MichaIng MichaIng added this to the v8.12 milestone Dec 6, 2022
@MichaIng MichaIng modified the milestones: v8.12, v8.13 Dec 18, 2022
@MichaIng MichaIng modified the milestones: v8.13, v8.14 Jan 15, 2023
@MichaIng MichaIng modified the milestones: v8.14, v8.15 Feb 11, 2023
@MichaIng MichaIng modified the milestones: v8.15, v8.16 Mar 11, 2023
@MichaIng MichaIng modified the milestones: v8.16, v8.17 Apr 7, 2023
@MichaIng MichaIng modified the milestones: v8.17, v8.18 May 6, 2023
@MichaIng MichaIng modified the milestones: v8.18, v8.19 Jun 3, 2023
@MichaIng MichaIng modified the milestones: v8.19, v8.20 Jul 2, 2023
@MichaIng MichaIng modified the milestones: v8.20, v8.21 Jul 29, 2023
@MichaIng MichaIng removed this from the v8.21 milestone Aug 27, 2023
@MichaIng MichaIng added this to the v8.24 milestone Oct 21, 2023
@MichaIng MichaIng modified the milestones: v8.24, v8.25 Nov 19, 2023
@MichaIng MichaIng modified the milestones: v8.25, v9.0 Dec 20, 2023
@MichaIng MichaIng modified the milestones: v9.0, v9.1 Jan 20, 2024
@MichaIng MichaIng modified the milestones: v9.1, v9.2 Feb 20, 2024
@MichaIng MichaIng modified the milestones: v9.2, v9.3 Mar 17, 2024
@MichaIng MichaIng modified the milestones: v9.3, v9.4 Apr 16, 2024
@MichaIng MichaIng modified the milestones: v9.4, v9.5 May 13, 2024
@MichaIng MichaIng modified the milestones: v9.5, v9.6 Jun 10, 2024
@MichaIng MichaIng modified the milestones: v9.6, v9.7 Jul 10, 2024
@MichaIng MichaIng modified the milestones: v9.7, v9.8 Aug 26, 2024
@MichaIng MichaIng modified the milestones: v9.8, v9.9 Oct 18, 2024
@MichaIng MichaIng linked a pull request Dec 15, 2024 that will close this issue
MichaIng added a commit that referenced this issue Dec 15, 2024
- DietPi-DDNS | The "IPv6or4" option to update IPv6 only, if supported by server, network and provider, has been replaced with "IPv4and6". A server being reachable via IPv6 only is rarely wanted, as many networks do not support it. Instead, usually one will want to have it reachable via both, IPv4 as well as IPv6, which is now possible when using DietPi-DDNS, and the new default. If, e.g. for security reason, IPv6 only is wanted, this option of course remains available, like before. Many thanks to @LOGIN-TB for doing this suggestion: #7278
- DietPi-DDNS | The cron job does now log server response messages and connection errors separately with respective severities. Some DDNS providers do not return an HTTP error code, but an error text embedded into a regular HTTP 200 response. This, as well as success responses can now be seen via "journalctl -t dietpi-ddns". Many thanks to @jtmoon79 for doing this suggestion: #5954
@MichaIng
Copy link
Owner

I implemented it just now: #7309
curl errors and actual HTTP error response codes will be logged with error severity, and all other server responses separately with info severity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement 💨 Solution available 🥂 Definite solution has been done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants