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 script for setting up k3d cluster for local development #2390

Merged
merged 5 commits into from
May 6, 2022

Conversation

MorpheusXAUT
Copy link
Contributor

This PR adds a bash script and Make target for setting up a basic k3d cluster with Flyte's dependencies for local development using the newly introduced single binary.

Whilst working on my latest changes, I've had to work on flyteadmin and flytepropeller simultaneously, which turned out to be a bit tedious using the default sandbox setup, especially building/pushing a new flyteadmin image to the sandbox cluster on every change to test.
Using the new single binary, I was able to develop much faster locally, however still had to run the dependencies for Flyte separately (mainly minio/postgres).

I've opted to setting up a basic k3d cluster with only Flyte's basic dependencies running (flyte single binary is built/run locally to allow for easier debugging). To make setup easier for my colleagues and me, I've put together a script for setting up all requirements for running and starting the cluster as well as the required containers (using the flyte-deps helm chart of this repository).
A separate config file for accessing local k3d resources has been added as well. In theory, this could also be gitignored and sed-replaced from the flyte_local.yaml file.

I figured I'd offer this script/setup for critique here to see if there's better ways of achieving the same goal (running all Flyte components locally via the single binary). I'm (somewhat 😄 ) aware of the sandbox-lite Dockerfile, however it seems to me that would still require image rebuilds/pushes to test changes.

Configures basic k3d cluster running flyte-deps helm chart to supply locally running flyte installation with required dependencies
Includes setup of required k3d/kubectl/helm binaries for easy start (configurable via env variables)

Signed-off-by: Nick Müller <[email protected]>
@welcome
Copy link

welcome bot commented Apr 21, 2022

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@hamersaw hamersaw requested review from pingsutw, hamersaw and EngHabu May 2, 2022 14:35
@hamersaw
Copy link
Contributor

hamersaw commented May 2, 2022

I really like this idea, will test out over the next week or so! The initial development experience is quite rough and certainly hinders onboarding new developers. I think this is a great first step, just want to make sure it's in the right direction. It's easier to get things right now than relearn in the future and cleanup the bloat that comes with committing a changing dev env continually.

@pingsutw wondering if you had any input here? When implementing single binary what did your workflow look like?
@katrogan are there any flyteadmin issues you see with this type of dev environment?

Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

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

I love the idea too, we need a basic container that only has Postgres and minio. it can make local development much easier.

In local development, I used to start a new sandbox cluster and use a single binary to start another flyte cluster locally with the config flyte_local.yaml, but it always required a large amount of resources.

btw, I just tested this script, and it works for me, thanks!!

Copy link
Contributor

@hamersaw hamersaw left a comment

Choose a reason for hiding this comment

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

A few comments to keep this from going stale.

Signed-off-by: Nick Müller <[email protected]>
@MorpheusXAUT
Copy link
Contributor Author

We could also move the k3d config from the script/command line params to a config file, which would allow for easier customization, but would drop just another config file into the repo somewhere...

One thing I just realized is that this script does not include a flytectl setup/config for the k3d cluster. Is there an easy way to have that generated without adding yet another template config to the repo? 😅 From what I can tell, we can flytectl config init and set the host, but cannot configure e.g. storage to point to minio.
Should we generate a flytectl config for this dev cluster as well in this script? If so, I guess the only option would be to add a default config file we can copy to the appropriate dir/set FLYTECTL_CONFIG...

@hamersaw
Copy link
Contributor

hamersaw commented May 5, 2022

We could also move the k3d config from the script/command line params to a config file, which would allow for easier customization, but would drop just another config file into the repo somewhere...

One thing I just realized is that this script does not include a flytectl setup/config for the k3d cluster. Is there an easy way to have that generated without adding yet another template config to the repo? sweat_smile From what I can tell, we can flytectl config init and set the host, but cannot configure e.g. storage to point to minio. Should we generate a flytectl config for this dev cluster as well in this script? If so, I guess the only option would be to add a default config file we can copy to the appropriate dir/set FLYTECTL_CONFIG...

As far as I know, there is not an easier way. I prefer to keep the number of dev config files checked in to a minimum, but that's something that we can always easily add later.

Moving forward it doesn't sound like there is not any strong opposition here. With the new single binary using k3d with the flyte-deps helm chart seems to provide a good initial development environment in terms of ease of setup, iteration speed, etc. There are a number of things that I ran into when setting this up, like manually copying the app/dist directory from flyteconsole to compile the single binary and manually creating the flytesnacks-* namespaces (I don't believe single binary handles this). However, these are things that can be added / modified later.

How do you feel about merging this and then we (I'm happy to spearhead it if necessary) can get an initial document to outline using this to set up a dev environment? It's a great start, but we can't expect to get everything 100% right away or this PR will be open for months. I think then we can easily tweak little things (ex. flytectl config, etc) as we iterate.

@MorpheusXAUT
Copy link
Contributor Author

As far as I know, there is not an easier way. I prefer to keep the number of dev config files checked in to a minimum, but that's something that we can always easily add later.

Agreed.

There are a number of things that I ran into when setting this up, like manually copying the app/dist directory from flyteconsole to compile the single binary and manually creating the flytesnacks-* namespaces (I don't believe single binary handles this). However, these are things that can be added / modified later.

True, that's also something I ran into, but figured that's something we can add at a later time since we'll have to figure a way to ensure the frontend repo is either available or grab the build from somewhere else.
The namespaces not being registered is something I noticed as well, can be added to the script as well, although I somewhat expected that the single binary would handle this instead.

How do you feel about merging this and then we (I'm happy to spearhead it if necessary) can get an initial document to outline using this to set up a dev environment? It's a great start, but we can't expect to get everything 100% right away or this PR will be open for months. I think then we can easily tweak little things (ex. flytectl config, etc) as we iterate.

Sounds like a good plan to me, I'm happy to continue improving the dev environment tooling as we continue along and help out with putting together some documentation for first steps, if you'd like.

@MorpheusXAUT
Copy link
Contributor Author

MorpheusXAUT commented May 5, 2022

I've added one last commit, updating the default version to v5.4.1 as per suggestion, seems to be working fine for me. I believe we could merge this as it is and continue iterating/improving on it 🙂

@hamersaw hamersaw self-requested a review May 5, 2022 16:06
@hamersaw hamersaw merged commit be57ba1 into flyteorg:master May 6, 2022
@welcome
Copy link

welcome bot commented May 6, 2022

Congrats on merging your first pull request! 🎉

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.

3 participants