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

Update to Stork 1.4.1 #30272

Merged
merged 1 commit into from
Jan 10, 2023
Merged

Update to Stork 1.4.1 #30272

merged 1 commit into from
Jan 10, 2023

Conversation

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 10, 2023

Failing Jobs - Building b3823a0

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 18 Build ⚠️ Check → Logs Raw logs

@geoand geoand merged commit 3f919e6 into quarkusio:main Jan 10, 2023
@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Jan 10, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jan 10, 2023

Milestone is already set for some of the items:

We haven't automatically updated the milestones for these items.

This message is automatically generated by a bot.

@edeandrea
Copy link
Contributor

@cescoffier I see this was merged but I don't see any updated documentation, either on the Quarkus stork guide nor the smallrye stork docs on how to actually use this?

@cescoffier
Copy link
Member Author

@edeandrea this is weird, the documentation is there: https://github.com/smallrye/smallrye-stork/blob/main/docs/docs/service-discovery/knative.md

But, it looks like something went wrong with the web site generation during the release.

@edeandrea
Copy link
Contributor

Yeah I don't see any docs for 1.4.1

@cescoffier
Copy link
Member Author

@edeandrea i've found the issue. I opened the release PR from my fork so it updated the wrong remote.

I've triggered another redeploy, but, Murphy's law applied... GitHub actions are down.

@cescoffier
Copy link
Member Author

@edeandrea Finally! https://smallrye.io/smallrye-stork/1.4.1/service-discovery/knative/#caching-the-service-instances

Sorry about that!

@edeandrea
Copy link
Contributor

Thanks! Once 2.16 ships I'll verify that it fixes quarkusio/quarkus-super-heroes#146

@edeandrea
Copy link
Contributor

Hey @cescoffier circling back to this.

When reading https://smallrye.io/smallrye-stork/1.4.1/service-discovery/knative/#caching-the-service-instances I see this property: quarkus.stork.my-knservice.service-discovery.knative-namespace

Is that property required? If left blank will it assume the same namespace as the app?

If its required, then this solution doesn't solve the original problem discussed in quarkusio/quarkus-super-heroes#146

@cescoffier
Copy link
Member Author

@aureamunoz ^

@edeandrea
Copy link
Contributor

edeandrea commented Feb 6, 2023

@cescoffier additionally, how do I tell the stork knative client what the name of the knative service is?

the docs say

quarkus.stork.my-knservice.service-discovery.type=knative

Stork looks for the Knative Service with the given name (my-knservice in the previous example) in the specified namespace.

But in my case my jaxrs client has @RegisterRestClient(configKey = "hero-client")

So how do I bind hero-client to the knative service name of rest-heroes? Seems there is something missing in the configuration? This configuration seems to assume that the client name in my app has the same name as the knative service name?

For example, with the kubernetes service discovery, I would put quarkus.stork.hero-service.service-discovery.application: rest-heroes

I don't think having quarkus.stork.my-knservice is correct? It isn't consistent with any of the other service discovery configs.

Other service discovery configs are in the format quarkus.stork.my-service.service-discovery.....

@edeandrea
Copy link
Contributor

edeandrea commented Feb 6, 2023

I would think I would want something like this (which I have verified does not work):

  quarkus.stork.hero-service.service-discovery.type: knative
  quarkus.stork.hero-service.service-discovery.application: rest-heroes
  quarkus.stork.villain-service.service-discovery.type: knative
  quarkus.stork.villain-service.service-discovery.application: rest-villains

This would say that the hero-service client used knative service discovery and was looking for the rest-heroes knative service within the same namespace as the current app.

Similar with the villain-service binding to the rest-villains knative service within the same namespace.

There may be situations where a single app is using multiple service discovery mechanisms, one per client. Say, for example, with the superheroes, the hero service is a knative app, but the villain service is a regular app deployed via a k8s Deployment. I'd want to bind the hero-service via the knative discovery type and point to the rest-heroes knative service, but then bind the villain-service via the kubernetes discovery type and point to the rest-villains kubernetes service.

@aureamunoz
Copy link
Member

Is that property required? If left blank will it assume the same namespace as the app?

The quarkus.stork.my-knservice.service-discovery.knative-namespace property is not required. If blank, the namespace used is the one from the kubernetes client configuration.

@edeandrea
Copy link
Contributor

@aureamunoz see my other comments in #30272 (comment) & #30272 (comment)

How do I bind the knative service discovery to a particular rest client within my app? Happy to chat offline if I've confused you :)

@aureamunoz
Copy link
Member

I would think I would want something like this:

quarkus.stork.hero-service.service-discovery.type: knative
quarkus.stork.hero-service.service-discovery.application: rest-heroes
quarkus.stork.villain-service.service-discovery.type: knative
quarkus.stork.villain-service.service-discovery.application: rest-villains

This would say that the hero-service client used knative service discovery and was looking for the rest-heroes knative > service within the same namespace as the current app

Similar with the villain-service binding to the rest-villains knative service within the same namespace.

There may be situations where a single app is using multiple service discovery mechanisms per client. Say, for
example, with the superheroes, the hero service is a knative app, but the villain service is a regular app deployed via a > k8s Deployment. I'd want to bind the hero-service via the knative discovery type, but then bind the villain-service via > the kubernetes discovery type.

It works exactly like described here. We have an issue displaying the documentation (I will open an issue for it). The following configuration properties are available for knative service discovery:

Attribute Mandatory Default Value Description
knative-host No The Knative API host.
knative-namespace No The namespace of the service. Use all to discover all namespaces.
application No The Knative application Id; if not defined Stork service name will be used.
refresh-period No 5M Service discovery cache refresh period.
secure No Whether the connection with the service should be encrypted with TLS.

@cescoffier
Copy link
Member Author

Stork web site fixed....

Do not ask what went wrong. I just retriggered the publication, and voilà....

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/stork
Projects
None yet
4 participants