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

dev: Make setup-single-cluster capable of setting up multiple clusters #2461

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

p-se
Copy link
Contributor

@p-se p-se commented May 27, 2024

By using different git worktrees with different environment variables for configuring the single-cluster environment, specifically the FLEET_E2E_CLUSTER variable, you can use ./dev/setup-single-cluster to start independent k3d clusters next to each other.

Please note that, at the time of creating a cluster, there is a possibility of a conflict still. While clusters can be run at the same time with dev/setup-single-cluster, they cannot be created simultaneously (yet).

  • Perhaps add a entry in dev/README.

@p-se p-se requested a review from a team as a code owner May 27, 2024 11:16
@p-se p-se self-assigned this May 27, 2024
@p-se p-se force-pushed the dev-parallel-single-cluster-instances branch from d5dd329 to b804c2f Compare May 28, 2024 13:11
dev/build-fleet Outdated
Comment on lines 30 to 31
# conflicts when the clusters are created simultaneously. It might be worth thinking about
# how to make the image name unique for each test run.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a use case I tend to avoid locally for performance reasons, but this may be useful to others.

Would something like this be more accurate?

Suggested change
# conflicts when the clusters are created simultaneously. It might be worth thinking about
# how to make the image name unique for each test run.
# conflicts when multiple Fleet builds are meant to run simultaneously. It might be
# worth thinking about how to make the image name unique for each test run.

My understanding is that clusters don't need to be created simultaneously, but rather used simultaneously, where an image meant to be used in one could end up being imported in another, just because the same image name and tag are used across all clusters.

As to ways to make image names unique, I suppose we could add an option to override the default dev tag on built Fleet images, eg. with the currently checked out branch, which could prevent conflicts: when using git worktrees, a given branch may only be checked out by a single worktree at a time.

Happy to discuss.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank for taking the time to explain to me why this was confusing! Please let me know if the updated phrasing (and position of the comment) is not a sufficient improvement!

@p-se p-se force-pushed the dev-parallel-single-cluster-instances branch from b804c2f to 575afd0 Compare June 25, 2024 14:12
dev/build-fleet Outdated
@@ -18,6 +18,14 @@ fi

export GOOS=linux

# The name of the container image created here is a potential source of conflict
# when fleet is set up simultaneously, as several scripts are calling
Copy link
Contributor

Choose a reason for hiding this comment

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

This simultaneously alone may leave the reader wondering what simultaneity we're talking about here.

Suggested change
# when fleet is set up simultaneously, as several scripts are calling
# when fleet is set up simultaneously in multiple sets (ie 1 upstream +
# 0 to n downstream) of clusters, as several scripts are calling

dev/build-fleet Outdated
Comment on lines 25 to 27
# be avoided by making sure this script is not called sequentially, e.g. by
# creating multiple clusters on a single host sequentially rather than
# simultaneously.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what calling a script "sequentially" means.
What would you think of something like this? How does it compare to what you have/had in mind?

Suggested change
# be avoided by making sure this script is not called sequentially, e.g. by
# creating multiple clusters on a single host sequentially rather than
# simultaneously.
# be avoided by making sure that images `rancher/fleet:dev` and
# `rancher/fleet-agent:dev` are rebuilt from the right context (eg. git clone,
# checkout or worktree) before being imported into clusters created for testing
# that context.
# This can be easily achieved by ensuring that, for instance, at most one instance
# of `./dev/setup-single-cluster` or `./dev/setup-multi-cluster` runs at any given
# point in time.

@p-se p-se force-pushed the dev-parallel-single-cluster-instances branch from 575afd0 to 2a282fd Compare June 26, 2024 07:33
weyfonk
weyfonk previously approved these changes Jun 26, 2024
p-se added 3 commits June 26, 2024 13:09
In particular, respect FLEET_E2E_CLUSTER and
FLEET_E2E_CLUSTER_DOWNSTREAM.
This enables to run several k3d clusters with the
dev/setup-single-cluster script at the same time.
This will prevent that a given name, for instance `k3d-upstream`, will
cause container images named `k3d-upstream-2-server-x` to be deleted.
Instead, only container images with `k3d-upstream-{server,agent}-x` will
be targeted for deletion.
@p-se p-se force-pushed the dev-parallel-single-cluster-instances branch from 2a282fd to e81f1e2 Compare June 26, 2024 11:09
@p-se p-se merged commit c544688 into rancher:main Jun 27, 2024
8 checks passed
@p-se p-se deleted the dev-parallel-single-cluster-instances branch June 27, 2024 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants