Skip to content

Commit 7ea5c74

Browse files
committed
refactor: extract Docker Image Builder with constants
- Extract DockerImageBuilder as independent module with builder pattern - Implement required field validation and comprehensive error handling - Add DEFAULT_IMAGE_NAME and DEFAULT_IMAGE_TAG constants to eliminate duplication - Provide fluent interface with method chaining for configuration - Maintain backward compatibility with existing provisioned container API - Add 13 unit tests for builder validation and error scenarios Implements point 1 of provisioned container refactoring plan
1 parent 975de6d commit 7ea5c74

File tree

5 files changed

+629
-41
lines changed

5 files changed

+629
-41
lines changed

docs/refactors/provisioned-container-refactoring.md

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -65,36 +65,58 @@ This structure enables:
6565

6666
## 🏗️ Architecture & Design Patterns
6767

68-
### 1. Extract Docker Image Builder
68+
### 1. Extract Docker Image Builder (Completed)
6969

70-
**Current Issue**: Docker image building logic is embedded in the container state machine.
70+
**Issue Resolved**: Docker image building logic was embedded in the container state machine.
7171

72-
**Proposed Solution**:
72+
**Implementation Completed**:
7373

7474
```rust
7575
pub struct DockerImageBuilder {
76-
image_name: String,
77-
tag: String,
78-
dockerfile_path: PathBuf,
79-
context_path: PathBuf,
80-
build_timeout: Duration,
76+
image_name: Option<String>, // Required field validation
77+
tag: String, // Default: "latest"
78+
dockerfile_path: Option<PathBuf>, // Required field validation
79+
context_path: PathBuf, // Default: "."
80+
build_timeout: Duration, // Default: 300 seconds
8181
}
8282

8383
impl DockerImageBuilder {
8484
pub fn new() -> Self { /* ... */ }
8585
pub fn with_name(mut self, name: impl Into<String>) -> Self { /* ... */ }
8686
pub fn with_tag(mut self, tag: impl Into<String>) -> Self { /* ... */ }
8787
pub fn with_dockerfile(mut self, path: PathBuf) -> Self { /* ... */ }
88+
pub fn with_context(mut self, path: PathBuf) -> Self { /* ... */ }
89+
pub fn with_build_timeout(mut self, timeout: Duration) -> Self { /* ... */ }
8890
pub fn build(&self) -> Result<()> { /* ... */ }
91+
pub fn image_tag(&self) -> String { /* ... */ }
92+
}
93+
94+
// Comprehensive error handling
95+
#[derive(Debug, thiserror::Error)]
96+
pub enum DockerBuildError {
97+
#[error("Failed to execute docker build command for image '{image_name}:{tag}': {source}")]
98+
DockerBuildExecution { /* ... */ },
99+
#[error("Docker build failed for image '{image_name}:{tag}' with stderr: {stderr}")]
100+
DockerBuildFailed { /* ... */ },
101+
#[error("Image name is required but was not provided")]
102+
ImageNameRequired,
103+
#[error("Dockerfile path is required but was not provided")]
104+
DockerfilePathRequired,
89105
}
90106
```
91107

92-
**Benefits**:
108+
**Benefits Achieved**:
109+
110+
- ✅ Single Responsibility Principle
111+
- ✅ Explicit configuration with required field validation
112+
- ✅ Comprehensive error handling with specific error types
113+
- ✅ Builder pattern with method chaining
114+
- ✅ Full test coverage (13 unit tests)
115+
- ✅ Integration with existing provisioned container error chain
116+
- ✅ Reusable across different container types
117+
- ✅ Configurable image parameters with sensible defaults
93118

94-
- Single Responsibility Principle
95-
- Easier testing of build logic
96-
- Configurable image parameters
97-
- Reusable across different container types
119+
**Module Location**: `src/e2e/containers/docker_builder.rs`
98120

99121
### 2. Container Configuration Builder
100122

@@ -513,9 +535,9 @@ let container = StoppedProvisionedContainer::builder()
513535
3.**Documentation Updates** - Updated all references to new module structure
514536
4.**Test Validation** - Ensured all tests pass with new structure
515537

516-
### Phase 1: Foundation (High Priority)
538+
### Phase 1: Foundation (High Priority) - Partially Complete
517539

518-
1. Extract Docker Image Builder
540+
1. **Extract Docker Image Builder** - Implemented independent `DockerImageBuilder` with builder pattern, explicit configuration, required field validation, comprehensive error handling, and full integration with provisioned container module
519541
2. Improve Error Context
520542
3. Extract Magic Numbers and Strings
521543
4. Split Large Functions

src/bin/e2e_config_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ use tracing::{error, info};
3333
use torrust_tracker_deploy::application::commands::ConfigureCommand;
3434
use torrust_tracker_deploy::config::{Config, InstanceName, SshCredentials};
3535
use torrust_tracker_deploy::container::Services;
36-
use torrust_tracker_deploy::e2e::environment::TestEnvironment;
3736
use torrust_tracker_deploy::e2e::containers::StoppedProvisionedContainer;
37+
use torrust_tracker_deploy::e2e::environment::TestEnvironment;
3838
use torrust_tracker_deploy::e2e::tasks::preflight_cleanup;
3939
use torrust_tracker_deploy::e2e::tasks::provision_docker_infrastructure::provision_docker_infrastructure;
4040
use torrust_tracker_deploy::logging::{self, LogFormat};

0 commit comments

Comments
 (0)