search

gclayburg / synology-diskstation-scripts

Update Synology DNS records from DHCP IP address reservation
169 stars 41 forks source link

DSM7 #34

Open christianmeier opened 3 years ago

christianmeier commented 3 years ago

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

dougmeek commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

dougmeek commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

I simply was suggesting changing the name of the topic.

christianmeier commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

The current discussion has become confused.

Following is my attempt to summarize the status:

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 commented 2 years ago

@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:

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 commented 2 years ago

@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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

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