diff --git a/.golangci.yml b/.golangci.yml index 580fcaf..a440ec3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -486,4 +486,5 @@ linters: - path: '^docs/' linters: - gochecknoglobals - - gochecknoinits \ No newline at end of file + - gochecknoinits + - godot \ No newline at end of file diff --git a/cmd/e2e-imagegen/main.go b/cmd/e2e-imagegen/main.go index 57889ab..caab1aa 100644 --- a/cmd/e2e-imagegen/main.go +++ b/cmd/e2e-imagegen/main.go @@ -1,6 +1,7 @@ package main import ( + "errors" "flag" "fmt" "os" @@ -25,7 +26,7 @@ func main() { verbose = flag.Bool("verbose", false, "Enable verbose logging") ) - flag.Usage = func() { + flag.Usage = func() { //nolint:reassign // customizing CLI usage output fmt.Fprintf(os.Stderr, "e2e-imagegen - Generate variant container images for e2e testing\n\n") fmt.Fprintf(os.Stderr, "Usage: e2e-imagegen [options]\n\n") fmt.Fprintf(os.Stderr, "Options:\n") @@ -54,7 +55,7 @@ func main() { fmt.Fprintf(os.Stderr, "Failed to create logger: %v\n", err) os.Exit(1) } - defer logger.Sync() //nolint:errcheck + defer logger.Sync() //nolint:errcheck // best-effort sync on exit // Build configuration config := imagegen.Config{ @@ -69,8 +70,8 @@ func main() { } // Validate configuration - if err := validateConfig(config); err != nil { - logger.Fatal("Invalid configuration", zap.Error(err)) + if validateErr := validateConfig(config); validateErr != nil { + logger.Fatal("Invalid configuration", zap.Error(validateErr)) } logger.Info("Starting image generation", @@ -91,8 +92,8 @@ func main() { } // Write manifest - if err := manifest.WriteManifest(*output); err != nil { - logger.Fatal("Failed to write manifest", zap.Error(err)) + if writeErr := manifest.WriteManifest(*output); writeErr != nil { + logger.Fatal("Failed to write manifest", zap.Error(writeErr)) } logger.Info("Image generation complete", @@ -103,56 +104,56 @@ func main() { ) // Print summary - fmt.Println() - fmt.Println("=== Generation Summary ===") - fmt.Printf("Total images: %d\n", manifest.Statistics.TotalImages) - fmt.Printf("Total size: %s\n", formatBytes(manifest.Statistics.TotalSize)) - fmt.Printf("Average image size: %s\n", formatBytes(manifest.Statistics.AverageSize)) - fmt.Printf("Total layers: %d\n", manifest.Statistics.TotalLayers) - fmt.Printf("Average layer size: %s\n", formatBytes(manifest.Statistics.AverageLayerSize)) - fmt.Printf("Images with sharing: %d\n", manifest.Statistics.ImagesWithSharing) - fmt.Printf("Manifest written to: %s\n", *output) + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, "=== Generation Summary ===") + fmt.Fprintf(os.Stdout, "Total images: %d\n", manifest.Statistics.TotalImages) + fmt.Fprintf(os.Stdout, "Total size: %s\n", formatBytes(manifest.Statistics.TotalSize)) + fmt.Fprintf(os.Stdout, "Average image size: %s\n", formatBytes(manifest.Statistics.AverageSize)) + fmt.Fprintf(os.Stdout, "Total layers: %d\n", manifest.Statistics.TotalLayers) + fmt.Fprintf(os.Stdout, "Average layer size: %s\n", formatBytes(manifest.Statistics.AverageLayerSize)) + fmt.Fprintf(os.Stdout, "Images with sharing: %d\n", manifest.Statistics.ImagesWithSharing) + fmt.Fprintf(os.Stdout, "Manifest written to: %s\n", *output) } func validateConfig(config imagegen.Config) error { if config.NumVariants < 1 { - return fmt.Errorf("variants must be at least 1") + return errors.New("variants must be at least 1") } if config.MinLayers < 1 { - return fmt.Errorf("min-layers must be at least 1") + return errors.New("min-layers must be at least 1") } if config.MaxLayers < config.MinLayers { - return fmt.Errorf("max-layers must be >= min-layers") + return errors.New("max-layers must be >= min-layers") } if config.LayerSharingPercent < 0 || config.LayerSharingPercent > 100 { - return fmt.Errorf("sharing must be between 0 and 100") + return errors.New("sharing must be between 0 and 100") } if config.Concurrency < 1 { - return fmt.Errorf("concurrency must be at least 1") + return errors.New("concurrency must be at least 1") } if config.BaseImage == "" { - return fmt.Errorf("base image cannot be empty") + return errors.New("base image cannot be empty") } if config.TargetRegistry == "" { - return fmt.Errorf("registry cannot be empty") + return errors.New("registry cannot be empty") } return nil } func formatBytes(bytes int64) string { const ( - KB = 1024 - MB = KB * 1024 - GB = MB * 1024 + kb = 1024 + mb = kb * 1024 + gb = mb * 1024 ) switch { - case bytes >= GB: - return fmt.Sprintf("%.2f GB", float64(bytes)/float64(GB)) - case bytes >= MB: - return fmt.Sprintf("%.2f MB", float64(bytes)/float64(MB)) - case bytes >= KB: - return fmt.Sprintf("%.2f KB", float64(bytes)/float64(KB)) + case bytes >= gb: + return fmt.Sprintf("%.2f GB", float64(bytes)/float64(gb)) + case bytes >= mb: + return fmt.Sprintf("%.2f MB", float64(bytes)/float64(mb)) + case bytes >= kb: + return fmt.Sprintf("%.2f KB", float64(bytes)/float64(kb)) default: return fmt.Sprintf("%d B", bytes) } diff --git a/docs/docs.go b/docs/docs.go index 3eb2495..56030ac 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -138,6 +138,182 @@ const docTemplate = `{ } } }, + "/api/v1/nodes/{nodeId}/blobs/finalize": { + "post": { + "description": "Completes a blob transfer by updating Redis location", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "rebalance" + ], + "summary": "Finalize blob transfer", + "parameters": [ + { + "type": "string", + "description": "Node identifier", + "name": "nodeId", + "in": "path", + "required": true + }, + { + "description": "Finalize request", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/rebalanceapi.FinalizeRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/rebalanceapi.FinalizeResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + } + } + } + }, + "/api/v1/nodes/{nodeId}/blobs/receive": { + "post": { + "description": "Receives blob data for a reserved transfer", + "consumes": [ + "application/octet-stream" + ], + "produces": [ + "application/json" + ], + "tags": [ + "rebalance" + ], + "summary": "Receive blob data", + "parameters": [ + { + "type": "string", + "description": "Node identifier", + "name": "nodeId", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Reservation ID", + "name": "X-Reservation-ID", + "in": "header", + "required": true + }, + { + "type": "string", + "description": "Expected blob digest", + "name": "X-Digest", + "in": "header", + "required": true + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/rebalanceapi.ReceiveResponse" + } + }, + "400": { + "description": "Digest mismatch", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "404": { + "description": "Reservation not found or expired", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + } + } + } + }, + "/api/v1/nodes/{nodeId}/blobs/reserve": { + "post": { + "description": "Reserves disk space for an incoming blob transfer", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "rebalance" + ], + "summary": "Reserve space for blob", + "parameters": [ + { + "type": "string", + "description": "Node identifier", + "name": "nodeId", + "in": "path", + "required": true + }, + { + "description": "Reserve request", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/rebalanceapi.ReserveRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/rebalanceapi.ReserveResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "409": { + "description": "Blob already exists", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "507": { + "description": "Insufficient storage", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + } + } + } + }, "/api/v1/nodes/{nodeId}/blobs/{digest}": { "get": { "description": "Returns information about a specific blob on the node", @@ -409,11 +585,6 @@ const docTemplate = `{ "nodesapi.NodeResponse": { "type": "object", "properties": { - "hostname": { - "description": "Hostname is the system hostname of the node.", - "type": "string", - "example": "barnacle-1" - }, "lastUpdated": { "description": "LastUpdated is the timestamp when this information was last updated.", "type": "string" @@ -450,6 +621,102 @@ const docTemplate = `{ } } }, + "rebalanceapi.FinalizeRequest": { + "type": "object", + "required": [ + "digest", + "reservationId" + ], + "properties": { + "digest": { + "description": "Digest is the digest of the blob being finalized.", + "type": "string", + "example": "sha256:abc123def456..." + }, + "reservationId": { + "description": "ReservationID is the reservation to finalize.", + "type": "string", + "example": "550e8400-e29b-41d4-a716-446655440000" + }, + "tier": { + "description": "Tier is the cache tier where the blob was stored.", + "type": "integer", + "example": 0 + } + } + }, + "rebalanceapi.FinalizeResponse": { + "type": "object", + "properties": { + "success": { + "description": "Success indicates whether finalization succeeded.", + "type": "boolean", + "example": true + } + } + }, + "rebalanceapi.ReceiveResponse": { + "type": "object", + "properties": { + "digest": { + "description": "Digest is the digest of the received blob.", + "type": "string", + "example": "sha256:abc123def456..." + }, + "size": { + "description": "Size is the size of the received blob in bytes.", + "type": "integer", + "example": 1048576 + } + } + }, + "rebalanceapi.ReserveRequest": { + "type": "object", + "required": [ + "digest", + "mediaType", + "size", + "sourceNodeId" + ], + "properties": { + "digest": { + "description": "Digest is the content-addressable digest of the blob to reserve space for.", + "type": "string", + "example": "sha256:abc123def456..." + }, + "mediaType": { + "description": "MediaType is the OCI media type of the blob.", + "type": "string", + "example": "application/vnd.oci.image.layer.v1.tar+gzip" + }, + "size": { + "description": "Size is the size of the blob in bytes.", + "type": "integer", + "minimum": 1, + "example": 1048576 + }, + "sourceNodeId": { + "description": "SourceNodeID is the node that currently holds the blob.", + "type": "string", + "example": "node-1" + } + } + }, + "rebalanceapi.ReserveResponse": { + "type": "object", + "properties": { + "expiresAt": { + "description": "ExpiresAt is when this reservation expires if not used.", + "type": "string", + "example": "2024-01-15T10:30:00Z" + }, + "reservationId": { + "description": "ReservationID is a unique identifier for this reservation.", + "type": "string", + "example": "550e8400-e29b-41d4-a716-446655440000" + } + } + }, "upstreamsapi.GetUpstreamResponse": { "type": "object", "properties": { @@ -507,7 +774,7 @@ const docTemplate = `{ ] }` -// SwaggerInfo holds exported Swagger Info so clients can modify it. +// SwaggerInfo holds exported Swagger Info so clients can modify it var SwaggerInfo = &swag.Spec{ Version: "1.0", Host: "localhost:8080", diff --git a/docs/swagger.json b/docs/swagger.json index 24a134a..83879a0 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -132,6 +132,182 @@ } } }, + "/api/v1/nodes/{nodeId}/blobs/finalize": { + "post": { + "description": "Completes a blob transfer by updating Redis location", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "rebalance" + ], + "summary": "Finalize blob transfer", + "parameters": [ + { + "type": "string", + "description": "Node identifier", + "name": "nodeId", + "in": "path", + "required": true + }, + { + "description": "Finalize request", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/rebalanceapi.FinalizeRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/rebalanceapi.FinalizeResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + } + } + } + }, + "/api/v1/nodes/{nodeId}/blobs/receive": { + "post": { + "description": "Receives blob data for a reserved transfer", + "consumes": [ + "application/octet-stream" + ], + "produces": [ + "application/json" + ], + "tags": [ + "rebalance" + ], + "summary": "Receive blob data", + "parameters": [ + { + "type": "string", + "description": "Node identifier", + "name": "nodeId", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Reservation ID", + "name": "X-Reservation-ID", + "in": "header", + "required": true + }, + { + "type": "string", + "description": "Expected blob digest", + "name": "X-Digest", + "in": "header", + "required": true + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/rebalanceapi.ReceiveResponse" + } + }, + "400": { + "description": "Digest mismatch", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "404": { + "description": "Reservation not found or expired", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + } + } + } + }, + "/api/v1/nodes/{nodeId}/blobs/reserve": { + "post": { + "description": "Reserves disk space for an incoming blob transfer", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "rebalance" + ], + "summary": "Reserve space for blob", + "parameters": [ + { + "type": "string", + "description": "Node identifier", + "name": "nodeId", + "in": "path", + "required": true + }, + { + "description": "Reserve request", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/rebalanceapi.ReserveRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/rebalanceapi.ReserveResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "409": { + "description": "Blob already exists", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + }, + "507": { + "description": "Insufficient storage", + "schema": { + "$ref": "#/definitions/httptk.ErrorsList" + } + } + } + } + }, "/api/v1/nodes/{nodeId}/blobs/{digest}": { "get": { "description": "Returns information about a specific blob on the node", @@ -403,11 +579,6 @@ "nodesapi.NodeResponse": { "type": "object", "properties": { - "hostname": { - "description": "Hostname is the system hostname of the node.", - "type": "string", - "example": "barnacle-1" - }, "lastUpdated": { "description": "LastUpdated is the timestamp when this information was last updated.", "type": "string" @@ -444,6 +615,102 @@ } } }, + "rebalanceapi.FinalizeRequest": { + "type": "object", + "required": [ + "digest", + "reservationId" + ], + "properties": { + "digest": { + "description": "Digest is the digest of the blob being finalized.", + "type": "string", + "example": "sha256:abc123def456..." + }, + "reservationId": { + "description": "ReservationID is the reservation to finalize.", + "type": "string", + "example": "550e8400-e29b-41d4-a716-446655440000" + }, + "tier": { + "description": "Tier is the cache tier where the blob was stored.", + "type": "integer", + "example": 0 + } + } + }, + "rebalanceapi.FinalizeResponse": { + "type": "object", + "properties": { + "success": { + "description": "Success indicates whether finalization succeeded.", + "type": "boolean", + "example": true + } + } + }, + "rebalanceapi.ReceiveResponse": { + "type": "object", + "properties": { + "digest": { + "description": "Digest is the digest of the received blob.", + "type": "string", + "example": "sha256:abc123def456..." + }, + "size": { + "description": "Size is the size of the received blob in bytes.", + "type": "integer", + "example": 1048576 + } + } + }, + "rebalanceapi.ReserveRequest": { + "type": "object", + "required": [ + "digest", + "mediaType", + "size", + "sourceNodeId" + ], + "properties": { + "digest": { + "description": "Digest is the content-addressable digest of the blob to reserve space for.", + "type": "string", + "example": "sha256:abc123def456..." + }, + "mediaType": { + "description": "MediaType is the OCI media type of the blob.", + "type": "string", + "example": "application/vnd.oci.image.layer.v1.tar+gzip" + }, + "size": { + "description": "Size is the size of the blob in bytes.", + "type": "integer", + "minimum": 1, + "example": 1048576 + }, + "sourceNodeId": { + "description": "SourceNodeID is the node that currently holds the blob.", + "type": "string", + "example": "node-1" + } + } + }, + "rebalanceapi.ReserveResponse": { + "type": "object", + "properties": { + "expiresAt": { + "description": "ExpiresAt is when this reservation expires if not used.", + "type": "string", + "example": "2024-01-15T10:30:00Z" + }, + "reservationId": { + "description": "ReservationID is a unique identifier for this reservation.", + "type": "string", + "example": "550e8400-e29b-41d4-a716-446655440000" + } + } + }, "upstreamsapi.GetUpstreamResponse": { "type": "object", "properties": { diff --git a/docs/swagger.yaml b/docs/swagger.yaml index fc29e85..2a27c07 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -83,10 +83,6 @@ definitions: type: object nodesapi.NodeResponse: properties: - hostname: - description: Hostname is the system hostname of the node. - example: barnacle-1 - type: string lastUpdated: description: LastUpdated is the timestamp when this information was last updated. type: string @@ -111,6 +107,79 @@ definitions: $ref: '#/definitions/nodesapi.DiskUsageResponse' type: array type: object + rebalanceapi.FinalizeRequest: + properties: + digest: + description: Digest is the digest of the blob being finalized. + example: sha256:abc123def456... + type: string + reservationId: + description: ReservationID is the reservation to finalize. + example: 550e8400-e29b-41d4-a716-446655440000 + type: string + tier: + description: Tier is the cache tier where the blob was stored. + example: 0 + type: integer + required: + - digest + - reservationId + type: object + rebalanceapi.FinalizeResponse: + properties: + success: + description: Success indicates whether finalization succeeded. + example: true + type: boolean + type: object + rebalanceapi.ReceiveResponse: + properties: + digest: + description: Digest is the digest of the received blob. + example: sha256:abc123def456... + type: string + size: + description: Size is the size of the received blob in bytes. + example: 1048576 + type: integer + type: object + rebalanceapi.ReserveRequest: + properties: + digest: + description: Digest is the content-addressable digest of the blob to reserve + space for. + example: sha256:abc123def456... + type: string + mediaType: + description: MediaType is the OCI media type of the blob. + example: application/vnd.oci.image.layer.v1.tar+gzip + type: string + size: + description: Size is the size of the blob in bytes. + example: 1048576 + minimum: 1 + type: integer + sourceNodeId: + description: SourceNodeID is the node that currently holds the blob. + example: node-1 + type: string + required: + - digest + - mediaType + - size + - sourceNodeId + type: object + rebalanceapi.ReserveResponse: + properties: + expiresAt: + description: ExpiresAt is when this reservation expires if not used. + example: "2024-01-15T10:30:00Z" + type: string + reservationId: + description: ReservationID is a unique identifier for this reservation. + example: 550e8400-e29b-41d4-a716-446655440000 + type: string + type: object upstreamsapi.GetUpstreamResponse: properties: alias: @@ -267,6 +336,123 @@ paths: summary: Get blob tags: - blobs + /api/v1/nodes/{nodeId}/blobs/finalize: + post: + consumes: + - application/json + description: Completes a blob transfer by updating Redis location + parameters: + - description: Node identifier + in: path + name: nodeId + required: true + type: string + - description: Finalize request + in: body + name: body + required: true + schema: + $ref: '#/definitions/rebalanceapi.FinalizeRequest' + produces: + - application/json + responses: + "200": + description: OK + schema: + $ref: '#/definitions/rebalanceapi.FinalizeResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/httptk.ErrorsList' + "404": + description: Not Found + schema: + $ref: '#/definitions/httptk.ErrorsList' + summary: Finalize blob transfer + tags: + - rebalance + /api/v1/nodes/{nodeId}/blobs/receive: + post: + consumes: + - application/octet-stream + description: Receives blob data for a reserved transfer + parameters: + - description: Node identifier + in: path + name: nodeId + required: true + type: string + - description: Reservation ID + in: header + name: X-Reservation-ID + required: true + type: string + - description: Expected blob digest + in: header + name: X-Digest + required: true + type: string + produces: + - application/json + responses: + "201": + description: Created + schema: + $ref: '#/definitions/rebalanceapi.ReceiveResponse' + "400": + description: Digest mismatch + schema: + $ref: '#/definitions/httptk.ErrorsList' + "404": + description: Reservation not found or expired + schema: + $ref: '#/definitions/httptk.ErrorsList' + summary: Receive blob data + tags: + - rebalance + /api/v1/nodes/{nodeId}/blobs/reserve: + post: + consumes: + - application/json + description: Reserves disk space for an incoming blob transfer + parameters: + - description: Node identifier + in: path + name: nodeId + required: true + type: string + - description: Reserve request + in: body + name: body + required: true + schema: + $ref: '#/definitions/rebalanceapi.ReserveRequest' + produces: + - application/json + responses: + "201": + description: Created + schema: + $ref: '#/definitions/rebalanceapi.ReserveResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/httptk.ErrorsList' + "404": + description: Not Found + schema: + $ref: '#/definitions/httptk.ErrorsList' + "409": + description: Blob already exists + schema: + $ref: '#/definitions/httptk.ErrorsList' + "507": + description: Insufficient storage + schema: + $ref: '#/definitions/httptk.ErrorsList' + summary: Reserve space for blob + tags: + - rebalance /api/v1/nodes/me: get: description: Returns information about this node diff --git a/internal/registry/cache/coordinator/coordinator.go b/internal/registry/cache/coordinator/coordinator.go index f7e095a..f285e36 100644 --- a/internal/registry/cache/coordinator/coordinator.go +++ b/internal/registry/cache/coordinator/coordinator.go @@ -322,7 +322,7 @@ func (c *coordinatorBlobCache) deleteLocation(ctx context.Context, digest string // even when multiple accesses occur in the same millisecond. // //nolint:gochecknoglobals // Required for atomic counter operations across instances -var accessCounter uint64 +var accessCounter atomic.Uint64 // recordAccess records an access event for a blob using a sorted set. // Each access is stored with the timestamp as the score and a unique ID as the member. @@ -336,7 +336,7 @@ func (c *coordinatorBlobCache) recordAccess(ctx context.Context, digest string) // Use atomic increment to ensure unique member even in same millisecond // Format: timestamp:counter to ensure uniqueness - counter := atomic.AddUint64(&accessCounter, 1) + counter := accessCounter.Add(1) member := fmt.Sprintf("%d:%d", now, counter) // Add the access event @@ -351,7 +351,7 @@ func (c *coordinatorBlobCache) recordAccess(ctx context.Context, digest string) // TODO: Refactor sliding window cleanup into a background task. // Clean up old entries outside the window (do this asynchronously to not block) windowStart := now - c.accessWindowDuration.Milliseconds() - go func() { + go func() { //nolint:gosec // intentionally outlives request context for async cleanup cleanErr := c.redis.ZRemRangeByScore( context.Background(), key, diff --git a/internal/registry/cache/coordinator/rebalance/capacity.go b/internal/registry/cache/coordinator/rebalance/capacity.go index 66c8e30..9db4a93 100644 --- a/internal/registry/cache/coordinator/rebalance/capacity.go +++ b/internal/registry/cache/coordinator/rebalance/capacity.go @@ -1,6 +1,8 @@ package rebalance import ( + "strconv" + "github.com/pdylanross/barnacle/internal/node" "github.com/pdylanross/barnacle/pkg/configuration" ) @@ -54,7 +56,7 @@ func (b *TierBucket) CanFit(sizeBytes int64) bool { // Assign adds a blob to this bucket and updates the assigned bytes. func (b *TierBucket) Assign(blob *EnrichedBlob) { b.AssignedBlobs = append(b.AssignedBlobs, blob) - b.AssignedBytes += uint64(max(0, blob.Size)) //nolint:gosec // size validated by CanFit before Assign + b.AssignedBytes += uint64(max(0, blob.Size)) } // BuildClusterCapacity creates a capacity snapshot from the node list. @@ -245,7 +247,7 @@ func PlaceBlobsOnNodes(buckets []*TierBucket, capacity *ClusterCapacity) []*Node if currentNodeCap, ok := nodeCapMap[currentNode]; ok && currentNodeCap.IsHealthy { if targetTier < len(currentNodeCap.Tiers) { tierCap := currentNodeCap.Tiers[targetTier] - key := currentNode + ":" + string(rune(targetTier)) + key := currentNode + ":" + strconv.Itoa(targetTier) used := assignedBytes[key] available := tierCap.FreeBytes if used < available { @@ -254,7 +256,7 @@ func PlaceBlobsOnNodes(buckets []*TierBucket, capacity *ClusterCapacity) []*Node available = 0 } - blobSize := uint64(max(0, blob.Size)) //nolint:gosec // size is always non-negative for valid blobs + blobSize := uint64(max(0, blob.Size)) if blobSize <= available { // Keep on current node placement.TargetNode = currentNode @@ -277,7 +279,7 @@ func PlaceBlobsOnNodes(buckets []*TierBucket, capacity *ClusterCapacity) []*Node } tierCap := nodeCap.Tiers[targetTier] - key := nodeCap.NodeID + ":" + string(rune(targetTier)) + key := nodeCap.NodeID + ":" + strconv.Itoa(targetTier) used := assignedBytes[key] available := tierCap.FreeBytes if used < available { @@ -286,7 +288,7 @@ func PlaceBlobsOnNodes(buckets []*TierBucket, capacity *ClusterCapacity) []*Node available = 0 } - blobSize := uint64(max(0, blob.Size)) //nolint:gosec // size is always non-negative for valid blobs + blobSize := uint64(max(0, blob.Size)) if blobSize <= available { placement.TargetNode = nodeCap.NodeID placement.NeedsMove = nodeCap.NodeID != blob.CurrentNode || targetTier != blob.CurrentTier diff --git a/internal/registry/cache/coordinator/rebalance/planner.go b/internal/registry/cache/coordinator/rebalance/planner.go index b40e71e..4e11baa 100644 --- a/internal/registry/cache/coordinator/rebalance/planner.go +++ b/internal/registry/cache/coordinator/rebalance/planner.go @@ -219,7 +219,6 @@ func (p *Planner) filterCooldownBlobs( if onCooldown { // Track size by tier for capacity calculation if blob.Tier < len(nodeCooldownSizes) { - //nolint:gosec // size is always non-negative for valid blobs nodeCooldownSizes[blob.Tier] += uint64(max(0, blob.Size)) } } else { diff --git a/internal/registry/upstream/caching.go b/internal/registry/upstream/caching.go index e3af779..edb2401 100644 --- a/internal/registry/upstream/caching.go +++ b/internal/registry/upstream/caching.go @@ -505,7 +505,7 @@ func (c *cachingUpstream) GetBlob(ctx context.Context, repo string, digest v1.Ha // Start a goroutine to read from upstream, write to cache, and close the pipe // Capture repo for use in the goroutine since it may outlive the request repoName := repo - go func() { + go func() { //nolint:gosec // intentionally outlives request context for background caching defer reservation.Release() defer upstreamReader.Close() defer pipeWriter.Close() diff --git a/pkg/configuration/cache.go b/pkg/configuration/cache.go index de268b7..2457053 100644 --- a/pkg/configuration/cache.go +++ b/pkg/configuration/cache.go @@ -71,7 +71,11 @@ func (t *DiskTierConfiguration) GetSizeLimitBytes() (uint64, error) { if err != nil { return 0, fmt.Errorf("invalid sizeLimit %q: %w", t.SizeLimit, err) } - return uint64(q.Value()), nil + v := q.Value() + if v < 0 { + return 0, fmt.Errorf("sizeLimit %q must not be negative", t.SizeLimit) + } + return uint64(v), nil } // Validate checks that the disk tier configuration is valid. @@ -90,7 +94,7 @@ func (t *DiskTierConfiguration) Validate() error { } if t.SizeLimit != "" { if _, err := t.GetSizeLimitBytes(); err != nil { - return fmt.Errorf("%w: disk tier %d: %v", + return fmt.Errorf("%w: disk tier %d: %w", ErrInvalidConfiguration, t.Tier, err) } } diff --git a/test/e2e/framework/barnacle.go b/test/e2e/framework/barnacle.go index 80f27d4..fdca7a0 100644 --- a/test/e2e/framework/barnacle.go +++ b/test/e2e/framework/barnacle.go @@ -2,13 +2,15 @@ package framework import ( "context" + "errors" "fmt" "io" "net/http" + "strconv" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "go.uber.org/zap" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Barnacle provides helpers for interacting with the barnacle deployment. @@ -21,12 +23,17 @@ type Barnacle struct { logger *zap.Logger } +const ( + barnaclePort = 8080 + httpClientTimeout = 10 * time.Second +) + // NewBarnacle creates a new barnacle helper. func NewBarnacle(cluster *Cluster, serviceName string, logger *zap.Logger) *Barnacle { return &Barnacle{ cluster: cluster, serviceName: serviceName, - port: 8080, + port: barnaclePort, logger: logger, } } @@ -68,7 +75,7 @@ func (b *Barnacle) ImageURL(upstream, imageName, tag string) string { if b.nodeAddress != "" { host = b.nodeAddress } else { - host = b.serviceName + "." + b.cluster.Namespace() + ".svc.cluster.local:" + fmt.Sprint(b.port) + host = b.serviceName + "." + b.cluster.Namespace() + ".svc.cluster.local:" + strconv.Itoa(b.port) } return fmt.Sprintf("%s/%s/%s:%s", host, upstream, imageName, tag) } @@ -101,7 +108,7 @@ func (b *Barnacle) CheckHealth(ctx context.Context, localPort int) error { b.logger.Debug("Checking health via localhost", zap.String("url", url)) } - client := &http.Client{Timeout: 10 * time.Second} + client := &http.Client{Timeout: httpClientTimeout} req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return fmt.Errorf("failed to create request: %w", err) @@ -126,13 +133,13 @@ func (b *Barnacle) CheckHealth(ctx context.Context, localPort int) error { // Returns an error if ingress host is not configured. func (b *Barnacle) CheckHealthViaIngress(ctx context.Context) error { if b.ingressHost == "" { - return fmt.Errorf("ingress host not configured") + return errors.New("ingress host not configured") } url := fmt.Sprintf("http://%s/healthz", b.ingressHost) b.logger.Debug("Checking health via ingress", zap.String("url", url)) - client := &http.Client{Timeout: 10 * time.Second} + client := &http.Client{Timeout: httpClientTimeout} req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return fmt.Errorf("failed to create request: %w", err) diff --git a/test/e2e/framework/cluster.go b/test/e2e/framework/cluster.go index 6a97e60..560a0d5 100644 --- a/test/e2e/framework/cluster.go +++ b/test/e2e/framework/cluster.go @@ -114,6 +114,8 @@ func (c *Cluster) WaitForPodComplete(ctx context.Context, name string, timeout t return true, nil case corev1.PodPending, corev1.PodRunning: return false, nil + case corev1.PodUnknown: + return false, nil default: return false, fmt.Errorf("unexpected pod phase: %s", pod.Status.Phase) } @@ -145,6 +147,8 @@ func (c *Cluster) WaitForPodRunning(ctx context.Context, name string, timeout ti return false, fmt.Errorf("pod completed unexpectedly with phase: %s", pod.Status.Phase) case corev1.PodPending: return false, nil + case corev1.PodUnknown: + return false, nil default: return false, fmt.Errorf("unexpected pod phase: %s", pod.Status.Phase) } @@ -197,7 +201,7 @@ func (c *Cluster) WaitForDeploymentReady(ctx context.Context, name string, timeo func (c *Cluster) CheckNamespaceExists(ctx context.Context) (bool, error) { _, err := c.clientset.CoreV1().Namespaces().Get(ctx, c.namespace, metav1.GetOptions{}) if err != nil { - return false, nil + return false, fmt.Errorf("failed to get namespace %s: %w", c.namespace, err) } return true, nil } diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 94abb62..0120bb0 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -71,8 +71,9 @@ func (f *Framework) Setup(ctx context.Context) error { } // Wait for barnacle to be ready - if err := f.barnacle.WaitForReady(ctx, 2*time.Minute); err != nil { - return fmt.Errorf("barnacle not ready: %w", err) + const readyTimeout = 2 * time.Minute + if readyErr := f.barnacle.WaitForReady(ctx, readyTimeout); readyErr != nil { + return fmt.Errorf("barnacle not ready: %w", readyErr) } f.logger.Info("Framework setup complete") diff --git a/test/e2e/framework/options.go b/test/e2e/framework/options.go index 7b3fdc4..a9585a1 100644 --- a/test/e2e/framework/options.go +++ b/test/e2e/framework/options.go @@ -62,12 +62,20 @@ type Options struct { BarnacleNodeAddress string } +const ( + defaultWorkers = 10 + defaultIterations = 10000 + defaultTimeout = 5 * time.Minute + defaultKubeQPS = float32(20000) + defaultKubeBurst = 40000 +) + // DefaultOptions returns the default options for e2e tests. func DefaultOptions() Options { return Options{ - Workers: 10, - Iterations: 10000, - Timeout: 5 * time.Minute, + Workers: defaultWorkers, + Iterations: defaultIterations, + Timeout: defaultTimeout, KubeContext: "barnacle-e2e", Namespace: "barnacle-e2e", BarnacleService: "barnacle", @@ -75,8 +83,8 @@ func DefaultOptions() Options { ResultsPath: "", PodImage: "busybox:latest", UpstreamName: "local", - KubeQPS: 20000, - KubeBurst: 40000, + KubeQPS: defaultKubeQPS, + KubeBurst: defaultKubeBurst, DeletePods: true, Verbose: false, BarnacleIngressHost: "barnacle.test", diff --git a/test/e2e/imagegen/generator.go b/test/e2e/imagegen/generator.go index 017d462..66e0b24 100644 --- a/test/e2e/imagegen/generator.go +++ b/test/e2e/imagegen/generator.go @@ -46,16 +46,23 @@ type Config struct { Insecure bool } +const ( + defaultNumVariants = 100 + defaultMaxLayers = 4 + defaultSharingPercent = 50 + defaultConcurrency = 4 +) + // DefaultConfig returns a default configuration. func DefaultConfig() Config { return Config{ BaseImage: "alpine:latest", TargetRegistry: "localhost:5000", - NumVariants: 100, + NumVariants: defaultNumVariants, MinLayers: 1, - MaxLayers: 4, - LayerSharingPercent: 50, - Concurrency: 4, + MaxLayers: defaultMaxLayers, + LayerSharingPercent: defaultSharingPercent, + Concurrency: defaultConcurrency, Insecure: true, } } @@ -139,19 +146,17 @@ func (g *Generator) Generate() (*ImageManifest, error) { // Start workers var wg sync.WaitGroup - for i := 0; i < g.config.Concurrency; i++ { - wg.Add(1) - go func(workerID int) { - defer wg.Done() + for range g.config.Concurrency { + wg.Go(func() { for idx := range workChan { - err := g.generateImage(idx, baseImg) - resultChan <- err + genErr := g.generateImage(idx, baseImg) + resultChan <- genErr } - }(i) + }) } // Queue work - for i := 0; i < g.config.NumVariants; i++ { + for i := range g.config.NumVariants { workChan <- i } close(workChan) @@ -207,25 +212,11 @@ func (g *Generator) generateImage(idx int, baseImg v1.Image) error { img := baseImg if idx > 0 && g.shouldShareLayers() { - parentEntry := g.getRandomPreviousImage() - if parentEntry != nil { - parentRef = parentEntry.Reference - ref, err := name.ParseReference(parentRef) - if err == nil { - opts := g.remoteOptions() - if parentImg, err := remote.Image(ref, opts...); err == nil { - img = parentImg - g.logger.Debug("Using parent image", - zap.String("parent", parentRef), - zap.String("child", imageName), - ) - } - } - } + img, parentRef = g.tryUseParentImage(img) } // Determine number of layers to add - numLayers := g.randomInt(g.config.MinLayers, g.config.MaxLayers) + numLayers := g.randomIntRange(g.config.MinLayers, g.config.MaxLayers) // Generate random layers var layerSizes []int64 @@ -233,7 +224,7 @@ func (g *Generator) generateImage(idx int, baseImg v1.Image) error { currentImg := img - for i := 0; i < numLayers; i++ { + for i := range numLayers { size := WeightedRandomSize() layerSizes = append(layerSizes, size) totalSize += size @@ -260,8 +251,8 @@ func (g *Generator) generateImage(idx int, baseImg v1.Image) error { opts := g.remoteOptions() - if err := remote.Write(targetRef, currentImg, opts...); err != nil { - return fmt.Errorf("failed to push image: %w", err) + if writeErr := remote.Write(targetRef, currentImg, opts...); writeErr != nil { + return fmt.Errorf("failed to push image: %w", writeErr) } // Get digest @@ -295,12 +286,36 @@ func (g *Generator) generateImage(idx int, baseImg v1.Image) error { return nil } +func (g *Generator) tryUseParentImage(fallback v1.Image) (v1.Image, string) { + parentEntry := g.getRandomPreviousImage() + if parentEntry == nil { + return fallback, "" + } + + ref, err := name.ParseReference(parentEntry.Reference) + if err != nil { + return fallback, "" + } + + opts := g.remoteOptions() + parentImg, err := remote.Image(ref, opts...) + if err != nil { + return fallback, "" + } + + g.logger.Debug("Using parent image", + zap.String("parent", parentEntry.Reference), + ) + + return parentImg, parentEntry.Reference +} + func (g *Generator) remoteOptions() []remote.Option { opts := []remote.Option{remote.WithAuthFromKeychain(authn.DefaultKeychain)} if g.config.Insecure { transport := &http.Transport{ TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, //nolint:gosec + InsecureSkipVerify: true, //nolint:gosec // e2e test tool pushes to local insecure registries }, } opts = append(opts, remote.WithTransport(transport)) @@ -308,16 +323,18 @@ func (g *Generator) remoteOptions() []remote.Option { return opts } +const percentThreshold = 256 + func (g *Generator) shouldShareLayers() bool { - // Generate random byte randByte := make([]byte, 1) - rand.Read(randByte) //nolint:errcheck + _, _ = rand.Read(randByte) - // Convert to percentage (0-100) - pct := int(randByte[0]) * 100 / 256 + pct := int(randByte[0]) * percentMultiplier / percentThreshold return pct < g.config.LayerSharingPercent } +const percentMultiplier = 100 + func (g *Generator) getRandomPreviousImage() *ImageEntry { g.mu.Lock() defer g.mu.Unlock() @@ -326,29 +343,43 @@ func (g *Generator) getRandomPreviousImage() *ImageEntry { return nil } - // Get random index - randBytes := make([]byte, 4) - rand.Read(randBytes) //nolint:errcheck + randBytes := make([]byte, randBytes4) + _, _ = rand.Read(randBytes) idx := int(randBytes[0]) % len(g.generated) return &g.generated[idx] } -func (g *Generator) randomInt(min, max int) int { - if min >= max { - return min - } +const ( + randBytes4 = 4 + bitShift8 = 8 + bitShift16 = 16 + bitShift24 = 24 +) - randBytes := make([]byte, 4) - rand.Read(randBytes) //nolint:errcheck +func (g *Generator) randomIntRange(lo, hi int) int { + if lo >= hi { + return lo + } - rangeSize := max - min + 1 - val := int(randBytes[0]) | int(randBytes[1])<<8 | int(randBytes[2])<<16 | int(randBytes[3])<<24 + randBytes := make([]byte, randBytes4) + _, _ = rand.Read(randBytes) + + rangeSize := hi - lo + 1 + val := int( + randBytes[0], + ) | int( + randBytes[1], + )<.bin +// The layer contains a single file with random data at /data/.bin. func RandomLayer(size int64) (v1.Layer, error) { buf := new(bytes.Buffer) tw := tar.NewWriter(buf) // Generate random filename - randName := make([]byte, 8) + randName := make([]byte, randNameLen) if _, err := rand.Read(randName); err != nil { return nil, fmt.Errorf("failed to generate random name: %w", err) } @@ -28,7 +42,7 @@ func RandomLayer(size int64) (v1.Layer, error) { // Create the directory entry dirHeader := &tar.Header{ Name: "data/", - Mode: 0755, + Mode: dirPerm, Typeflag: tar.TypeDir, ModTime: time.Now(), } @@ -39,7 +53,7 @@ func RandomLayer(size int64) (v1.Layer, error) { // Create the file header header := &tar.Header{ Name: filename, - Mode: 0644, + Mode: filePerm, Size: size, ModTime: time.Now(), } @@ -48,7 +62,6 @@ func RandomLayer(size int64) (v1.Layer, error) { } // Write random content in chunks to avoid memory issues with large files - const chunkSize = 1024 * 1024 // 1MB chunks remaining := size chunk := make([]byte, chunkSize) @@ -90,45 +103,41 @@ func RandomLayerStream(size int64) (v1.Layer, error) { tw := tar.NewWriter(pw) // Generate random filename - randName := make([]byte, 8) - rand.Read(randName) //nolint:errcheck + randName := make([]byte, randNameLen) + _, _ = rand.Read(randName) filename := fmt.Sprintf("data/%x.bin", randName) // Create the directory entry - tw.WriteHeader(&tar.Header{ //nolint:errcheck + _ = tw.WriteHeader(&tar.Header{ Name: "data/", - Mode: 0755, + Mode: dirPerm, Typeflag: tar.TypeDir, ModTime: time.Now(), }) // Create the file header - tw.WriteHeader(&tar.Header{ //nolint:errcheck + _ = tw.WriteHeader(&tar.Header{ Name: filename, - Mode: 0644, + Mode: filePerm, Size: size, ModTime: time.Now(), }) // Write random content in chunks - const chunkSize = 1024 * 1024 // 1MB chunks remaining := size chunk := make([]byte, chunkSize) for remaining > 0 { - toWrite := int64(chunkSize) - if remaining < toWrite { - toWrite = remaining - } + toWrite := min(remaining, int64(chunkSize)) - rand.Read(chunk[:toWrite]) //nolint:errcheck - tw.Write(chunk[:toWrite]) //nolint:errcheck + _, _ = rand.Read(chunk[:toWrite]) + _, _ = tw.Write(chunk[:toWrite]) remaining -= toWrite } - tw.Close() - pw.Close() + _ = tw.Close() + _ = pw.Close() }() // Use stream.NewLayer to avoid writing temporary files to disk @@ -136,37 +145,31 @@ func RandomLayerStream(size int64) (v1.Layer, error) { } // WeightedRandomSize returns a random layer size following the weighted distribution: -// 80% of layers are 1-100Mi, 20% are up to 1Gi +// 80% of layers are 1-100Mi, 20% are up to 1Gi. func WeightedRandomSize() int64 { - // Generate random byte for weighting decision weightByte := make([]byte, 1) - rand.Read(weightByte) //nolint:errcheck + _, _ = rand.Read(weightByte) - // Use the random byte to determine weight (0-255) - // 80% = 204/256, so if < 204, use small size - if weightByte[0] < 204 { - // Small layer: 1MB to 100MB - return randomInt64(1*1024*1024, 100*1024*1024) + if weightByte[0] < weightThreshold { + return randomInt64Range(smallLayerMin, smallLayerMax) } - // Large layer: 100MB to 1GB - return randomInt64(100*1024*1024, 1024*1024*1024) + return randomInt64Range(largeLayerMin, largeLayerMax) } -// randomInt64 returns a random int64 between min and max (inclusive) -func randomInt64(min, max int64) int64 { - if min >= max { - return min +// randomInt64Range returns a random int64 between lo and hi (inclusive). +func randomInt64Range(lo, hi int64) int64 { + if lo >= hi { + return lo } - rangeSize := max - min + 1 - randBytes := make([]byte, 8) - rand.Read(randBytes) //nolint:errcheck + rangeSize := hi - lo + 1 + randBytes := make([]byte, randInt64Bytes) + _, _ = rand.Read(randBytes) - // Convert to uint64 and scale to range randVal := uint64(randBytes[0]) | uint64(randBytes[1])<<8 | uint64(randBytes[2])<<16 | uint64(randBytes[3])<<24 | uint64(randBytes[4])<<32 | uint64(randBytes[5])<<40 | - uint64(randBytes[6])<<48 | uint64(randBytes[7])<<56 + uint64(randBytes[6])<<48 | uint64(randBytes[7])<= len(sorted) { idx = len(sorted) - 1 } @@ -204,6 +213,8 @@ func (r *Reporter) calculateImageStats(imageName string, results []WorkResult) I return stats } +const filePermissions = 0644 + // WriteJSON writes the report to a JSON file. func (r *Report) WriteJSON(path string) error { data, err := json.MarshalIndent(r, "", " ") @@ -211,24 +222,26 @@ func (r *Report) WriteJSON(path string) error { return fmt.Errorf("failed to marshal report: %w", err) } - if err := os.WriteFile(path, data, 0644); err != nil { - return fmt.Errorf("failed to write report: %w", err) + if writeErr := os.WriteFile(path, data, filePermissions); writeErr != nil { + return fmt.Errorf("failed to write report: %w", writeErr) } return nil } +const dirPermissions = 0755 + // WriteOutputDir writes the report and failed pod events to an output directory. func (r *Reporter) WriteOutputDir(dir string) error { - if err := os.MkdirAll(dir, 0755); err != nil { - return fmt.Errorf("failed to create output directory: %w", err) + if mkdirErr := os.MkdirAll(dir, dirPermissions); mkdirErr != nil { + return fmt.Errorf("failed to create output directory: %w", mkdirErr) } report := r.Generate() // Write report.json - if err := report.WriteJSON(filepath.Join(dir, "report.json")); err != nil { - return err + if reportErr := report.WriteJSON(filepath.Join(dir, "report.json")); reportErr != nil { + return reportErr } // Collect failed pod events from results @@ -240,14 +253,14 @@ func (r *Reporter) WriteOutputDir(dir string) error { } // Write failed-pod-events.json - eventsData, err := json.MarshalIndent(failedEvents, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal failed pod events: %w", err) + eventsData, marshalErr := json.MarshalIndent(failedEvents, "", " ") + if marshalErr != nil { + return fmt.Errorf("failed to marshal failed pod events: %w", marshalErr) } eventsPath := filepath.Join(dir, "failed-pod-events.json") - if err := os.WriteFile(eventsPath, eventsData, 0644); err != nil { - return fmt.Errorf("failed to write failed pod events: %w", err) + if writeErr := os.WriteFile(eventsPath, eventsData, filePermissions); writeErr != nil { + return fmt.Errorf("failed to write failed pod events: %w", writeErr) } // Collect eventual success events from results @@ -259,14 +272,14 @@ func (r *Reporter) WriteOutputDir(dir string) error { } // Write eventual-success-events.json - esData, err := json.MarshalIndent(eventualSuccessEvents, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal eventual success events: %w", err) + esData, esMarshalErr := json.MarshalIndent(eventualSuccessEvents, "", " ") + if esMarshalErr != nil { + return fmt.Errorf("failed to marshal eventual success events: %w", esMarshalErr) } esPath := filepath.Join(dir, "eventual-success-events.json") - if err := os.WriteFile(esPath, esData, 0644); err != nil { - return fmt.Errorf("failed to write eventual success events: %w", err) + if esWriteErr := os.WriteFile(esPath, esData, filePermissions); esWriteErr != nil { + return fmt.Errorf("failed to write eventual success events: %w", esWriteErr) } return nil @@ -274,22 +287,22 @@ func (r *Reporter) WriteOutputDir(dir string) error { // PrintSummary prints a human-readable summary to stdout. func (r *Report) PrintSummary() { - fmt.Println() - fmt.Println("=== E2E Test Results ===") - fmt.Println() - fmt.Printf("Total Iterations: %d\n", r.Summary.TotalIterations) - fmt.Printf("Success Count: %d\n", r.Summary.SuccessCount) - fmt.Printf("Failure Count: %d\n", r.Summary.FailureCount) - fmt.Printf("Eventual Successes: %d\n", r.Summary.EventualSuccessCount) - fmt.Printf("Success Rate: %.2f%%\n", r.Summary.SuccessRate) - fmt.Println() - fmt.Println("Latency Percentiles (Pull Time):") - fmt.Printf(" Min: %.3fs\n", r.Latencies.Min) - fmt.Printf(" P50: %.3fs\n", r.Latencies.P50) - fmt.Printf(" P90: %.3fs\n", r.Latencies.P90) - fmt.Printf(" P95: %.3fs\n", r.Latencies.P95) - fmt.Printf(" P99: %.3fs\n", r.Latencies.P99) - fmt.Printf(" Max: %.3fs\n", r.Latencies.Max) - fmt.Printf(" Mean: %.3fs\n", r.Latencies.Mean) - fmt.Println() + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, "=== E2E Test Results ===") + fmt.Fprintln(os.Stdout) + fmt.Fprintf(os.Stdout, "Total Iterations: %d\n", r.Summary.TotalIterations) + fmt.Fprintf(os.Stdout, "Success Count: %d\n", r.Summary.SuccessCount) + fmt.Fprintf(os.Stdout, "Failure Count: %d\n", r.Summary.FailureCount) + fmt.Fprintf(os.Stdout, "Eventual Successes: %d\n", r.Summary.EventualSuccessCount) + fmt.Fprintf(os.Stdout, "Success Rate: %.2f%%\n", r.Summary.SuccessRate) + fmt.Fprintln(os.Stdout) + fmt.Fprintln(os.Stdout, "Latency Percentiles (Pull Time):") + fmt.Fprintf(os.Stdout, " Min: %.3fs\n", r.Latencies.Min) + fmt.Fprintf(os.Stdout, " P50: %.3fs\n", r.Latencies.P50) + fmt.Fprintf(os.Stdout, " P90: %.3fs\n", r.Latencies.P90) + fmt.Fprintf(os.Stdout, " P95: %.3fs\n", r.Latencies.P95) + fmt.Fprintf(os.Stdout, " P99: %.3fs\n", r.Latencies.P99) + fmt.Fprintf(os.Stdout, " Max: %.3fs\n", r.Latencies.Max) + fmt.Fprintf(os.Stdout, " Mean: %.3fs\n", r.Latencies.Mean) + fmt.Fprintln(os.Stdout) } diff --git a/test/e2e/workload/scheduler.go b/test/e2e/workload/scheduler.go index 1e5415d..2d54d7b 100644 --- a/test/e2e/workload/scheduler.go +++ b/test/e2e/workload/scheduler.go @@ -17,10 +17,12 @@ type Scheduler struct { workers []*Worker } +const workerBufferMultiplier = 10 + // NewScheduler creates a new scheduler. func NewScheduler(fw *framework.Framework, logger *zap.Logger) *Scheduler { numWorkers := fw.Options().Workers - bufferSize := numWorkers * 10 + bufferSize := numWorkers * workerBufferMultiplier return &Scheduler{ framework: fw, @@ -42,7 +44,7 @@ func (s *Scheduler) Run(ctx context.Context) ([]WorkResult, error) { // Start workers var wg sync.WaitGroup - for i := 0; i < opts.Workers; i++ { + for i := range opts.Workers { s.workers[i] = NewWorker(i, s.framework, s.workChan, s.resultChan) wg.Add(1) go func(w *Worker) { @@ -54,7 +56,7 @@ func (s *Scheduler) Run(ctx context.Context) ([]WorkResult, error) { // Queue work items go func() { defer close(s.workChan) - for i := 0; i < opts.Iterations; i++ { + for i := range opts.Iterations { select { case <-ctx.Done(): return @@ -121,7 +123,7 @@ func (s *Scheduler) RunWithProgress(ctx context.Context, progressFn func(complet // Start workers var wg sync.WaitGroup - for i := 0; i < opts.Workers; i++ { + for i := range opts.Workers { s.workers[i] = NewWorker(i, s.framework, s.workChan, s.resultChan) wg.Add(1) go func(w *Worker) { @@ -133,7 +135,7 @@ func (s *Scheduler) RunWithProgress(ctx context.Context, progressFn func(complet // Queue work items go func() { defer close(s.workChan) - for i := 0; i < opts.Iterations; i++ { + for i := range opts.Iterations { select { case <-ctx.Done(): return diff --git a/test/e2e/workload/worker.go b/test/e2e/workload/worker.go index 386ff7f..5be38e4 100644 --- a/test/e2e/workload/worker.go +++ b/test/e2e/workload/worker.go @@ -20,19 +20,19 @@ type WorkItem struct { // WorkResult represents the result of a single work item. type WorkResult struct { - Iteration int `json:"iteration"` - ImageName string `json:"image_name"` - ImageRef string `json:"image_ref"` - Success bool `json:"success"` - Error string `json:"error,omitempty"` - Duration float64 `json:"duration_s"` - PullTime float64 `json:"pull_time_s"` - StartTime time.Time `json:"start_time"` - EndTime time.Time `json:"end_time"` - PodName string `json:"pod_name"` - WorkerID int `json:"worker_id"` - FailedPodEvents *PodEventRecord `json:"failed_pod_events,omitempty"` - EventualSuccessEvents *PodEventRecord `json:"eventual_success_events,omitempty"` + Iteration int `json:"iteration"` + ImageName string `json:"image_name"` + ImageRef string `json:"image_ref"` + Success bool `json:"success"` + Error string `json:"error,omitempty"` + Duration float64 `json:"duration_s"` + PullTime float64 `json:"pull_time_s"` + StartTime time.Time `json:"start_time"` + EndTime time.Time `json:"end_time"` + PodName string `json:"pod_name"` + WorkerID int `json:"worker_id"` + FailedPodEvents *PodEventRecord `json:"failed_pod_events,omitempty"` + EventualSuccessEvents *PodEventRecord `json:"eventual_success_events,omitempty"` } // Worker executes work items by creating pods that pull images through barnacle. @@ -75,6 +75,12 @@ func (w *Worker) Run(ctx context.Context) { } } +const ( + podNameModulo = 10000 + watcherTimeout = 5 * time.Second + cleanupTimeout = 30 * time.Second +) + func (w *Worker) processItem(ctx context.Context, item WorkItem) WorkResult { result := WorkResult{ Iteration: item.Iteration, @@ -84,7 +90,7 @@ func (w *Worker) processItem(ctx context.Context, item WorkItem) WorkResult { WorkerID: w.id, } - podName := fmt.Sprintf("e2e-pull-%d-%d", item.Iteration, time.Now().UnixNano()%10000) + podName := fmt.Sprintf("e2e-pull-%d-%d", item.Iteration, time.Now().UnixNano()%podNameModulo) result.PodName = podName // Create the pod @@ -120,7 +126,7 @@ func (w *Worker) processItem(ctx context.Context, item WorkItem) WorkResult { result.PullTime = pr.watchResult.Duration.Seconds() sawPullError = pr.watchResult.SawPullError } - case <-time.After(5 * time.Second): + case <-time.After(watcherTimeout): // Timed out waiting for watcher result; leave PullTime as zero } @@ -131,38 +137,17 @@ func (w *Worker) processItem(ctx context.Context, item WorkItem) WorkResult { if fetchErr == nil { result.FailedPodEvents = eventRecord } - w.cleanupPod(ctx, podName) + w.cleanupPod(podName) result.EndTime = time.Now() result.Duration = result.EndTime.Sub(result.StartTime).Seconds() return result } // Check pod status - if completedPod.Status.Phase == corev1.PodSucceeded { - result.Success = true - if sawPullError { - eventRecord, fetchErr := w.eventWatcher.FetchPodEvents(ctx, podName, item.ImageRef, "transient image pull error") - if fetchErr == nil { - result.EventualSuccessEvents = eventRecord - } - } - } else { - result.Error = fmt.Sprintf("pod failed with phase: %s", completedPod.Status.Phase) - if len(completedPod.Status.ContainerStatuses) > 0 { - cs := completedPod.Status.ContainerStatuses[0] - if cs.State.Terminated != nil && cs.State.Terminated.Message != "" { - result.Error = fmt.Sprintf("%s: %s", result.Error, cs.State.Terminated.Message) - } - } - // Fetch events for the failed pod - eventRecord, fetchErr := w.eventWatcher.FetchPodEvents(ctx, podName, item.ImageRef, result.Error) - if fetchErr == nil { - result.FailedPodEvents = eventRecord - } - } + w.recordPodStatus(ctx, &result, completedPod, podName, item.ImageRef, sawPullError) // Cleanup pod - w.cleanupPod(ctx, podName) + w.cleanupPod(podName) // Record timing for pod events if available if createdPod != nil { @@ -175,6 +160,37 @@ func (w *Worker) processItem(ctx context.Context, item WorkItem) WorkResult { return result } +func (w *Worker) recordPodStatus( + ctx context.Context, + result *WorkResult, + completedPod *corev1.Pod, + podName, imageRef string, + sawPullError bool, +) { + if completedPod.Status.Phase == corev1.PodSucceeded { + result.Success = true + if sawPullError { + eventRecord, fetchErr := w.eventWatcher.FetchPodEvents(ctx, podName, imageRef, "transient image pull error") + if fetchErr == nil { + result.EventualSuccessEvents = eventRecord + } + } + return + } + + result.Error = fmt.Sprintf("pod failed with phase: %s", completedPod.Status.Phase) + if len(completedPod.Status.ContainerStatuses) > 0 { + cs := completedPod.Status.ContainerStatuses[0] + if cs.State.Terminated != nil && cs.State.Terminated.Message != "" { + result.Error = fmt.Sprintf("%s: %s", result.Error, cs.State.Terminated.Message) + } + } + eventRecord, fetchErr := w.eventWatcher.FetchPodEvents(ctx, podName, imageRef, result.Error) + if fetchErr == nil { + result.FailedPodEvents = eventRecord + } +} + func (w *Worker) buildPod(name, imageRef string) *corev1.Pod { return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -199,10 +215,9 @@ func (w *Worker) buildPod(name, imageRef string) *corev1.Pod { } } -func (w *Worker) cleanupPod(ctx context.Context, name string) { +func (w *Worker) cleanupPod(name string) { if w.framework.Options().DeletePods { - // Use a separate context for cleanup to ensure it completes - cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + cleanupCtx, cancel := context.WithTimeout(context.Background(), cleanupTimeout) defer cancel() _ = w.framework.Cluster().DeletePod(cleanupCtx, name)