-
Notifications
You must be signed in to change notification settings - Fork 10
adding secret watcher to restart ptp4l process #111
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
Draft
gtannous-spec
wants to merge
1
commit into
k8snetworkplumbingwg:main
Choose a base branch
from
gtannous-spec:auth-tlv
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+151
−1
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ import ( | |
| "syscall" | ||
| "time" | ||
|
|
||
| "github.com/fsnotify/fsnotify" | ||
| "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/parser" | ||
| "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/synce" | ||
| "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/utils" | ||
|
|
@@ -54,6 +55,7 @@ const ( | |
| MessageTagSuffixSeperator = ":" | ||
| TBC = "T-BC" | ||
| TGM = "T-GM" | ||
| PtpSecFolder = "/etc/ptp-secret-mount/" | ||
| ) | ||
|
|
||
| var ( | ||
|
|
@@ -306,6 +308,7 @@ type Daemon struct { | |
|
|
||
| // Allow vendors to include plugins | ||
| pluginManager plugin.PluginManager | ||
| saFileWatcher *fsnotify.Watcher | ||
| } | ||
|
|
||
| // New LinuxPTP is called by daemon to generate new linuxptp instance | ||
|
|
@@ -335,6 +338,23 @@ func New( | |
| ptpEventHandler: event.Init(nodeName, stdoutToSocket, eventSocket, eventChannel, closeManager, Offset, ClockState, ClockClassMetrics), | ||
| } | ||
| tracker.processManager = pm | ||
|
|
||
| // Initialize fsnotify watcher for sa_file change detection | ||
| watcher, err := fsnotify.NewWatcher() | ||
| if err != nil { | ||
| glog.Errorf("Failed to create fsnotify watcher for sa_file monitoring: %v", err) | ||
| glog.Warning("sa_file change detection will be disabled") | ||
| watcher = nil | ||
| } else { | ||
| glog.Info("fsnotify watcher initialized for sa_file change detection") | ||
| // Watch the known security mount folder from startup | ||
| if watchErr := watcher.Add(PtpSecFolder); watchErr != nil { | ||
| glog.Warningf("Failed to watch %s (may not exist yet): %v", PtpSecFolder, watchErr) | ||
| } else { | ||
| glog.Infof("Watching %s for sa_file changes", PtpSecFolder) | ||
| } | ||
| } | ||
|
|
||
| return &Daemon{ | ||
| nodeName: nodeName, | ||
| namespace: namespace, | ||
|
|
@@ -348,19 +368,76 @@ func New( | |
| processManager: pm, | ||
| readyTracker: tracker, | ||
| stopCh: stopCh, | ||
| saFileWatcher: watcher, | ||
| } | ||
| } | ||
|
|
||
| // Run in a for loop to listen for any LinuxPTPConfUpdate changes | ||
| // This function handles two types of configuration changes: | ||
| // 1. PtpConfig changes (via ConfigMap) - triggers UpdateCh | ||
| // 2. Authentication file changes (via Secret) - triggers fsnotify events (instant detection) | ||
| // Both trigger applyNodePTPProfiles() which restarts PTP processes WITHOUT restarting the pod | ||
| func (dn *Daemon) Run() { | ||
| go dn.processManager.ptpEventHandler.ProcessEvents() | ||
|
|
||
| // Setup fsnotify channels (may be nil if watcher initialization failed) | ||
| var watcherEvents chan fsnotify.Event | ||
| var watcherErrors chan error | ||
| if dn.saFileWatcher != nil { | ||
| watcherEvents = dn.saFileWatcher.Events | ||
| watcherErrors = dn.saFileWatcher.Errors | ||
| defer dn.saFileWatcher.Close() | ||
| glog.Info("Using fsnotify for instant sa_file change detection") | ||
| } else { | ||
| glog.Warning("fsnotify unavailable, sa_file change detection disabled") | ||
| } | ||
|
|
||
| for { | ||
| select { | ||
| case <-dn.ptpUpdate.UpdateCh: | ||
| // PtpConfig change detected via ConfigMap | ||
|
|
||
| glog.Info("PtpConfig change detected, restarting PTP processes") | ||
| err := dn.applyNodePTPProfiles() | ||
| if err != nil { | ||
| glog.Errorf("linuxPTP apply node profile failed: %v", err) | ||
| } | ||
|
|
||
| case event, ok := <-watcherEvents: | ||
| // File system event on sa_file directory | ||
| if !ok { | ||
| glog.Error("fsnotify watcher channel closed, disabling sa_file monitoring") | ||
| watcherEvents = nil | ||
| continue | ||
| } | ||
|
|
||
| if event.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Remove) == 0 { | ||
| continue | ||
| } | ||
| if strings.HasPrefix(filepath.Base(event.Name), ".") { | ||
| continue // Ignore hidden files like ..data | ||
| } | ||
|
|
||
| glog.Infof("Security file changed: %s (op: %s), restarting PTP processes", event.Name, event.Op.String()) | ||
| err := dn.applyNodePTPProfiles() | ||
| if err != nil { | ||
| glog.Errorf("linuxPTP apply node profile failed after security file change: %v", err) | ||
| } | ||
|
|
||
| case err, ok := <-watcherErrors: | ||
| // fsnotify watcher error | ||
| if !ok { | ||
| watcherErrors = nil | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should you recreate the watcher here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should keep recreate until watcher is not having error |
||
| // recreate the watcher | ||
| dn.saFileWatcher, err = fsnotify.NewWatcher() | ||
| if err != nil { | ||
| glog.Errorf("Failed to recreate fsnotify watcher for sa_file monitoring: %v", err) | ||
| continue | ||
| } | ||
| glog.Info("fsnotify watcher reinitialized for sa_file change detection") | ||
| } | ||
| glog.Errorf("fsnotify watcher error: %v", err) | ||
|
|
||
| case <-dn.stopCh: | ||
| dn.stopAllProcesses() | ||
| glog.Infof("linuxPTP stop signal received, existing..") | ||
|
|
@@ -546,6 +623,66 @@ func printNodeProfile(nodeProfile *ptpv1.PtpProfile) { | |
| glog.Infof("------------------------------------") | ||
| } | ||
|
|
||
| // extractAuthSettingsForPhc2sys extracts sa_file, spp, and active_key_id from ptp4lConf | ||
| // and returns a phc2sys-compatible [global] section with those settings | ||
| func extractAuthSettingsForPhc2sys(ptp4lConf *string) string { | ||
| if ptp4lConf == nil || *ptp4lConf == "" { | ||
| return "" | ||
| } | ||
|
|
||
| var saFile, spp, activeKeyID string | ||
| inGlobal := false | ||
|
|
||
| for _, line := range strings.Split(*ptp4lConf, "\n") { | ||
| line = strings.TrimSpace(line) | ||
|
|
||
| if line == "[global]" { | ||
| inGlobal = true | ||
| continue | ||
| } | ||
| if strings.HasPrefix(line, "[") && line != "[global]" { | ||
| inGlobal = false | ||
| continue | ||
| } | ||
|
|
||
| if inGlobal { | ||
| if strings.HasPrefix(line, "sa_file") { | ||
| parts := strings.Fields(line) | ||
| if len(parts) >= 2 { | ||
| saFile = parts[1] | ||
| } | ||
| } else if strings.HasPrefix(line, "spp ") { | ||
| parts := strings.Fields(line) | ||
| if len(parts) >= 2 { | ||
| spp = parts[1] | ||
| } | ||
| } else if strings.HasPrefix(line, "active_key_id") { | ||
| parts := strings.Fields(line) | ||
| if len(parts) >= 2 { | ||
| activeKeyID = parts[1] | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Only generate config if we have auth settings | ||
| if saFile == "" { | ||
| return "" | ||
| } | ||
|
|
||
| var result strings.Builder | ||
| result.WriteString("[global]\n") | ||
| result.WriteString(fmt.Sprintf("sa_file %s\n", saFile)) | ||
| if spp != "" { | ||
| result.WriteString(fmt.Sprintf("spp %s\n", spp)) | ||
| } | ||
| if activeKeyID != "" { | ||
| result.WriteString(fmt.Sprintf("active_key_id %s\n", activeKeyID)) | ||
| } | ||
|
|
||
| return result.String() | ||
| } | ||
|
|
||
| /* | ||
| update: March 7th 2024 | ||
| To support PTP HA phc2sys profile is appended to the end | ||
|
|
@@ -633,6 +770,19 @@ func (dn *Daemon) applyNodePtpProfile(runID int, nodeProfile *ptpv1.PtpProfile) | |
| } | ||
| configFile = fmt.Sprintf("phc2sys.%d.config", runID) | ||
| configPath = fmt.Sprintf("%s/%s", configPrefix, configFile) | ||
| if clockType == event.GM && nodeProfile.Ptp4lConf != nil { | ||
| phc2sysAuthConfig := extractAuthSettingsForPhc2sys(nodeProfile.Ptp4lConf) | ||
| if phc2sysAuthConfig != "" { | ||
| if configInput == nil || *configInput == "" { | ||
| configInput = &phc2sysAuthConfig | ||
| } else { | ||
| // Prepend auth settings to existing config | ||
| merged := phc2sysAuthConfig + "\n" + *configInput | ||
| configInput = &merged | ||
| } | ||
| glog.Infof("Injected auth settings into phc2sys config for grandmaster profile %s", *nodeProfile.Name) | ||
| } | ||
| } | ||
| case ts2phcProcessName: | ||
| configInput = nodeProfile.Ts2PhcConf | ||
| configOpts = nodeProfile.Ts2PhcOpts | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not run as goroutines so they are blocking meaning there is no need for this flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, there is no concurrent access.. I will fix that