draft: Fix heartbeat before oomkill - #147
Draft
jessegoodier wants to merge 12 commits into
Draft
Conversation
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
Signed-off-by: Ishaan Mittal <ishaanmittal123@gmail.com>
This commit fixes several critical issues identified in PR #143: 1. Race Condition in Stop() Method - Fixed potential race condition where writer could be nil between check and Sync() call - Moved stopCh close outside mutex to prevent deadlock - Copy writer reference under lock, then use outside lock 2. Resource Leak Prevention - Improved error handling in rotateFile() to always attempt file close even if sync fails - Prevents file descriptor leaks under error conditions - Logs both sync and close errors for better debugging 3. Configuration Validation - Added comprehensive Validate() method with reasonable bounds: * BufferSize: 1MB - 100MB * SyncInterval: >= 1 second (if set) * UploadInterval: >= 1 minute * LogDirPath: must be absolute and non-empty * ClusterName: must be non-empty * PathPrefix: validated for invalid characters - Validation called automatically in NewLogExporter() 4. Code Quality Improvements - Added constants for file permissions (dirPermissions, filePermissions) - Added logFilePattern constant for consistent file matching - Fixed sync timing issue: initialize lastSyncTime to zero for immediate first sync instead of delaying by full interval These changes improve reliability, prevent resource leaks, and ensure invalid configurations are caught early. Signed-off-by: Bob <bob@example.com>
Added extensive test coverage for all logExporter components: 1. Config Validation Tests (config_test.go) - Tests all validation rules for buffer size, intervals, paths - Tests edge cases and boundary conditions - 12 test cases covering valid and invalid configurations 2. FileWriter Tests (writer_test.go) - Tests file creation, rotation, and sync behavior - Tests concurrent writes for thread safety - Tests error handling and resource cleanup - Tests pending file tracking - 8 test suites with multiple sub-tests 3. LogExporter Tests (logexporter_test.go) - Tests initialization with valid/invalid configs - Tests Stop() method including concurrent stops (race condition test) - Tests file upload, compression, and cleanup - Tests gzip compression/decompression - Mock storage implementation for isolated testing - 6 test suites covering core functionality Test Results: - All tests pass with race detection enabled (-race flag) - No race conditions detected - Validates all critical fixes from previous commit - Tests cover: validation, rotation, sync, upload, compression, error handling, concurrent access, and resource cleanup Signed-off-by: Bob <bob@example.com>
Send an initial heartbeat immediately after agent initialization completes, before starting the periodic heartbeat timer. This provides immediate visibility that the agent has started successfully and confirms bucket connectivity. Implementation: - Start heartbeat controller with 1ms interval to trigger immediate export - Wait 100ms for the first heartbeat to be sent - Stop and restart with normal periodic interval Benefits: - Immediate confirmation agent is running and healthy - Early detection of bucket connectivity issues - Version and cluster info available immediately - Non-blocking startup (uses short delay) The heartbeat is sent after all initialization completes: - Bucket storage is initialized and validated - Cluster info and metadata providers are ready - All pipeline controllers are started This addresses the delay where the first heartbeat would only be sent after the full heartbeat interval (typically 5-15 minutes) had elapsed. Signed-off-by: Bob <bob@example.com>
Added test coverage for the immediate heartbeat feature: 1. TestKubecostEmitter_ImmediateHeartbeat - Integration test placeholder (skipped in unit tests) - Documents what should be tested in full integration environment - Requires actual bucket storage and cluster setup 2. TestKubecostEmitter_HeartbeatTiming - Documents expected timing behavior - Validates timing constants are reasonable - Serves as living documentation for the feature 3. TestKubecostEmitter_ID - Verifies emitter returns correct ID - Basic functionality test 4. TestKubecostEmitter_NewKubecostEmitter - Tests constructor behavior - Validates config and diagnostics are set correctly Test Results: - All tests pass - Integration test properly skipped (requires full setup) - Timing test validates constants are reasonable - Constructor tests verify basic functionality The immediate heartbeat feature is best validated through: - Manual testing: Deploy and check bucket within 5 seconds - E2E tests: Automated deployment with bucket verification - Log verification: Check for 'Sending initial heartbeat...' message Signed-off-by: Bob <bob@example.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
waiting for #143