Skip to content

servicemesh: add make target for local testing #35169

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 8, 2024
Merged

servicemesh: add make target for local testing #35169

merged 1 commit into from
Oct 8, 2024

Conversation

marseel
Copy link
Contributor

@marseel marseel commented Oct 2, 2024

Currently, we didn't have an easy way to run ingress/gatewayapi locally to test it. Let's add new make kind-servicemesh-install-cilium target that enables both gatewayapi and ingress for easier development and testing locally.

Usage:

make kind && make kind-image && make kind-servicemesh-install-cilium

@marseel marseel added the release-note/misc This PR makes changes that have no direct user impact. label Oct 2, 2024
@marseel marseel requested review from a team and youngnick and removed request for a team October 2, 2024 09:20
@marseel
Copy link
Contributor Author

marseel commented Oct 2, 2024

/test

@marseel marseel marked this pull request as ready for review October 2, 2024 13:29
@marseel marseel requested review from a team as code owners October 2, 2024 13:29
@marseel marseel requested review from bimmlerd and borkmann October 2, 2024 13:29
bimmlerd
bimmlerd approved these changes Oct 3, 2024
View reviewed changes
Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

On a meta note, I think Makefile.kind and kind.sh has reached a level of complexity where it should probably be a Go program which allows you to spin up a kind cluster with whatever config you want 🤔

@marseel
Copy link
Contributor Author

marseel commented Oct 3, 2024

On a meta note, I think Makefile.kind and kind.sh has reached a level of complexity where it should probably be a Go program which allows you to spin up a kind cluster with whatever config you want 🤔

Or clusters for clustermesh 😅

I agree it would be nice to have something better than Makefile, I'm not sure if the Go program would be easier though.
Maybe, spinning up clusters + loading images as Makefile, and installing Cilium with whatever features as a bash script 🤔

@bimmlerd
Copy link
Member

bimmlerd commented Oct 3, 2024 •
edited
Loading

On a meta note, I think Makefile.kind and kind.sh has reached a level of complexity where it should probably be a Go program which allows you to spin up a kind cluster with whatever config you want 🤔

Or clusters for clustermesh 😅

I agree it would be nice to have something better than Makefile, I'm not sure if the Go program would be easier though. Maybe, spinning up clusters + loading images as Makefile, and installing Cilium with whatever features as a bash script 🤔

I've just spent some time thinking about this with @glrf, we might come up with design doc at some point - for now, this is fine.

sayboras
sayboras approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

I have one comment as per below, the rest looks good.

Thanks a lot for fomalizing this local dev setup.

@marseel marseel force-pushed the pr/marseel/local_servicemesh branch from d273753 to 57d86c1 Compare October 4, 2024 11:34
@marseel
Copy link
Contributor Author

marseel commented Oct 4, 2024

/test

@joestringer
Copy link
Member

On a meta note, I think Makefile.kind and kind.sh has reached a level of complexity where it should probably be a Go program which allows you to spin up a kind cluster with whatever config you want

Whatever tooling we think is broadly useful to different contributors we can integrate, but personally I find that the more we diverge from make, make build, make test into custom tooling, the harder it is to understand what the different tools do. I like that all I need to know is that you start by running make help and then go from there. Even for a lot of the Go tooling, I forget the various extra arguments we need to pass to filter the right directories to test or run a benchmark. We can combat this with good documentation of course. Maybe what I'm saying is "yes, and": As long as we aren't compromising the simple "make" workflows then by all means we can create tailored tooling for this.

joestringer
joestringer approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

LGTM for build. Couple of minor optimizations/suggestions.

@marseel
servicemesh: add make target for local testing
26e7576 
Currently, we didn't have an easy way to run ingress/gatewayapi locally
to test it. Let's add new `make kind-servicemesh-install-cilium` target
that enables both gatewayapi and ingress for easier development and
testing locally.

Usage:
make kind && make kind-image && make kind-servicemesh-install-cilium

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel force-pushed the pr/marseel/local_servicemesh branch from 57d86c1 to 26e7576 Compare October 7, 2024 09:03
@marseel
Copy link
Contributor Author

marseel commented Oct 7, 2024

/test

@marseel marseel added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 8, 2024
@marseel
Copy link
Contributor Author

marseel commented Oct 8, 2024

I marked it as ready-to-merge, no idea why maintainer's little helper didn't kick in.
I have a review from build, contributing and also servicemesh teams.

@sayboras sayboras added this pull request to the merge queue Oct 8, 2024
Merged via the queue into cilium:main with commit d49bec4 Oct 8, 2024
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@bimmlerd bimmlerd bimmlerd approved these changes

@sayboras sayboras sayboras approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marseel @bimmlerd @joestringer @sayboras