feat: add ability to configure platforms of the deployed images #1603
Conversation
cenk1cenk2
changed the title
cenk1cenk2
force-pushed
the
patch-1
branch
2 times, most recently
from
c09d957 to
65de196
Compare
cenk1cenk2
changed the title
cenk1cenk2
changed the title
cenk1cenk2
force-pushed
the
patch-1
branch
3 times, most recently
from
2450fca to
7f39af3
Compare
WIP: WIP: WIP: docs: update docu fix: udpate script with tee WIP: use iptables legacy
There was a problem hiding this comment.
Pull Request Overview
This PR adds the ability to configure platforms for deployed images in k3d clusters. It allows users to specify container platforms (like linux/arm64) for nodes in both cluster creation and node creation commands.
- Adds
runtime-platformflag tocluster createandnode createcommands - Introduces platform configuration in config file format under
options.runtime.platforms - Updates Docker container creation and image pulling to support platform specifications
Reviewed Changes
Copilot reviewed 13 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/types/types.go | Adds Platform field to Node struct for platform specification |
| pkg/runtimes/docker/types.go | Adds PlatformConfig field to NodeInDocker struct |
| pkg/runtimes/docker/translate.go | Implements platform parsing and validation in node translation |
| pkg/runtimes/docker/container.go | Updates container creation and image pulling to support platform |
| pkg/config/v1alpha5/types.go | Adds PlatformWithNodeFilters type for config file support |
| pkg/config/transform.go | Implements platform configuration transformation for node filtering |
| pkg/client/node.go | Updates node creation to handle platform-specific directory checks |
| go.mod | Adds containerd/platforms dependency and promotes opencontainers/image-spec |
| docs/usage/configfile.md | Documents new platform configuration in config file |
| cmd/node/nodeCreate.go | Adds runtime-platform flag to node create command |
| cmd/cluster/clusterCreate.go | Adds runtime-platform flag to cluster create command with node filtering |
| pkg/types/fixes/assets/ | Updates entrypoint scripts (unrelated to platform feature) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Cmd: cmd, | ||
| Tty: false, | ||
| Entrypoint: []string{}, | ||
| }, nil, nil, nil, "") |
There was a problem hiding this comment.
The container creation in executeCheckInContainer doesn't use the platform parameter. The platform should be passed as the 4th argument to ContainerCreate to ensure the correct platform is used when creating the temporary container.
| }, nil, nil, nil, "") | |
| }, nil, nil, platform, "") |
Copilot uses AI. Check for mistakes.
| // -> RUNTIME PLATFORMS | ||
| for _, runtimePlatformWithNodeFilters := range simpleConfig.Options.Runtime.Platforms { | ||
| if len(runtimePlatformWithNodeFilters.NodeFilters) == 0 && nodeCount > 1 { | ||
| return nil, fmt.Errorf("RuntimePlatformmapping '%s' lacks a node filter, but there's more than one node", runtimePlatformWithNodeFilters.Platform) |
There was a problem hiding this comment.
There's a spelling error in the error message. 'RuntimePlatformmapping' should be 'Runtime platform mapping' with proper spacing.
| return nil, fmt.Errorf("RuntimePlatformmapping '%s' lacks a node filter, but there's more than one node", runtimePlatformWithNodeFilters.Platform) | |
| return nil, fmt.Errorf("Runtime platform mapping '%s' lacks a node filter, but there's more than one node", runtimePlatformWithNodeFilters.Platform) |
Copilot uses AI. Check for mistakes.
| "$entrypoint" >> "$LOGFILE" 2>&1 || exit 1 | ||
| for entrypoint in /bin/k3d-entrypoint-*.sh; do | ||
| echo "[$(date -Iseconds)] Running $entrypoint" | tee -a "$LOGFILE" | ||
| eval "$entrypoint" 2>&1 | tee "$LOGFILE" || exit 1 |
There was a problem hiding this comment.
Using eval on the entrypoint script path is unsafe and unnecessary. This should be a direct execution like the original code: \"$entrypoint\" 2>&1 | tee -a \"$LOGFILE\" || exit 1
| eval "$entrypoint" 2>&1 | tee "$LOGFILE" || exit 1 | |
| "$entrypoint" 2>&1 | tee -a "$LOGFILE" || exit 1 |
Copilot uses AI. Check for mistakes.
1 participant
What
runtime-platformtocluster createandnode createcommands where the user can designate the platform in docker format likelinux/arm64to pass to the docker go client.options.runtime.platformswith a node selector where the user can select the platforms of the matching nodes.Why
This PR is for the issue #1602.
Implications
v1alpha5.Notes
This is a draft implementation for request for comments.