-
Notifications
You must be signed in to change notification settings - Fork 93
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
WildFly mini series: REST Client #671
base: main
Are you sure you want to change the base?
WildFly mini series: REST Client #671
Conversation
f9811e8
to
3bbebb8
Compare
@bstansberry can you assign a reviewer for this one? Also wildfly-extras/guides#4 would need a reviewer ... |
As this involves REST Client, I think @jamezp might want to review |
I'll try to have a look this week or early next week. I've got to get a RESTEasy release out, so once that's done I should have some time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had to stop the process of checking this on the second part as it's not working for me.
@@ -3,13 +3,14 @@ | |||
:jaxrs-example-project-artifactId: jaxrs | |||
:jaxrs-example-project-version: 11.0.0.Final-SNAPSHOT | |||
:version-bootable-jar: 11.0.0.Beta1 | |||
:version-wildfly: 32.0.0.Final |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we using WildFly 32? Shouldn't we use WildFly 34?
|
||
@RegisterRestClient(configKey="simple-microservice-server") | ||
@Path("/hello") | ||
public interface GettingStartedEndpointInterface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should name this something like GettingStartedClient
or GettingStartedEndpointClient
.
* remove folder *src/test* | ||
* remove all test scope dependencies | ||
|
||
NOTE: we remove tests in preparation for the upcoming guides |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow this and it seems a bit odd to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamezp I changed the wording ... I was basically trying to say that the tests that were included with the archetype wouldn't be relevant anymore since we are using service to service invocation
|
||
==== pom.xml | ||
|
||
Set `<artifactId>simple-microservice-server</artifactId>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to be more descriptive here and state something like:
Update the
artifactId
to<artifactid>simple-microservice-server</artifactId>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamezp tnks, changed
Since you copied {source-code-git-repository}/simple-microservice[simple-microservice], the Dockerfile from link:https://github.com/wildfly/wildfly-s2i/blob/main/examples/docker-build/Dockerfile[examples/docker-build/Dockerfile, window="_blank"] should already be at the root of your project; | ||
|
||
==== Build the Docker Image | ||
|
||
[source,bash,subs="normal"] | ||
---- | ||
podman build -t {my-jaxrs-app-docker-image-name-server}:latest . | ||
---- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we don't use the wildfly-maven-plugin
to generate the Dockerfile/image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamezp thank you for the suggestion ... if you agree I'll add a note that tells about this option (https://docs.wildfly.org/wildfly-maven-plugin/releases/5.1/image-mojo.html) ... throughout the guides I tried to keep it stupid simple and just use the minimal set of tools/configurations the user might be already familiar with ... for example I didn't use glow ... my fear is that the reader might abandon the reading if feeling overwhelmed with too many tools/configurations ... I also wanted to show what's going on under the hood to show it's actually simple stuff
---- | ||
podman run --rm -p 8080:8080 -p 9990:9990 \ | ||
--network={podman-network-name} \ | ||
--env "SIMPLE_MICROSERVICE_SERVER_URI=http://{my-jaxrs-app-docker-image-name-server}:8080/hello" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hello
needs to be removed at the end since the GettingStartedEndpointInterface
includes @Path("/hello")
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamezp great catch! Thank you
|
||
=== Deploy on Kubernetes | ||
|
||
Create a file named `{my-jaxrs-app-docker-image-name-server}-deployment.yaml`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do I create this file? I assume in simple-microservice-server
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added instructions on where to create kubernetes files
|
||
[source,bash,subs="normal"] | ||
---- | ||
kubectl apply -f {my-jaxrs-app-docker-image-name-server}-deployment.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails if Kubernetes is not running. We should add a step for minikube start
.
|
||
=== Create Kubernetes ClusterIP Service | ||
|
||
We create a service to consume the services exposed by **{my-jaxrs-app-docker-image-name-server}** from inside Kubernetes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I assume this needs to happen in the client, but it's not really described. Or I guess in any directory, but seems it should specify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a link to https://kubernetes.io/docs/concepts/services-networking/service/ which should clarify this statement
|
||
[source,bash,subs="normal"] | ||
---- | ||
kubectl run --rm -it --tty curl-{my-jaxrs-app-docker-image-name-server} --image=curlimages/curl --restart=Never ‐‐ {my-jaxrs-app-docker-image-name-server}-service:8080/hello/pippo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work for me. It's possible I don't have minikube
started correctly. I just did minikube start
.
If you don't see a command prompt, try pressing enter.
curl: (6) Could not resolve host: xxx
Hello 'pippo'.pod "curl-my-jaxrs-app-server" deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just deleted and re-installed a fresh minikube .... perhaps the two dashed get copied to some other character ...
try:
kubectl run --rm -it --tty curl-simple-microservice-server --image=curlimages/curl --restart=Never -- http://simple-microservice-server-service:8080/hello/pippo
it worked for me ....
b97e844
to
ffb745e
Compare
cf300a6
to
c14e95d
Compare
@jamezp Thank you for your review! I hope I have fixed everything you pointed out |
@jamezp Thank you again fro your review, do you think this is ready to be merged now? |
@jamezp Do you think this is ready to be merged now (together with wildfly-extras/guides#4)? |
Hello @tommaso-borgato. I apologize, I just haven't had a chance to look at this again. These are fairly large changes and it takes some time to review them and go through the guide. I don't know if I'll have time this week or not, but I can try to make some time next week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still can't get this to working locally. minikube
doesn't seem to work for me with these guides. I'm not sure what the issue is TBH.
:version-wildfly: 34.0.1.Final | ||
:version-wildfly-galleon-pack: 34.0.1.Final |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be 35.0.0.Final or we could just leave the version off and use channels which will grab the latest version of WildFly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:version-wildfly-maven-plugin: 5.0.0.Final | ||
:version-junit-jupiter-api: 5.11.3 | ||
:version-wildfly-cloud-galleon-pack: 7.0.2.Final | ||
:version-wildfly-maven-plugin: 5.0.1.Final |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be 5.1.1.Final
* remove folder *src/test* | ||
* remove all test scope dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we doing this? Should the tests just be removed from the example if we remove them here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is about service ti service invocation and, unless we start both services in the same test, it would fail... perhaps we could leave the tests in the "server" service ...
|
||
[source,bash,subs="normal"] | ||
---- | ||
kubectl run --rm -it --tty curl-{simple-microservice-server} --image=curlimages/curl --restart=Never -- http://{simple-microservice-server}-service:8080/hello/pippo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible I've missed something, but this step fails for me with:
If you don't see a command prompt, try pressing enter.
curl: (28) Failed to connect to simple-microservice-server-service port 8080 after 130287 ms: Could not connect to server
pod "curl-simple-microservice-server" deleted
pod default/curl-simple-microservice-server terminated (Error)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's supposed to work if the following have already been deployed:
apiVersion: apps/v1
kind: Deployment
metadata:
name: simple-microservice-server-deployment
labels:
app: simple-microservice-server
spec:
replicas: 1
selector:
matchLabels:
app: simple-microservice-server
template:
metadata:
labels:
app: simple-microservice-server
spec:
containers:
- name: simple-microservice-server
image: quay.io/tborgato/simple-microservice-server
ports:
- containerPort: 8080
- containerPort: 9990
livenessProbe:
httpGet:
path: /health/live
port: 9990
readinessProbe:
httpGet:
path: /health/ready
port: 9990
startupProbe:
httpGet:
path: /health/started
port: 9990
apiVersion: v1
kind: Service
metadata:
name: simple-microservice-server-service
labels:
app: simple-microservice-server
spec:
ports:
- name: http
protocol: TCP
port: 8080
targetPort: 8080
selector:
app: simple-microservice-server
type: ClusterIP
Then it is working for me:
]$ kubectl run --rm -it --tty curl-simple-microservice-server --image=curlimages/curl --restart=Never -- http://simple-microservice-server-service:8080/hello/tommy
Hello 'tommy'.pod "curl-simple-microservice-server" deleted
The "Hello 'tommy'" part tells the service is OK
c14e95d
to
42feb0e
Compare
@jamezp Thank you for your review! I hope I have answered/fixed everything ... |
This PR contains 3 more guides to be added to the Java Microservices on Kubernetes with WildFly:
wildfly-extras/guides#4 contains the source code for all the 3 new guides
#676 contains a proposal on HOW-TO re-organize the whole "WildFly mini series" contents
Fixes #687