make kind the standard - #1718
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/copilot-review |
|
Copilot code review has been requested by @UncivilizedGhost. Please allow a few moments for the review to be added. |
There was a problem hiding this comment.
Pull request overview
Removes Minikube support and standardizes Kind as the bootstrap cluster.
Changes:
- Defaults bootstrap clusters to Kind while retaining Tilt.
- Removes Minikube installation, networking, lifecycle, and Ironic paths.
- Updates related documentation and test workflows.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
vars.md |
Updates supported cluster options and IPv6 guidance. |
tests/roles/run_tests/tasks/move.yml |
Removes Minikube pivot handling. |
tests/roles/run_tests/tasks/move_back.yml |
Removes Minikube repivot handling. |
README.md |
Removes Minikube resource information. |
lib/network.sh |
Removes Minikube IPv6 addressing. |
lib/download.sh |
Removes Minikube and KVM2 downloads. |
lib/common.sh |
Makes Kind the default across runtimes. |
config_example.sh |
Documents Kind and Tilt options. |
cluster_cleanup.sh |
Removes Minikube cleanup. |
AGENTS.md |
Updates bootstrap-cluster guidance. |
04_verify.sh |
Removes Minikube-specific Ironic checks. |
03_launch_mgmt_cluster.sh |
Removes Minikube launch and Ironic branches. |
02_configure_host.sh |
Removes Minikube VM and networking setup. |
01_prepare_host.sh |
Removes Minikube installation handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| The Minikube machine is deployed with 4GB of RAM, and 2 vCPUs, and the target | ||
| hosts with 4 vCPUs and 4GB of RAM. | ||
| The target hosts are deployed with 4 vCPUs and 4GB of RAM. |
| - `IMAGE_OS` - Target OS: `ubuntu`, `centos`, `flatcar` (default: `ubuntu`) | ||
| - `CAPM3_VERSION`, `CAPI_VERSION` - Component versions | ||
| - `EPHEMERAL_CLUSTER` - Cluster type: `kind`, `minikube`, `tilt` | ||
| - `EPHEMERAL_CLUSTER` - Cluster type: `kind`, `tilt` |
f49b090 to
5db05fd
Compare
|
/test metal3-centos-e2e-integration-test-release-1-13 |
5db05fd to
8346616
Compare
|
/copilot-review |
|
Copilot code review has been requested by @UncivilizedGhost. Please allow a few moments for the review to be added. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (7)
03_launch_mgmt_cluster.sh:689
- These commands have the same rootless-Podman ownership issue: the Kind container was created as
${USER}, butsudo podman execlooks in root's container storage. This makes the IPv6 Kind path fail on the non-Ubuntu hosts that now default to Kind; execute through the current user's runtime instead.
sudo "${CONTAINER_RUNTIME}" exec kind-control-plane mkdir -p /etc/containerd/certs.d/"${REGISTRY}"
sudo "${CONTAINER_RUNTIME}" exec kind-control-plane cp /hosts.toml /etc/containerd/certs.d/"${REGISTRY}"/hosts.toml
lib/kind.sh:33
- On non-Ubuntu hosts,
configure_kind_networkalready assignsCLUSTER_BARE_METAL_PROVISIONER_IPto the host-sideironicendpoint(02_configure_host.sh:149-157). Assigning that same address here puts duplicate IPs on the same provisioning L2 when in-cluster Ironic is enabled. Make host configuration omit the cluster IP in this mode, or transfer/remove it before configuring the Kind interface.
sudo nsenter -t "${kind_pid}" -n ip addr add "${ip_with_cidr}" dev "${iface}"
AGENTS.md:57
- The documented variable name does not exist in the codebase; cluster selection is configured through
BOOTSTRAP_CLUSTER. KeepingEPHEMERAL_CLUSTERhere directs users and agents to set an ignored variable.
- `EPHEMERAL_CLUSTER` - Cluster type: `kind`, `tilt`
lib/kind.sh:26
- Kind is created as
${USER}, so on the non-Ubuntu Podman path its node is a rootless container. Inspecting it throughsudo podmanswitches to root's separate container storage and returns “no such container”, leaving the newly created veth pair behind and aborting setup. Run the runtime command as the same user that launched Kind.
This issue also appears on line 33 of the same file.
kind_pid=$(sudo "${CONTAINER_RUNTIME}" inspect -f '{{.State.Pid}}' "${container_name}")
03_launch_mgmt_cluster.sh:218
- The new in-cluster mode is not handled by the pivot flow.
move.ymlstill fetches logs from and removes local Ironic containers for every Kind cluster, so with this option enabled it fails on the first nonexistent container;move_back.ymlalso always restores local Ironic. Propagate this setting into the test role and branch both pivot directions between Kubernetes and local-container operations.
This issue also appears on line 688 of the same file.
if [[ "${IRONIC_DEPLOY_IN_CLUSTER}" == "true" ]]; then
# Deploy Ironic in-cluster using kustomize manifests
"${BMOPATH}/tools/deploy.sh" -i "${BMO_IRONIC_ARGS[@]}"
lib/common.sh:352
- This introduces a user-configurable deployment mode, but it is absent from both
vars.mdandconfig_example.sh. Users therefore have no documented way to discover its purpose, allowed values, default, or networking implications. Document the variable alongside the other Ironic options.
export IRONIC_DEPLOY_IN_CLUSTER="${IRONIC_DEPLOY_IN_CLUSTER:-false}"
vars.md:16
- The documentation cleanup is incomplete relative to the PR's stated removal of all Minikube support:
PoCs/README.md:54still describes moving an IP from Minikube, and line 78 instructs users to runminikube addons enable metallb, although this PR removes Minikube installation. Update those PoC instructions for Kind or clearly mark them as archival and unsupported.
| BOOTSTRAP_CLUSTER | Tool for running management/bootstrap cluster. | kind, tilt | "kind" |
5300af1 to
7266af0
Compare
|
/test metal3-centos-e2e-integration-test-release-1-13 |
|
/test metal3-centos-e2e-integration-test-release-1-13 |
|
@UncivilizedGhost: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Make kind the default cluster Delete any minikube downloading or installing functions Remove any ironic logic related to minikube Stop fetching minikube clusters from move.yml Remove minikube cleanup from cluster_cleanup Add option to deploy in cluster with kind and function `connect_kind_to_provisioning_network` which creates a virtual Ethernet connection to connect ironic in a kind node to the provisioning network. Modify all documentation Signed-off-by: Saad Zia <saad.zia@est.tech>
7266af0 to
b6808a4
Compare
Make kind the default cluster
Add option to deploy ironic in cluster with kind and function
connect_kind_to_provisioning_networkwhich creates a virtual Ethernet connection to connect the kind node to ironic.Modify some documentation