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

Use Stork registrars in standalone way #42161

Merged
merged 1 commit into from
Sep 22, 2024

Conversation

aureamunoz
Copy link
Member

@aureamunoz aureamunoz commented Jul 26, 2024

This requires a new Stork release. Will made it after holidays, from 19th August.

Copy link

quarkus-bot bot commented Jul 26, 2024

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with ellipsis (make sure the title is complete)

This message is automatically generated by a bot.

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/smallrye area/stork labels Jul 26, 2024
@aureamunoz aureamunoz changed the title Use latest Stork and refactor extension according to new registrars p… Use Stork registrars in standalone way Jul 26, 2024
@aureamunoz aureamunoz force-pushed the use-latest-stork-registration branch 2 times, most recently from 5fa7ebd to 084b4f0 Compare August 29, 2024 07:28
@aureamunoz aureamunoz marked this pull request as ready for review August 29, 2024 07:28
@aureamunoz
Copy link
Member Author

cc @cescoffier

@cescoffier
Copy link
Member

Can you mark the PR as draft to make sure we do not merge it too eagerly.

@aureamunoz aureamunoz marked this pull request as draft August 29, 2024 08:02
Copy link

github-actions bot commented Aug 29, 2024

🎊 PR Preview 8e6e43d has been successfully built and deployed to https://quarkus-pr-main-42161-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@melloware
Copy link
Contributor

melloware commented Sep 16, 2024

@cescoffier @aureamunoz i think this is what I need. Will this allow me to programatically add a Consult client. Right now I have a customer converting from Spring Boot and they don't like that they have to do this...

 /**
     * Register our two services in Consul.
     *
     * Note: this method is called on a worker thread, and so it is allowed to block.
     * @throws UnknownHostException 
     */
    public void init(@Observes StartupEvent ev, Vertx vertx) throws UnknownHostException {
        String consulHost = ConfigProvider.getConfig().getValue("quarkus.stork.my-service.service-discovery.consul-host", String.class);
        int consulPort = ConfigProvider.getConfig().getValue("quarkus.stork.my-service.service-discovery.consul-port", Integer.class);
        ConsulClient client = ConsulClient.create(vertx, new ConsulClientOptions().setHost(consulHost).setPort(consulPort));

        int port = ConfigProvider.getConfig().getValue("quarkus.stork.my-service.service-discovery.consul-port", Integer.class);
        String address = InetAddress.getLocalHost().getHostAddress();
        String name = ConfigProvider.getConfig().getValue("quarkus.application.name", String.class);
        client.registerService(
                new ServiceOptions().setPort(port).setAddress(address).setName(name).setId("greeting-service"));
    }

They don't want to write Java code in every service they just want to use configs to register their services?

@aureamunoz
Copy link
Member Author

Yes, this is what you need @melloware . With this feature you don't event need to instantiate the ConsulClient, stork will do it for you. You just will need to configure in properties the host and port of the Consul.
This will be only available in Quarkus 3.16 though.

Let me try to provide a quickstart.

@melloware
Copy link
Contributor

Darn not until 3.16? Also as part of this PR don't the docs need to be updated?

@aureamunoz
Copy link
Member Author

This PR uses Stork 2.7.0 which contains the documentation. I will also add a quickstart with it in the quickstarts repository. As an example, for now you can check the integration tests.

@melloware
Copy link
Contributor

@aureamunoz thanks but the IT still does this.

 Stork.getInstance().getService("my-service").getServiceRegistrar().registerServiceInstance("my-service", "localhost",
                red);

My client is looking for a ZERO code option so they can just do this.

quarkus.stork.my-service.service-registrar.type=consul

And it automatically registers the service by quarkus.application.name, quarkus.http.port etc so they don't have to code anything. Are you saying the above will be possible in Quarkus 3.16?

@aureamunoz
Copy link
Member Author

@aureamunoz thanks but the IT still does this.

 Stork.getInstance().getService("my-service").getServiceRegistrar().registerServiceInstance("my-service", "localhost",
                red);

My client is looking for a ZERO code option so they can just do this.

quarkus.stork.my-service.service-registrar.type=consul

And it automatically registers the service by quarkus.application.name, quarkus.http.port etc so they don't have to code anything. Are you saying the above will be possible in Quarkus 3.16?

Ah, I didn't get that. No, that is not possible at the moment. Still need to pass the host and port of the application to register. We had mention something like this here. I could do it but not sure to be able to work on that inmediately.

@melloware
Copy link
Contributor

OK they are coming from Spring Boot and Spring Boot does this: https://cloud.spring.io/spring-cloud-consul/reference/html/

The default service name, instance id and port, taken from the Environment, are ${spring.application.name}, the Spring Context ID and ${server.port} respectively.

I think Quarkus should definitely do something similar.

@aureamunoz aureamunoz marked this pull request as ready for review September 18, 2024 09:51
@aureamunoz
Copy link
Member Author

Ready for review @cescoffier

@cescoffier
Copy link
Member

@melloware this is heavily dependent on the environment. These days it's rare an application knows it's public IP and public port (typically, as soon as you run in a container you do not have this info anymore). So I would be very cautious about providing such a feature which may end up registering unreachable services.

This comment has been minimized.

Copy link

quarkus-bot bot commented Sep 18, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit be13dac.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Sep 18, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit be13dac.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@cescoffier cescoffier merged commit 656dd40 into quarkusio:main Sep 22, 2024
69 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/smallrye area/stork
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants