Skip to content

Conversation

@nicholas-fedor
Copy link
Owner

This PR fixes the --notification-split-by-container feature that was implemented in response to issue #720 but found to be non-functional during testing and reported in issue #771. The feature was not working properly in general, beyond the specific edge case mentioned in #771.

Some time was also allocated towards refactoring the root.go file, which is sorely needed to facilitate future maintainability of the project.

Key Changes

fix: notification split by container when no update is performed

  • Modify runUpdatesWithNotifications to handle notification splitting in both report and log modes
  • Add GetEntries() and SendFilteredEntries() methods to notifier interface
  • Update all notifier implementations with new interface methods
  • Remove entry reset in Shoutrrr notifier to preserve log batch across multiple sends

refactor: modularize root command and extract utility functions

  • Move runUpdatesWithNotifications function to actions.RunUpdatesWithNotifications
  • Extract HTTP API setup logic to internal/api package
  • Create internal/config package for configuration management
  • Move startup logging to internal/logging package
  • Extract scheduling logic to internal/scheduling package
  • Add utility functions for duration formatting to internal/util package
  • Update all import statements and package references
  • Add comprehensive documentation and inline comments

Technical Details

Notification Splitting Implementation

// Report mode: Individual SingleContainerReport instances per updated container
// Log mode: Filtered logrus entries by container name
if notificationSplitByContainer && len(result.Updated()) > 0 {
    for _, updatedContainer := range result.Updated() {
        singleReport := &SingleContainerReport{
            UpdatedReports: []types.ContainerReport{updatedContainer},
            // ... include session context
        }
        notifier.SendNotification(singleReport)
    }
}

Package Structure Changes

internal/
├── actions/     # Update execution logic
├── api/         # HTTP API orchestration  
├── config/      # Configuration management
├── logging/     # Startup logging
├── scheduling/  # Cron scheduling
└── util/        # Utility functions

…e is performed

- Modify runUpdatesWithNotifications to handle notification splitting in both report and log modes
- Add GetEntries and SendFilteredEntries methods to Notifier interface and implementations
- Implement sortableContainers for deduplicating and sorting container reports
- Update All method in singleContainerReport to ensure unique containers prioritized by state
- Refactor global variables into a var block and introduce RunConfig struct for better organization

Fixes #771
- Move `runUpdatesWithNotifications` function to `actions.RunUpdatesWithNotifications`
- Extract API setup logic to `internal/api.SetupAndStartAPI`
- Move scheduling logic to `internal/scheduling.RunUpgradesOnSchedule`
- Extract logging utilities to `internal/logging.WriteStartupMessage`
- Move configuration struct to `internal/config.RunConfig`
- Relocate utility functions (`FormatDuration`, `FormatTimeUnit`, `FilterEmpty`) to `internal/util`
- Move report types (`SingleContainerReport`, `SortableContainers`) to `pkg/session/report.go`
- Update notifier interface with new methods for filtered entries
- Adjust tests to use new module functions and types
- Remove unused imports and clean up code structure in `cmd/root.go
@codecov
Copy link

codecov bot commented Oct 7, 2025

Codecov Report

❌ Patch coverage is 75.51020% with 120 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/actions/actions.go 71.02% 30 Missing and 1 partial ⚠️
internal/scheduling/scheduling.go 80.51% 13 Missing and 2 partials ⚠️
pkg/session/report.go 63.41% 15 Missing ⚠️
internal/api/api.go 72.54% 12 Missing and 2 partials ⚠️
internal/logging/startup.go 88.46% 8 Missing and 4 partials ⚠️
pkg/notifications/shoutrrr.go 0.00% 10 Missing ⚠️
pkg/api/api.go 69.56% 4 Missing and 3 partials ⚠️
pkg/notifications/email.go 0.00% 4 Missing ⚠️
pkg/notifications/gotify.go 0.00% 4 Missing ⚠️
pkg/notifications/msteams.go 0.00% 4 Missing ⚠️
... and 1 more

❌ Your changes status has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #775      +/-   ##
==========================================
+ Coverage   73.57%   73.81%   +0.23%     
==========================================
  Files          45       49       +4     
  Lines        6482     6923     +441     
==========================================
+ Hits         4769     5110     +341     
- Misses       1556     1643      +87     
- Partials      157      170      +13     
Files with missing lines Coverage Δ
internal/util/util.go 100.00% <100.00%> (ø)
pkg/api/update/update.go 93.68% <100.00%> (+0.91%) ⬆️
pkg/notifications/email.go 81.13% <0.00%> (-3.19%) ⬇️
pkg/notifications/gotify.go 71.26% <0.00%> (-3.44%) ⬇️
pkg/notifications/msteams.go 60.00% <0.00%> (-5.22%) ⬇️
pkg/notifications/slack.go 89.15% <0.00%> (-4.52%) ⬇️
pkg/api/api.go 93.96% <69.56%> (-6.04%) ⬇️
pkg/notifications/shoutrrr.go 76.88% <0.00%> (-6.17%) ⬇️
internal/logging/startup.go 88.46% <88.46%> (ø)
internal/api/api.go 72.54% <72.54%> (ø)
... and 3 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ting

- modify SetupAndStartAPI in internal/api/api.go to accept optional server parameter
- update New and Start methods in pkg/api/api.go to support server injection
- add nil check for notifier in WriteStartupMessage in internal/logging/startup.go
- add tests for FormatDuration, FormatTimeUnit, and FilterEmpty in internal/util/util_test.go
- add test files for actions, api, logging, and scheduling modules
- add debug logging to TestConcurrentScheduledAndAPIUpdate for lock acquisition visibility
- reorder goroutine execution to ensure scheduled update acquires lock first
- add debug logs to update handler for improved tracing
@nicholas-fedor nicholas-fedor merged commit aefb498 into main Oct 7, 2025
14 of 16 checks passed
@nicholas-fedor nicholas-fedor deleted the fix/771-watchtower_notification_split_by_container-does-not-separate-if-no-update-is-performed branch October 7, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WATCHTOWER_NOTIFICATION_SPLIT_BY_CONTAINER does not separate if no update is performed

1 participant