feat: add fallback NTP server support #782
Merged
Conversation
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
Reviewer's GuideAdds fallback NTP server support by introducing a new configuration option, extending the Manager to store and retrieve a fallback server, and modifying the timesyncd setup logic to append the fallback NTP when using the default or when the primary fails. Sequence Diagram: Setting NTP Server with Fallback LogicsequenceDiagram
participant Caller
participant M as Manager
participant DS_Manager as "ConfigManager.Manager"
participant TSCFG as "timesyncd.conf"
participant SD as systemd
Caller->>M: setNTPServer(primaryNTP)
M->>M: setNTPServerToTimeSyncd(primaryNTP)
M->>DS_Manager: IsDefaultValue(0, dsettingsKeyNTPServer)
DS_Manager-->>M: isDefault
alt If isDefault is true
M->>M: currentNTP = primaryNTP + " " + m.fallbackNTPServer
M->>M: log("Set fallback NTP server: " + m.fallbackNTPServer)
else Else (primaryNTP is not default)
M->>M: currentNTP = primaryNTP
end
M->>TSCFG: Write NTP=currentNTP to timeSyncCfgFile
M->>SD: Restart("systemd-timesyncd.service")
SD-->>M: RestartResult
Class Diagram: Manager Update for Fallback NTP SupportclassDiagram
class Manager {
%% Existing fields (abbreviated for brevity)
+core timedate1.Timedate
+service *dbusutil.Service
+PropsMu sync.RWMutex
+NTPServer string
+timesyncd timesync1.Timesync1
+systemd systemd1.Manager
+setNTPServerMu sync.RWMutex
+signalLoop *dbusutil.SignalLoop
+dsManager ConfigManager.Manager
%% New field
+fallbackNTPServer string
%% Existing methods (potentially modified)
+start()
+getDsgNTPServer() string
+setDsgObsoleteNTPServer(server string) error
+setNTPServer(value string) error // Now calls setNTPServerToTimeSyncd
%% New methods or significantly changed internal logic
+getDsgFallbackNTPServer() string
+setNTPServerToTimeSyncd(server string) error // Previously global setNTPServer, now a method with fallback logic
}
note for Manager "New field: fallbackNTPServer string\nNew method: getDsgFallbackNTPServer()\nNew method: setNTPServerToTimeSyncd() (incorporates fallback logic, was previously global setNTPServer)\nModified: start() initializes fallbackNTPServer\nModified: setNTPServer() now calls internal setNTPServerToTimeSyncd()"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @mhduiy - I've reviewed your changes - here's some feedback:
- Translate the inline Chinese comment in setNTPServerToTimeSyncd into English and clarify the rationale behind using this fallback approach.
- Rename the variable
setFallbackto something more descriptive likeuseFallbackServerorisDefaultNTPServerto improve readability. - Add a guard to only append
fallbackNTPServerwhen it’s non-empty to avoid introducing trailing spaces or invalid NTP entries.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
fly602
previously approved these changes
May 13, 2025
1. Added FallbackNTPServer configuration in dsg-configs with aliyun as default 2. Extended Manager struct to include fallbackNTPServer field 3. Implemented getDsgFallbackNTPServer method to retrieve fallback server 4. Modified setNTPServerToTimeSyncd to append fallback server when default NTP is used 5. Added logic to handle fallback server when primary NTP server fails The changes provide redundancy for time synchronization by automatically falling back to a secondary NTP server (ntp.aliyun.com) when the primary server is unavailable or using default settings. This improves system time reliability. feat: 添加后备NTP服务器支持 1. 在dsg-configs中添加FallbackNTPServer配置，默认使用阿里云服务器 2. 扩展Manager结构体包含fallbackNTPServer字段 3. 实现getDsgFallbackNTPServer方法获取后备服务器 4. 修改setNTPServerToTimeSyncd方法，在使用默认NTP时追加后备服务器 5. 添加处理逻辑在主NTP服务器不可用时使用后备服务器 这些修改通过在主NTP服务器不可用或使用默认设置时自动回退到备用NTP服务器 (ntp.aliyun.com)，为时间同步提供了冗余，提高了系统时间的可靠性。 pms: BUG-315181
deepin pr auto review代码审查意见：
综上所述，代码在功能实现上基本正确，但在错误处理、日志记录、代码重复、配置管理、安全性、性能和代码风格方面存在一些改进空间。 |
fly602
approved these changes
May 13, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
|
TAG Bot New tag: 6.1.31 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The changes provide redundancy for time synchronization by automatically falling back to a secondary NTP server (ntp.aliyun.com) when the primary server is unavailable or using default settings. This improves system time reliability.
feat: 添加后备NTP服务器支持
这些修改通过在主NTP服务器不可用或使用默认设置时自动回退到备用NTP服务器
(ntp.aliyun.com)，为时间同步提供了冗余，提高了系统时间的可靠性。
pms: BUG-315181
Summary by Sourcery
Add support for a secondary NTP server by introducing a fallback configuration and automatically appending and switching to it when the primary NTP server is unavailable or using default settings
New Features:
Enhancements: