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

Support for cloud-based platforms #7

Merged
merged 15 commits into from
Sep 1, 2020
Merged

Conversation

odedia
Copy link
Contributor

@odedia odedia commented Aug 2, 2020

This version has been adapted to Spring Boot 2.3 which supports building docker images with cloud-native buildpacks, and supports deploying to Cloud Foundry and Kubernetes with free Wavefront monitoring.

…mages, and supports deploying to Cloud Foundry and Kubernetes with free Wavefront monitoring.
@odedia odedia closed this Aug 2, 2020
@odedia odedia reopened this Aug 2, 2020
@odedia
Copy link
Contributor Author

odedia commented Aug 2, 2020

.travis.yml has been bumped to jdk11

@axymthr
Copy link

axymthr commented Aug 16, 2020

Hi @odedia is this still planned to be merged? If there's any more work required on the Kubernetes end, could I help out somehow? Unfortunately I may not be very helpful with Cloud Foundry and Wavefront.

@odedia
Copy link
Contributor Author

odedia commented Aug 16, 2020

Hi. It is pretty much done for this version.

I created a PR for the main repo, it’s in their hands now. I don’t have permissions to merge.

Thanks for the interest!

@arey
Copy link
Member

arey commented Aug 16, 2020

Sorry for the delay. I m on vacation without laptops . I try to review it tomorrow.

@odedia
Copy link
Contributor Author

odedia commented Aug 16, 2020

No code reviews while on holiday! :)
Enjoy your vacation, these things can wait.

Copy link
Member

@arey arey left a comment

Choose a reason for hiding this comment

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

Here is my first review. For structural changes, we could create GitHub issues.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
```
If your operator deployed the wavefront proxy in your Cloud Foundry environment, point the URI to the proxy instead. You can obtain the value of the IP and port by creating a service key of the wavefront proxy and viewing the resulting JSON file.

Contine with creating the services and deploying the application's microservices. A sample is available at `scripts/deployToCloudFoundry.sh`. Note that some of the services' plans may be different in your environment, so please review before executing. For example, you want want to fork [https://github.com/odedia/spring-petclinic-microservices-config.git]https://github.com/odedia/spring-petclinic-microservices-config.git if you want to make changes to the configuration.
Copy link
Member

Choose a reason for hiding this comment

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

I see you have done some changes to the spring-petclinic-microservices-config repo: spring-petclinic/spring-petclinic-microservices-config@master...odedia:master

Majors changes are in new config files for the cloud and k8s spring profiles. Only the api-gateway.yml has been rewritten. But I suspect an issue in the existing one that sould be empty (see spring-petclinic/spring-petclinic-microservices-config#12)

We have several options:

  1. Create a new spring-petclinic-cloud-config repo with your config
  2. Create a dedicated branch in the existing spring-petclinic-microservices-config one
  3. Adapt the existing spring-petclinic-microservices-config to support both spring-petclinic-microservices and spring-petclinic-cloud.

The third solution has my preference for maintenance work but we could have regression.
What is your position?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer a separate repo to avoid any lockin and regression tests between the two repos. They seem to be going in slightly different directions. It will also allow us to experiment more with the cloud-based config without impacting the existing microservices repo.

Copy link
Member

Choose a reason for hiding this comment

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

Ok thus I've create the https://github.com/spring-petclinic/spring-petclinic-cloud-config repo. I let you published your config.

README.md Show resolved Hide resolved

You free account has now been created.

Create a user-provided service for Wavefront using the data above. For example:
Copy link
Member

Choose a reason for hiding this comment

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

What happens if we don't create the Wavefront service? Does the application works without Wavefront?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you will need to remove the dependency from the pom.xml to disable it.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. So it's a point of failure. Sleuth and OpenTracing have a different behavior

k8s/init-services/02-config-map.yaml Show resolved Hide resolved
enabled: true

customers-service-id: http://customers-service.spring-pet-clinic.svc.cluster.local:8080
visits-service-id: http://vists-service.spring-pet-clinic.svc.cluster.local:8080
Copy link
Member

Choose a reason for hiding this comment

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

Typo: vists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks.

@@ -0,0 +1,22 @@
echo "Creating Required Services..."
{
cf create-service -c '{ "git": { "uri": "https://github.com/odedia/spring-petclinic-microservices-config.git", "periodic": true }, "count": 3 }' p.config-server standard config &
Copy link
Member

Choose a reason for hiding this comment

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

URL to review

spring-petclinic-config-server/pom.xml Show resolved Hide resolved
<version>2.3.1</version>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>2.3.1.RELEASE</version>
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove usage of the parent pom? Did you change it to a full reactor pom?

I suppose to have microservice that share nothing (no code and no maven config) ? A default is that we have plenty of copy paste in pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No shared parent anymore since each microservice has a completely separate lifecycle now. Some microservices may want to stay on 2.3.1, while others may want to upgrade to 2.3.2 and JDK 14. It becomes a problem when they are bound to the same upstream.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's go with this choice.

README.md Outdated Show resolved Hide resolved
@arey arey merged commit a1dda0b into spring-petclinic:master Sep 1, 2020
HakjunMIN pushed a commit to HakjunMIN/spring-petclinic-cloud that referenced this pull request Mar 20, 2024
Support for cloud-based platforms
HakjunMIN pushed a commit to HakjunMIN/spring-petclinic-cloud that referenced this pull request Mar 20, 2024
Support for cloud-based platforms
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants