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.

Sign up for GitHub

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

Jump to bottom

DSM7 #34

Open
christianmeier opened this issue Jan 23, 2021 · 68 comments
Open

DSM7 #34

christianmeier opened this issue Jan 23, 2021 · 68 comments

Comments

@christianmeier
Copy link

christianmeier commented Jan 23, 2021 •
edited
Loading

New path dhcpd.conf.leases
DSM6 ATIME=stat /etc/dhcpd/dhcpd-leases.log | grep Modify
DSM7 ATIME=stat /etc/dhcpd/dhcpd.conf.leases g | grep Modify
EDIT:dhcpd.conf.leases
@gclayburg
Copy link
Owner

Hi. Thanks for noticing this.

I haven't looked at this in a while, but on my system, I do see both of those files. For me they are both have the same contents and the same timestamp. Are you saying something needs to be changed here?

@christianmeier
Copy link
Author

christianmeier commented Jan 23, 2021 via email

@gclayburg
Copy link
Owner

Oh that could be. I haven't tried these scripts with that version yet. There definitely were some changes we needed to do for the DSM 5 to version 6 some time ago. Maybe synology is using a newer version of DNS?

@christianmeier
Copy link
Author

christianmeier commented Jan 27, 2021 •
edited
Loading

The script is working as it should.
I'd changed the following line from reload to restart
It seems that reload does not publish the changes. (Changes are added and removed to the zones correctly) and only a restart make them available to query.

$ZoneRootDir/script/reload.sh
root@nas:/var/services/homes/admin/scripts# cat diskstation_dns_modify.sh |grep restart
$ZoneRootDir/script/restart.sh

@gclayburg
Copy link
Owner

Ok. What version of the DNS package are you running?

@christianmeier
Copy link
Author

christianmeier commented Jan 28, 2021 via email

@gclayburg
Copy link
Owner

Ok. I am running version 2.2.2 5027 and I am not seeing this problem. The standard reload.sh seems to work fine. My concern in changing the script to do a restart is that it could lead to more hiccups for everyday clients while the service is restarted. And this script can run quite often especially on a network with several dhcp clients.

Are you sure the restart is required for you? Or was there some sort of timeout involved when you were testing? Could you try it again with a new dhcp lease and a reload.sh step in the script?

@christianmeier
Copy link
Author

christianmeier commented Jan 28, 2021 via email

@sw2828
Copy link

sw2828 commented May 12, 2021

Hi @gclayburg ,
I have been having the same issue as @christianmeier over the past few months.. At one point DHCP clients were dynamically registering to DNS and I was able to resolve the FQDNs(My first to systems). Now the DHCP clients dynamically register to DNS, but they aren't published until I restart the DNS service with the following command: "/volume1/@appstore/DNSServer/script/restart.sh". Once I run that, everything resolves with no issue.
So, before I restart DNS, the records are in the table, but nslookup fails until I restart. This sounds like the same issue @christianmeier had.

I now have the same issue on all three of my Synology systems in my labs. Two of them worked for a while, but then stopped. One is a new install and does not work.

I'm running the following versions:
DSM DNS
NAS61 DSM 6.2.3-25426 Update 3 2.2.2-5027
NAS08 DSM 6.2.3-25426 2.2.1-5024
NAS17 DSM 6.2.4-25556 <having an issue displaying version, but this is the newest NAS>

I have not tried @christianmeier work around yet since I just came across this thread. I'm waiting to upgrade all my systems to the same versions because I was thinking this was a version issue, but as you can see, my issues are with various version.

Please let me know if you have been able to determine the root cause and can provide a fix.

Thank you.

@gclayburg
Copy link
Owner

Hmm. I have not seen this issue on my synology diskstation. I wonder if your system is doing more DHCP changes than my network? Maybe DHCP is assigning new IP addresses to existing names more often? Do you see this issue for a new DHCP client being added to the network, i.e. a using a name that did not exist in DNS? Or does this issue only occur for existing DHCP clients?

BTW, I'm not sure if it matters, but I am using the latest DNS version now:
2.2.3-5028

Although for some reason, I'm not able to bring up the DNS package details screen from the package center using the web UI. To me this looks like some sort of strange UI issue, unrelated to this problem. I do see the version number for DNSServer in the /var/log/synopkg.log file like this:

2021/04/24 08:14:33	upgrade DNSServer 2.2.3-5028 Begin preinst

@sw2828
Copy link

sw2828 commented May 12, 2021 •
edited
Loading

@gclayburg ,
Thanks for the quick response.

I had a good amount of time to troubleshoot today and I found the root cause, but not a complete solution.

The issues is that I configure all my DNS server 'Zone Update Rules' with a key. This way only systems with the same key can update the zone.

When I remove this rule completely, the zone gets updated as expected with your script and when I run the reload.sh command manually. When I add the key rule, the zone does not get updated with your script or the manual command. This makes perfect sense.

So, in order to try to resolve the issue, I tried a few things that did not work and I'm hoping someone can point me in the right direction.

Here's what I tried.

  1. Remove rule completely. Everything works as expected with your script. Command below is successful also.
    /var/packages/DNSServer/target/bin/rndc -k /var/packages/DNSServer/target/named/rndc.key reload test.lab.com

  2. Use the HMAC-SHA512 rndc key that I created through the DNS key GUI (DNS, Keys, Create, Key Name: xxx, Algorithm: HMAC-SHA512
    -Run the following command and get an error below
    /var/packages/DNSServer/target/bin/rndc -k /volume1/@appstore/DNSServer/named/etc/key/rndcupdatekey reload test.lab.com
    "rndc: unsupported algorithm: HMAC-SHA512"

  3. Use the HMAC-MD5 rndc key that I created through the DNS key GUI (DNS, Keys, Create, Key Name: xxx, Algorithm: HMAC-MD5
    -Run the following command and get an error below
    /var/packages/DNSServer/target/bin/rndc -k /volume1/@appstore/DNSServer/named/etc/key/rndcupdatekey reload test.lab.com
    **rndc: connection to remote host closed
    This may indicate that

    • the remote server is using an older version of the command protocol,
    • this host is not authorized to connect,
    • the clocks are not synchronized, or
    • the key is invalid.**

  4. Use the default key that works with the original command by exporting it, then importing it using the DNS key GUI (DNS, Keys, import, browse for rndc key I exported from /var/packages/DNSServer/target/named/rndc.key)
    -This failed with an error "This name is reserved for system use. Please enter a different name.".
    -I then rename the file, but get same error. It doesn't seem to allow me to import the default system's rndc key.

Do you have any ideas about the rndc key?

########################
As for the DNS versions...

I wanted to upgrade DNS, but I didn't want to break anything further since I know it did work at one time on the current version of the two systems. Now that I know it's not a version issue, I will go ahead and upgrade to the latest.

Your command to get the DNS version worked. So I'm running three different versions, all have the same issue.
NAS.......DSM.................................................DNS
NAS61 DSM 6.2.3-25426 Update 3....2.2.2-5027
NAS08 DSM 6.2.3-25426.......................2.2.1-5024
NAS17 DSM 6.2.4-25556.......................2.2.3-5028

@gclayburg
Copy link
Owner

ok, so the script is working for you but you are having trouble configuring zone update rules? I haven't tried using anything other than the default, so I can't offer much help on that.

@sw2828
Copy link

sw2828 commented May 13, 2021

@gclayburg ,
Thank you. If I figure out a way to do it, I will post my solution here.

@christianmeier
Copy link
Author

@ALL If I can support you in case. let me know

@sw2828
Copy link

sw2828 commented May 14, 2021

@christianmeier,
Thank you.
I have tried many options, but I haven not been able to get this working when 'Zone Update Rules' are enabled . If you have time to look into this further, that would be a great help. At this point, I'm out of ideas. I know this isn't directly related to this script, but if anyone does have 'Zone Update Rules' enabled, the script won't work.

The quickest way to test is running the following command, changing the full path of the rndc key and the domain name.
/var/packages/DNSServer/target/bin/rndc -k /volume1/@appstore/DNSServer/named/etc/key/rndcupdatekey reload test.lab.com

@brainchild0
Copy link

Would I understand accurately that as of now the main branch is not reliable for use with DSM 7?

@dougmeek
Copy link

dougmeek commented Jul 5, 2021

@brainchild0 I'd say it's stable, but only after updating the command in poll-dhcp-changes.sh to be:

ATIME='stat /etc/dhcpd/dhcpd.conf.leases g | grep Modify'

@brainchild0
Copy link

Is there any reason for not introducing this change into the distribution?

@christianmeier
Copy link
Author

christianmeier commented Jul 5, 2021 via email

@dougmeek
Copy link

dougmeek commented Jul 6, 2021

@brainchild0 I actually submitted a pull request yesterday to do exactly that. Someone with the correct permissions needs to approve and merge, but based on the other two that have been sitting out there for a while I wouldn't expect it to happen in the near future. I know @gclayburg hasn't spent much time on this script lately, and that's totally understandable. You may just need to make the change yourself.

@gclayburg
Copy link
Owner

Hi guys - I should have some time later tonight to look at this. My issue though is that I'm using an older synology that can't run DSM 7 so I can't test it the way I'd like. I can at least verify that it doesn't break anything with DSM 6 though.

@christianmeier
Copy link
Author

christianmeier commented Jul 7, 2021 via email

@brainchild0
Copy link

I see the PR, so it seems one option is that I simply take the snapshot from a10c15c. However, it would be nice to have a merge, or any more official pathway for obtaining a functional snapshots.

@gclayburg
Copy link
Owner

Ok guys, it looks like we need some better error detection for poll-dhcp-changes.sh. So I'm looking on my system running DSM 6 and I do see both of these files:
/etc/dhcpd/dhcpd-leases.log
/etc/dhcpd/dhcpd.conf.leases

From what I see both of these files get modified when a new DHCP lease occurs. Does this always happen? I've been using the script for years with the script only doing a reload when the /etc/dhcpd/dhcpd-leases.log file changes. Apparently this doesn't always happen anymore under DSM7?

I want to fix this to work under DSM 7, but I don't want to break anything for DSM 6 users like me. So I think what I might do is change the script to do a DNS reload if either of these files are modified.

@dougmeek Thank you for your contribution. If I run the command in your patch I get an error. Is this intentional or just an oversight? I bet it probably works though.

# stat /etc/dhcpd/dhcpd-leases.log g
  File: ‘/etc/dhcpd/dhcpd-leases.log’
  Size: 587       	Blocks: 8          IO Block: 4096   regular file
Device: 900h/2304d	Inode: 83866       Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2021-07-10 16:23:12.184351925 -0600
Modify: 2021-07-10 16:23:08.910381405 -0600
Change: 2021-07-10 16:23:08.910381405 -0600
 Birth: -
stat: cannot stat ‘g’: No such file or directory

gclayburg added a commit that referenced this issue Jul 11, 2021
@gclayburg
fix for #34
fa9da40 
better error checking for DHCPD files
Apparently DSM 7 may not always have a dhcpd-leases.log file
@gclayburg
Copy link
Owner

FYI - I plan on testing this change I just created for the next few days on DSM 6. Any volunteers to test this change on DSM 7?

@christianmeier
Copy link
Author

christianmeier commented Jul 11, 2021 via email

@dougmeek
Copy link

dougmeek commented Jul 11, 2021 •
edited
Loading

@gclayburg I think that may have been a typo on my part. It was supposed to be:

stat /etc/dhcpd/dhcpd.conf.leases | grep Modify

As far as the difference between versions goes, if both files are present and updated on DSM 6, I suggest that we target the file that exists on DSM 7. That said, it probably wouldn't be too difficult to build an if statement to determine the release version, after determining the version:

cat /etc/VERSION | grep majorversion

Give me a bit and I'll update my PR to reflect the correct command, that targets both versions.

I appreciate you looking at this.

EDIT: I didn't see your updates after your message to me because I was looking on mobile. Looks like you've already made a change to address this. I'll throw it up on my DSM 7 box and give it a try. Will let you know what I see in a bit. I'll kill a bunch of clients and have them get new addresses, which should generate some tests.

@dougmeek dougmeek mentioned this issue Jul 11, 2021
Closed
@dougmeek
Copy link

@gclayburg I've dropped the change on my NAS. I'm seeing a 100% success rate on 28 systems checking back in with new IPs. The old records are being removed/updated, and the new IPs are correct both in the forward and reverse zones. Additionally for context, I'm using a class B reverse zone rather than a class C, and it's working as expected.

I'd call that a success on DSM 7. I don't have a DSM 6 box laying around to test with, but I'm good with this change from my testing results.

Thanks for looking into this.

@christianmeier I'd be down with integrating the log file to the DSM Log Center, but I don't even know how to go about doing that. If you do, that would be a pretty nice enhancement. I'd even help you work on the coding of it if you had some knowledge of the logging mechanisms of DSM. My guess is that it uses the /var/log files but I have no idea.

@christianmeier
Copy link
Author

It's running stable as said - but I had issues with the syno dns app to manage dns entries. I saw, that the file permission of the zones belong to nobody:nobody. This was causing that I was not able edit the zones.

@brainchild0
Copy link

@christianmeier: To me, this observation would mean, unfortunately, that the package is not yet stable.

You said you tried to make a branch. Do you have a working solution to the problem? You can simply clone project to your Github account, push your fix to your account, and then create a pull request. Hopefully, the author would be happy to accept if it is a good solution.

@christianmeier
Copy link
Author

@christianmeier: To me, this observation would mean, unfortunately, that the package is not yet stable.

You said you tried to make a branch. Do you have a working solution to the problem? You can simply clone project to your Github account, push your fix to your account, and then create a pull request. Hopefully, the author would be happy to accept if it is a good solution.

I stick to it, that it runs stable. Stable means for me that it does what it is expected. The issues with the permissions on the zone need to be verified from others. May I'm the only one with that symptom : )

@brainchild0
Copy link

Do you have an improved version available for testing?

@christianmeier
Copy link
Author

imported it into my gitlab and invited you

@brainchild0
Copy link

I see the project, but no code. It may be helpful to put it in Github, since the project started here, and as far I know pull requests are only supported within the platform. More important, it would seem helpful to share with everyone, not just with me.

@christianmeier
Copy link
Author

Got your point, I'll stay in Gitlab. Most of my code is work-related and I'm not allowed to share it . Why you do not import the projekt into your github?

@dougmeek
Copy link

dougmeek commented Dec 5, 2021 •
edited
Loading

@brainchild0 you are not the only one having issues. I have the same permissions issue and have resorted to just updating the master zones with SSH if I need to modify a record. The GUI is broken when running this. There's another open issue #38 that documents this.

@christianmeier
Copy link
Author

hi, try this

diskstation_dns_modify.sh

Change:
user:group nobody to DNSServer
! chown DNSServer:DNSServer $BackupPath/$ForwardMasterFile.bumped $BackupPath/$ReverseMasterFile.bumped ; then

@dougmeek
Copy link

dougmeek commented Dec 5, 2021

@christianmeier I will when I get a few. Currently in great need of coffee lol. I see that #38 has a potential fix, it just needs to be put into a PR and tested.

@dougmeek
Copy link

dougmeek commented Dec 5, 2021

@christianmeier I'd say at this point with the original intent of this issue, it's probably safe to close this issue. It's been close to a year and the only major problem that's cropped up is this permissions issue that seems relatively new and is documented in a separate issue.

@christianmeier
Copy link
Author

@christianmeier I'd say at this point with the original intent of this issue, it's probably safe to close this issue. It's been close to a year and the only major problem that's cropped up is this permissions issue that seems relatively new and is documented in a separate issue.

Agree

@dougmeek
Copy link

dougmeek commented Dec 5, 2021

Can confirm that #38 does resolve the permissions issue for zone files in DSM 7. One of us can easily do a PR to fix that, but we'll need someone with DSM 6 to test with. @gclayburg @christianmeier @brainchild0 do any of you have a DSM 6 box to test with?

@christianmeier
Copy link
Author

Can confirm that #38 does resolve the permissions issue for zone files in DSM 7. One of us can easily do a PR to fix that, but we'll need someone with DSM 6 to test with. @gclayburg @christianmeier @brainchild0 do any of you have a DSM 6 box to test with?

nope, I'm bleeding edge

@brainchild0
Copy link

brainchild0 commented Dec 5, 2021 •
edited
Loading

I don't have any equipment running DSM 6 any longer. I believe Synology offers a tool chain for running a second version virtually, for testing purposes, but it is probably a hassle to configure.

How bad would it be simply to drop support for earlier versions of DSM?

@brainchild0
Copy link

Against my suggestion that the issue remain open until all major stability issues are resolved, including the one recently mentioned, I suggest at least that anyone who would close this thread do so only while leaving a comment explaining the remaining issues and providing references to the topics tracking them.

@dougmeek
Copy link

dougmeek commented Dec 6, 2021

@brainchild0 the original issue in this issue has been resolved, being the leases file. The only other issue that I'm aware of is documented in #38.

There are likely quite a few people still running DSM 6. My guess is that it's quite simple to modify the script to support both DSM 6 and DSM 7, but we'd need a DSM 6 filesystem to look at permissions.

If you want to stand up a virtual DSM 6 I will provide you with a fixed script that you can test with. I'm running a modified version that fixes the permissions issue documented in #38. It works flawlessly. Thus, if the same code works for DSM 6, we can resolve the outstanding issues #34 and #38. At present IMO, #34 is resolved. The submitter for this issue (@christianmeier) also agrees that this should be closed, as the leases issue was resolved many months ago. Likely this issue should've been closed then.

For 100% clarity at the request of your last post, the only outstanding issue is #38. Apply that code and this is stable.

@brainchild0
Copy link

@dougmeek: Perhaps the title of this topic has misled me in regard to what others consider its scope.

@dougmeek
Copy link

dougmeek commented Dec 6, 2021

@brainchild0 I totally agree. This issue technically should've been closed after the last two commits to master. Which is why @christianmeier said in his opinion the release was stable.

@brainchild0
Copy link

Well, I would be less apprehensive about withdrawing my request for keeping the topic open (which is irrelevant, anyway, having been overruled) if the title were changed to represent the comparatively narrow scope of the actual discussion.

@dougmeek
Copy link

dougmeek commented Dec 6, 2021

I mean, if you run into issues, you can just open an issue for that? I'm not sure I understand your apprehension or frustration with closing out this stale issue. It's a functional script that works for DSM 7 just fine. If you're trying to run this in production, that's on you anyway. DNS/DHCP/IPAM should be run on something more robust in a prod environment, like Infoblox. Anyway, good luck.

@brainchild0
Copy link

I simply was suggesting changing the name of the topic.

@christianmeier
Copy link
Author

I simply was suggesting changing the name of the topic.

I can also recommend Infoblox, beside this I recommend to focus on features and not only on issues. I always appreciate people that are contributing to community projects instead of raising just issues. Don't get me wrong. But I was reading thru this thread and the only thing that was coming from your side was orders how we shall proceed. It's a peaceful feedback from my side.

@brainchild0
Copy link

@christianmeier: I'm sorry you didn't find my suggestions constructive. You may not be familiar with common patterns in Github. I felt it might be easier for everyone if you would try to follow them. I was trying to be helpful, not controlling or negative.

@brainchild0
Copy link

brainchild0 commented Dec 7, 2021 •
edited
Loading

The current discussion has become confused.

Following is my attempt to summarize the status:

  • The earlier version, stable on DSM 6, is not stable on DSM 7.
  • At least one reason, the only identified so far, relates to file permissions, and is documented in the current issue as well as in DSM 7 - Zone File Permissions #38.
  • At least one user has attempted a solution, but so far the proposed improvements are available only to that user locally, and not published as a pull request, or even a project fork. However, a contribution may be forthcoming.

One question I have not resolved is the relation between the current issue and #38, for instance, whether they are duplicates, and whether they may be resolved most easily under a single merged pull request.

In any case, I am eager to have access to an improved version that attempts to resolve the known issues for DSM 7. Surely it would be helpful to any using the project, since most will not wish to remain at an earlier version of DSM.

@dougmeek
Copy link

dougmeek commented Dec 7, 2021 •
edited
Loading

@christianmeier: I'm sorry you didn't find my suggestions constructive. You may not be familiar with common patterns in Github. I felt it might be easier for everyone if you would try to follow them. I was trying to be helpful, not controlling or negative.

@brainchild0 honestly, that reads really condescending and I don't appreciate the way you're talking to @christianmeier. No one here is paid to support this repo and you demanding that someone do or not do something really comes off the wrong way. If you don't like how things are being managed here, you're welcome to fork and do it yourself.

I don't know what you find so difficult about understanding this issue. I've stated it in multiple comments now and at this point I'm convinced you're just trolling. If you read this issue you'll see that the issue was opened to report an issue with the leases file. The description is irrelevant. Read the first post in the issue.

It seems to me that you are the one that doesn't understand basic GitHub patterns:

  • This is an open source repo. As such, you're welcome to review the code for yourself.
  • No one here is your paid support team to do your bidding as you wish. Keep things cordial and maybe we'll help.
  • If you don't like how something is done, you're welcome to fork it.
  • If you know there is an issue that needs resolving, you're welcome to fix it and submit a PR. Just like the rest of us.

The current discussion has become confused.

There's nothing confusing about this to anyone else. This issue is pertaining to a leases file, which has been fixed. The only thing that's confusing is why it wasn't closed after the commits that resolved it.

One question I have not resolved is the relation between the current issue and #38, for instance, whether they are duplicates, and whether they may be resolved most easily under a single merged pull request.

As I've stated, there is no relation between the issues. They are not duplicates. This issue, #34, was resolved in commits 35d5680 and d399181. Issue #38 has been addressed in PR #39 already, but has not been reviewed and merged to master as only @gclayburg has access to do that. Feel free to test #39 and provide feedback on that.

Hopefully that helps.

@christianmeier would you please close this issue? It doesn't look like I have the ability.

@brainchild0
Copy link

brainchild0 commented Dec 7, 2021 •
edited
Loading

@brainchild0 honestly, that reads really condescending and I don't appreciate the way you're talking to @christianmeier. No one here is paid to support this repo and you demanding that someone do or not do something really comes off the wrong way. If you don't like how things are being managed here, you're welcome to fork and do it yourself.

The title is the text displayed in the listing, to anyone browsing or searching issues in the tracker. It is difficult, and sometimes confusing, if the title of an issue does not match its scope. The pull request is the standard means for users to share contributions, and seemed sensible to suggest because someone was blocked by a permissions issue related to pushing directly to the project branch.

If you experience my suggestions as demands, you are free to disregard them. Others might have experienced them as constructive. In all my experience on Github, giving suggestions of the kinds I have given in this discussion, by me or anyone else, has never been controversial, much less caused anyone offense. To apologize once more, I am sorry it seems to have done so here.

@gclayburg
Copy link
Owner

Hi guys, sorry but I was away from github for a few days. There is some cleanup here to do for sure. I'll work on some of that tonight. I can also test out the #38 fix on DSM 6. I suspect there are many like me using older hardware that are stuck with DSM 6.

@ChrizK-oldGit
Copy link

ChrizK-oldGit commented Apr 3, 2023 •
edited
Loading

As said, I’m on DSM 7 and can test it. No issue. Regarding the logging I suggest to place them to /var/logs and also set up log-rotate. let me knew witch snapshot I can test.

hi @christianmeier , sorry, appreciate this discussion is a little old.
I am investigating log rotate in regard to the Slinger project which can run as a service under DSM (systemctl, systemd).
I would ideally like the console (stdout, stderr) to output as a log managed by Log Center, but I am not progressing with this thought yet. The alternative (and perhaps more simple idea) is to output to a file, which I would liked to be 'managed', hence looking at logrotate. Can you tell me if logrotate can only manage files in /var/logs? Is DSM 7 the 'generic/standards' implementation, ie can I just read about logrotate in general to learn how to configure it, or is it restricted by Synology? (I have ended up here as I am searching for 'DSM log-rotate', and there appears to be very little web discussion on the subject).

@christianmeier
Copy link
Author

@gclayburg, would it be better to create a issue for the request? BR Chris

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@gclayburg @christianmeier @dougmeek @brainchild0 @sw2828 @ChrizK-oldGit