performance-and0coverage#8
Merged
Merged
Conversation
- Add coverage profile generation with per-package breakdown - Generate HTML coverage reports for artifacts - Add TestComprehensive_ContainerLifecycle for state machine testing - Add TestComprehensive_ErrorTransitions for edge cases - Add TestComprehensive_Concurrency for thread-safety - Add TestComprehensive_Persistence for state recovery - Add BenchmarkContainerCreate, StateTransitions, ContainerListing - Add BenchmarkLogWrite, LogRead, ConcurrentReads - Add TestBench_StateManagerPerformance with throughput/latency assertions - Add TestBench_LogManagerPerformance with throughput/latency assertions - CI: Generate and upload coverage reports as artifacts - CI: Run integration tests with coverage profiles - Performance targets: creation 50+/sec, listing <10ms, logs >1000/sec
- Add check to prevent re-initialization of existing containers - InitContainer now returns error if container already exists - Prevents accidental state overwrites - Validates state transitions: can only create new containers This ensures the test TestComprehensive_ErrorTransitions passes
- Move container existence check BEFORE log in InitContainer - Add error checks on all InitContainer/MarkRunning calls in test - Test now properly validates: cannot exit non-running container - Test now properly validates: cannot re-init existing container - Improves clarity of error flows and constraints
- Fix lines 139, 142, 143: Use strconv.Itoa() for int-to-string conversion - Was using string(int) which creates rune, not decimal string - Add strconv import - Simplify Close() error handling to avoid linter issues - Now compiles without errors
- Add transition validation to SetExit method - Add transition validation to SetError method - Only allow exiting from running state - Only allow error transition from created or running states - Prevents invalid state changes that violate state machine This fixes TestComprehensive_ErrorTransitions
- Fix TestStateManager_UpdateStatus_InvalidTransition - Error message is 'invalid transition:' not 'invalid state transition' - Update string contains check to match actual error
- Remove -covermode=atomic flag causing 'no such tool covdata' errors - Use default coverage mode which is compatible with all Go versions - Simplifies CI configuration without sacrificing coverage reporting - Tests already passing, just fixing exit code issue
- cgroups: Simplified error handling, removed wrapping, added write() closure - container: Better ID generation, graceful network failures, state tracking - fs: Structured Mount type, reverse unmounting on failure, security flags - network: Thread-safe IPAM with atomic file ops, idempotent bridge creation - security: 27 capabilities dropped, 153 syscall whitelist, path masking All changes maintain backward compatibility and improve production readiness.
- Subnet was declared in both bridge.go and ipam.go - Keep single definition in ipam.go (primary owner) - bridge.go now references Subnet from ipam.go package
Tests were expecting old verbose error messages. Updated to match the simplified error messages from cgroups refactor: - 'ContainerID must not be empty' → 'ContainerID required' - 'invalid Memory' → 'invalid memory string' - 'below minimum' → 'memory limit too low' - 'invalid CPU value' → 'strconv.ParseFloat' - 'PIDs must be non-negative' → 'PIDs cannot be negative' - 'swap limit' → 'swap must be'
- Run all tests with sudo (not just subset) - Allow tests to fail gracefully if kernel features unavailable - Collect integration test coverage separately - Run all benchmarks, not just specific ones - Log results for analysis This will attempt to test root-required operations in CI. Tests that need specific kernel features will be skipped naturally if the CI environment doesn't support them.
- Enable passwordless sudo for integration test runner - Verify namespace support (user, network, mount) - Check unprivileged namespace settings - Log capability checks for debugging - Allows root-required operations in CI environment
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
1 participant
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.
This pull request introduces significant improvements to both the CI workflow and the
cgroupsandcontainerpackages, focusing on enhanced test coverage, error handling, and code simplification. The CI pipeline now collects and reports detailed code coverage and ensures proper permissions for integration testing. Thecgroupspackage has been refactored for clearer error messages, simpler logic, and improved reliability. Thecontainerpackage has been streamlined for more concise error handling and environment variable usage.CI/CD Workflow Enhancements:
.github/workflows/ci.yml)sudoconfiguration and root capability checks to support integration tests that require elevated privileges. Integration test results are now summarized, and benchmarks are run as part of the workflow. (.github/workflows/ci.yml)cgroups Package Refactoring:
fmt.Errorfwith simplererrors.Newmessages for validation and operational errors, resulting in clearer and more user-friendly error output. (cgroups/cgroup.go)writeFile) and inlines file writing logic, reducing indirection and improving readability. (cgroups/cgroup.go)cgroups/cgroup.go)cgroups/cgroup.go)cgroups/cgroup.go) [1] [2]container Package Improvements:
fmt.Errorftoerrors.Newfor error creation, and simplifies environment variable handling for container setup. (container/init.go,container/run.go) [1] [2]container/init.go)container/init.go)