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.

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

feat(hostnamegenerator): implement MeshExternalService support #10379

Merged
merged 15 commits into from
Jun 5, 2024
Merged

feat(hostnamegenerator): implement MeshExternalService support #10379

merged 15 commits into from
Jun 5, 2024

Conversation

lukidzi
Copy link
Contributor

@lukidzi lukidzi commented Jun 4, 2024 •
edited by slonka
Loading

xrel #10238

Checklist prior to review

  • extracted common part of the code
  • created separate implementation for MeshService and MeshExternalService
  • changed meshName generation to use resource name when K8sNameComponent/Universal
  • extracted common API structure to HostnameGenerator
  • Link to relevant issue as well as docs and UI issues --
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
  • Tests (Unit test, E2E tests, manual test on universal and k8s) --
    • Don't forget ci/ labels to run additional/fewer tests
  • Do you need to update UPGRADE.md? --
  • Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label) --

michaelbeaumont and others added 13 commits June 3, 2024 09:12
@michaelbeaumont
feat(HostnameGenerator): generate hostnames
d0c9c62 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
test(HostnameGenerator): add basic tests
d92a6ec 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
feat(HostnameGenerator): add validation
cf728c4 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
feat(HostnameGenerator): handle collisions
7a6ce85 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
test(HostnameGenerator): add collision test
b01d0be 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
Merge branch 'master' into feat/ms-hostname-generate
e4964ac
@michaelbeaumont
refactor: use IPAM allocation interval
7edcf4f 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
chore: imports and comment
eac2d61 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
feat: add service name to error
cb1e494 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@michaelbeaumont
refactor: don't track collisions between meshes
625edc4 
Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
@lukidzi
extract common model
db0c77f 
Signed-off-by: Lukasz Dziedziak <lukidzi@gmail.com>
@lukidzi
separated implementations for resources
283b7d9 
Signed-off-by: Lukasz Dziedziak <lukidzi@gmail.com>
@lukidzi
Merge branch 'master' into feat/mes-hostname-generate
5b976f0 
Signed-off-by: Lukasz Dziedziak <lukidzi@gmail.com>
@lukidzi
Copy link
Contributor Author

lukidzi commented Jun 4, 2024

/format

@lukidzi
fix
3cc5493 
Signed-off-by: Lukasz Dziedziak <lukidzi@gmail.com>
slonka
slonka reviewed Jun 5, 2024
View reviewed changes
Copy link
Contributor

@slonka slonka left a comment

Choose a reason for hiding this comment

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

just a couple of things, in general looks good

pkg/core/resources/apis/hostnamegenerator/api/v1alpha1/hostnamegenerator.go Outdated Show resolved Hide resolved
pkg/core/resources/apis/meshexternalservice/hostname/generator.go Outdated Show resolved Hide resolved
pkg/core/resources/apis/meshexternalservice/hostname/generator.go Outdated Show resolved Hide resolved
pkg/core/resources/apis/meshexternalservice/hostname/generator_test.go Outdated Show resolved Hide resolved
pkg/core/resources/apis/meshexternalservice/hostname/generator_test.go Show resolved Hide resolved
pkg/core/resources/apis/meshexternalservice/hostname/generator_test.go Outdated Show resolved Hide resolved
@slonka slonka marked this pull request as ready for review June 5, 2024 07:27
@slonka slonka requested a review from a team as a code owner June 5, 2024 07:27
@slonka slonka requested review from slonka, Automaat, lobkovilya, michaelbeaumont and jakubdyszkiewicz and removed request for a team June 5, 2024 07:27
@slonka
feat(hostnamegenerator): post review changes
e538f30 
Signed-off-by: slonka <slonka@users.noreply.github.com>
@lukidzi lukidzi merged commit 4d02c0b into kumahq:master Jun 5, 2024
15 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Jun 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelbeaumont michaelbeaumont michaelbeaumont left review comments

@slonka slonka slonka approved these changes

@Automaat Automaat Awaiting requested review from Automaat Automaat is a code owner automatically assigned from kumahq/kuma-maintainers

@lobkovilya lobkovilya Awaiting requested review from lobkovilya lobkovilya is a code owner automatically assigned from kumahq/kuma-maintainers

@jakubdyszkiewicz jakubdyszkiewicz Awaiting requested review from jakubdyszkiewicz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lukidzi @michaelbeaumont @slonka @jakubdyszkiewicz