feat(local-setup): use cloud-provider-kind for local-setup#973
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces MetalLB with cloud-provider-kind for LoadBalancer support, adds configurable ChangesMetalLB to cloud-provider-kind migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@make/dependencies.mk`:
- Around line 19-22: Replace the broad pkill-based management with PID-file
based management: stop using `pkill -f cloud-provider-kind` and instead check a
dedicated PID file (e.g., /tmp/cloud-provider-kind.pid), read the PID, verify
the process is `cloud-provider-kind` (or alive), send a targeted TERM/kill to
that PID, and remove the PID file if the process is gone; when starting
`cloud-provider-kind` redirect logs to /tmp/cloud-provider-kind.log, run it in
background and write its PID ($!) to /tmp/cloud-provider-kind.pid; also update
the corresponding stop/cleanup block (the other occurrence around the same rule)
to use the same PID-file logic and handle stale PID files safely.
In `@make/kind.mk`:
- Line 7: The Make target currently masks all failures by appending `|| echo
"Cluster already exists"` to the `KIND_EXPERIMENTAL_PROVIDER=$(CONTAINER_ENGINE)
kind create cluster --name $(KIND_CLUSTER_NAME)` command; instead, change the
logic to detect whether the cluster already exists and only skip creation in
that specific case (for example, run `kind get clusters` and check for
$(KIND_CLUSTER_NAME)) and otherwise run the `kind create cluster` command and
let it fail on real errors; update the invocation that uses
KIND_EXPERIMENTAL_PROVIDER/CONTAINER_ENGINE and KIND_CLUSTER_NAME so that
unexpected failures are not swallowed and the make target returns a non-zero
exit on real errors.
In `@README.md`:
- Around line 45-50: The blockquote containing the Docker/Podman guidance in
README.md has a blank line inside the quoted block (causing MD028); remove the
empty line so every quoted line begins with '>' consecutively (i.e., merge the
two blockquote paragraphs into a single continuous blockquote) and ensure the
fenced code block remains inside that blockquote without an intervening blank
'>'-less line.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27fa4e05-c002-4b41-819f-565d07c05a24
📒 Files selected for processing (6)
README.mdmake/dependencies.mkmake/kind.mkmake/local-setup.mkmake/vars.mktestsuite/gateway/envoy/__init__.py
817c7a2 to
c97a55f
Compare
c97a55f to
a7d2a19
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
45-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the blank blockquote line to satisfy MD028.
Line 46 breaks the blockquote sequence and triggers markdownlint (
no-blanks-blockquote). Keep the blockquote continuous.Proposed fix
> **Docker users:** Set `CONTAINER_ENGINE=docker` when running `make local-setup`. Podman is used by default. - > **macOS container runtime:** This setup has been tested with [Podman Desktop](https://podman-desktop.io/). Other Docker-compatible runtimes (OrbStack, Docker Desktop, Rancher Desktop) may not work because `cloud-provider-kind` uses `docker ps` internally to discover Kind containers — if the Docker CLI points to a different runtime, it won't find the Podman-based Kind cluster.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 45 - 47, Remove the empty blank line that breaks the blockquote so the two consecutive quoted lines remain a single continuous blockquote; specifically, edit the README so the line containing "**macOS container runtime:** This setup..." immediately follows the preceding "**Docker users:** ..." blockquote line without an intervening blank line to satisfy MD028 (no-blanks-blockquote).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@README.md`:
- Around line 45-47: Remove the empty blank line that breaks the blockquote so
the two consecutive quoted lines remain a single continuous blockquote;
specifically, edit the README so the line containing "**macOS container
runtime:** This setup..." immediately follows the preceding "**Docker users:**
..." blockquote line without an intervening blank line to satisfy MD028
(no-blanks-blockquote).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec591941-1eb1-4b78-8378-848e3be8f91f
📒 Files selected for processing (6)
README.mdmake/dependencies.mkmake/kind.mkmake/local-setup.mkmake/vars.mktestsuite/gateway/envoy/__init__.py
🚧 Files skipped from review as they are similar to previous changes (4)
- make/local-setup.mk
- testsuite/gateway/envoy/init.py
- make/vars.mk
- make/dependencies.mk
a7d2a19 to
f289bcf
Compare
f289bcf to
df8d26b
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
45-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMarkdown blockquote formatting may still need attention.
The static analysis tool continues to flag MD028 (blank line inside blockquote) at line 46, despite a past review marking this as addressed. If line 46 contains a
>prefix (making it a blank line inside the blockquote), the two blockquote paragraphs should either be merged by removing line 46 entirely, or split into separate blockquotes by removing the>prefix from line 46.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 45 - 47, MD028 is triggered by a blank line that still has a blockquote prefix between the two quoted paragraphs; fix by either merging the paragraphs into a single blockquote (remove the extraneous blank line with the leading ">" so "**Docker users:** Set `CONTAINER_ENGINE=docker`..." and "**macOS container runtime:** This setup..." are consecutive lines in the same blockquote) or split them into separate blockquotes (remove the ">" prefix from the blank line so the blank line is not inside a blockquote), ensuring the Markdown now has no empty ">"-prefixed line between those two blockquote paragraphs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@README.md`:
- Around line 45-47: MD028 is triggered by a blank line that still has a
blockquote prefix between the two quoted paragraphs; fix by either merging the
paragraphs into a single blockquote (remove the extraneous blank line with the
leading ">" so "**Docker users:** Set `CONTAINER_ENGINE=docker`..." and "**macOS
container runtime:** This setup..." are consecutive lines in the same
blockquote) or split them into separate blockquotes (remove the ">" prefix from
the blank line so the blank line is not inside a blockquote), ensuring the
Markdown now has no empty ">"-prefixed line between those two blockquote
paragraphs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58573817-8b21-4789-ad2d-6f0468359462
📒 Files selected for processing (6)
README.mdmake/dependencies.mkmake/kind.mkmake/local-setup.mkmake/vars.mktestsuite/gateway/envoy/__init__.py
🚧 Files skipped from review as they are similar to previous changes (4)
- make/vars.mk
- make/local-setup.mk
- make/kind.mk
- make/dependencies.mk
df8d26b to
4b69e56
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
46-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove blank line inside blockquote to fix MD028.
The blank line between the two blockquote paragraphs triggers a Markdown linting warning (MD028). Remove it to create a continuous blockquote block.
📝 Proposed fix
> **Podman users:** Set `CONTAINER_ENGINE=podman` when running `make local-setup`. Docker is used by default. - > **macOS container runtime:** Use [Podman Desktop](https://podman-desktop.io/) or [Docker Desktop](https://www.docker.com/products/docker-desktop/) as the container runtime. Other Docker-compatible runtimes (e.g. OrbStack) may not work due to LoadBalancer networking incompatibilities with `cloud-provider-kind`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 46, Remove the empty blank line inside the blockquote so MD028 no longer triggers: edit the README block that contains the "<details><summary>📝 Proposed fix</summary>" section and delete the blank line separating the two blockquote paragraphs so the blockquote becomes continuous (merge the paragraphs into the same blockquote block).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@make/vars.mk`:
- Line 6: The default container runtime is still set to Docker via the
CONTAINER_ENGINE variable; change its default to podman by updating the
CONTAINER_ENGINE assignment (CONTAINER_ENGINE ?= docker -> CONTAINER_ENGINE ?=
podman) so local setups default to Podman as intended; ensure any documentation
or scripts that rely on CONTAINER_ENGINE continue to work with the new default.
---
Duplicate comments:
In `@README.md`:
- Line 46: Remove the empty blank line inside the blockquote so MD028 no longer
triggers: edit the README block that contains the "<details><summary>📝 Proposed
fix</summary>" section and delete the blank line separating the two blockquote
paragraphs so the blockquote becomes continuous (merge the paragraphs into the
same blockquote block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c94eda5-6119-41e0-96be-ff8ea1e8ad3c
📒 Files selected for processing (6)
README.mdmake/dependencies.mkmake/kind.mkmake/local-setup.mkmake/vars.mktestsuite/gateway/envoy/__init__.py
🚧 Files skipped from review as they are similar to previous changes (3)
- make/kind.mk
- make/dependencies.mk
- make/local-setup.mk
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
4b69e56 to
5012a2a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
45-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the blank line inside the blockquote (MD028).
Line 46 breaks the blockquote block and triggers markdownlint
MD028.Suggested fix
> **Podman users:** Set `CONTAINER_ENGINE=podman` when running `make local-setup`. Docker is used by default. - > **macOS container runtime:** Use [Podman Desktop](https://podman-desktop.io/) or [Docker Desktop](https://www.docker.com/products/docker-desktop/) as the container runtime. Other Docker-compatible runtimes (e.g. OrbStack) may not work due to LoadBalancer networking incompatibilities with `cloud-provider-kind`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 45 - 47, The blockquote is broken by an empty line between the two quoted lines; remove the blank line so the blockquote remains continuous (i.e., join "**Podman users:** Set `CONTAINER_ENGINE=podman` when running `make local-setup`. Docker is used by default." and "**macOS container runtime:** Use Podman Desktop or Docker Desktop..." into the same blockquote without an intervening blank line) to satisfy markdownlint MD028.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@README.md`:
- Around line 45-47: The blockquote is broken by an empty line between the two
quoted lines; remove the blank line so the blockquote remains continuous (i.e.,
join "**Podman users:** Set `CONTAINER_ENGINE=podman` when running `make
local-setup`. Docker is used by default." and "**macOS container runtime:** Use
Podman Desktop or Docker Desktop..." into the same blockquote without an
intervening blank line) to satisfy markdownlint MD028.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac4db38d-ccac-429d-8e6f-4c8a6816b0a9
📒 Files selected for processing (6)
README.mdmake/dependencies.mkmake/kind.mkmake/local-setup.mkmake/vars.mktestsuite/gateway/envoy/__init__.py
🚧 Files skipped from review as they are similar to previous changes (5)
- make/vars.mk
- make/local-setup.mk
- make/kind.mk
- make/dependencies.mk
- testsuite/gateway/envoy/init.py
crstrn13
left a comment
There was a problem hiding this comment.
Tested on Fedora 43, works like a charm! 🎉 ![]()
Important
This PR modifies shared/core testsuite code that could potentially affect multiple test areas. 2 reviewers should review this PR to ensure adequate coverage.
Closes #957
Description
cloud-provider-kindfor LoadBalancer IP assignment in local Kind clustersChanges
Local Setup
install-metallbtarget withstart-cloud-providerandstop-cloud-providertargets inmake/dependencies.mkCONTAINER_ENGINEvariable (default:docker) tomake/vars.mkKIND_EXPERIMENTAL_PROVIDERtokind create/delete clustercommands inmake/kind.mkcloud-provider-kindonlocal-cleanupinmake/local-setup.mkMETALLB_VERSIONandMETALLB_TIMEOUTvariablesBug Fix
self.service.wait_for_ready()after Envoy LoadBalancer service creation intestsuite/gateway/envoy/__init__.py. Unlike MetalLB which assigns IPs synchronously,cloud-provider-kindassigns them asynchronously, sotests need to wait for the external IP before sending requests
Documentation
README.mdprerequisites to listcloud-provider-kindinstead of MetalLBVerification steps
Summary by CodeRabbit
Documentation
New Features
Chores
Bug Fixes