Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Add new management cluster environment option #1163

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yiannistri
Copy link
Contributor

@yiannistri yiannistri commented Mar 19, 2025

What this PR does / why we need it:

This PR adds a new option (internal-kind) for creating a local bootstrap cluster (using kind with extra port mappings) that accepts requests from the host. This can be useful in environments where the CAPI provider is running in the local network (eg. vSphere/CAPV). The new setup basically replicates this guide.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1162

Special notes for your reviewer:
I have added a new bootstrap cluster provider instead of updating existing code we have in place for creating a kind cluster since the port mappings settings were ignored when the docker socket extra mount was specified (useful for CAPD tests).

To test the setup of the bootstrap cluster run the following:

MANAGEMENT_CLUSTER_ENVIRONMENT="internal-kind" TAG=v0.0.1 GINKGO_LABEL_FILTER=vsphere make test-e2e

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@yiannistri yiannistri force-pushed the 1162-capv-tests branch 2 times, most recently from 61e89e4 to b2c5f31 Compare March 19, 2025 16:36
@yiannistri yiannistri marked this pull request as ready for review March 19, 2025 17:33
@yiannistri yiannistri requested a review from a team as a code owner March 19, 2025 17:33
@yiannistri yiannistri self-assigned this Mar 19, 2025
@anmazzotti
Copy link
Contributor

@yiannistri I think the extraMapping are ignored just because they are overwritten by the docker socket code.

Would it be possible to fix this (it's a bug in our test suite anyway) and apply your changes to the current kind environment?

One other question I have is what provides type: LoadBalancer service? Do we have anything installed in the kind cluster for LB?

@yiannistri
Copy link
Contributor Author

yiannistri commented Mar 20, 2025

@anmazzotti yes for sure, we can fix this upstream. I have added a new provider in the meantime only to get things working for both CAPD tests as well as the CAPV tests that will need the extra port mappings.

I will create an issue upstream and prepare a PR to address this.

Regarding your second question, I have added a new manifest as part of this PR that creates an nginx load balancer.

@yiannistri
Copy link
Contributor Author

One downside of this approach (using extra port mappings) is that if the previous kind cluster is still running on the machine, the creation of a new kind cluster will fail because the 443 port will still be allocated to the previous cluster. We can ensure that no kind clusters are running when we begin a new test run, to mitigate this.

Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for the PR! One question/suggestion inline:

ShortTestLabel = "short"
FullTestLabel = "full"
DontRunLabel = "dontrun"
LocalTestLabel = "local"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recall this was introduced by me in https://github.com/rancher/turtles/pull/187/files#diff-f41624bd6051a35a1694431afebcf5012b89a1cb9ac856464228f797065d34b1R125 while adding CAPV e2e tests but looking at the current code, that label and some more logic looks redundant to me. Do we want to cleanup those as part of the same PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress (8 max)
Development

Successfully merging this pull request may close these issues.

Add support for creating bootstrap clusters in an internal network
4 participants