spring-cloud / spring-cloud-cloudfoundry

Integration between Cloudfoundry and the Spring Cloud APIs
Apache License 2.0
80 stars 73 forks source link

Upgrade CF Java Client dependency #65

Closed twoseat closed 4 years ago

twoseat commented 4 years ago

A user of this project has reported an issue against Cloud Foundry Java Client that may be related to the outdated version of the java client included here (3.9.0.RELEASE instead of either 3.25.0.RELEASE or 4.8.0.RELEASE).

We're planning to EOL the 3.x line of the java client around the end of October, in line with the end of updates for Boot 2.1. Hence upgrading to the 4.x line, or possibly even the 5.x line that will be released around the same time, would be a worthwhile effort.

(The only difference beween 4.x and 5.x should be the underlying version of various reactor components, most noteably reactor-netty, so no functionality would be missed by upgrading to 4.x now and 5.x once it has stabilised a little.)

spencergibb commented 4 years ago

We can upgrade to any version on master we want since it is a major version. 2.2.x will have to stay on 3.x of the client though. Support for the 2.2.x branch is to mid-next year.

ahmedmq commented 4 years ago

@TYsewyn - interested to work on this issue and start contributing to OSS.

TYsewyn commented 4 years ago

That's great Mushtaq! I'd recommend to read the README first since it explains what you need to do to start contributing and how you need to set up the project. If you have any questions don't hesitate to ping me here!

ahmedmq commented 4 years ago

Sure, Will do. Will reach out to you directly if I need detailed answers

twoseat commented 4 years ago

Update: The user forced the relevant dependencies on the java client to 3.25.0, which solved their problem and has resulted in no other issues (to my knowledge).

TYsewyn commented 4 years ago

@twoseat This has been fixed for 2.2.x and will be released shortly. 🙂 @ahmedmq I'd suggest to upgrade to 5.x on the main branch for 2020.0.0-RELEASE (Ilford) unless we have dependency conflicts.

twoseat commented 4 years ago

@ahmedmq Note that 5.x won't be available until shortly after Spring Boot 2.4 GAs, so no immediate rush!

spencergibb commented 4 years ago

I belive this is causing problems with tests.

Error creating bean with name 'cloudFoundryOperations' defined in org.springframework.cloud.cloudfoundry.CloudFoundryClientAutoConfiguration: Unsatisfied dependency expressed through method 'cloudFoundryOperations' parameter 0; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'cloudFoundryClient': Invocation of init method failed; nested exception is java.lang.NoSuchMethodError: reactor.core.publisher.Mono.retry(Ljava/util/function/Predicate;)Lreactor/core/publisher/Mono;

In CloudFoundryClientAutoConfigurationTest.autoConfiguresBeansWithAllProperties and CloudFoundryClientAutoConfigurationTest.autoConfiguresBeansWithMinimalProperties

TYsewyn commented 4 years ago

@spencergibb are you referring to 2.2.x? Or did you make changes on master?

ahmedmq commented 4 years ago

@TYsewyn - The master branch is on 3.9.0 version, but the tests mentioned above are ignored in master. I also see Spring Boot 2.4 will be GA on September mid. Once 5.x version is available I will put it here and check for issues.

TYsewyn commented 4 years ago

Hi @ahmedmq! Can you upgrade the master branch to the latest version of 4.x? According to Paul the major difference between 4.x and 5.x are the underlying reactor components, so already upgrading to 4.x would save us some time if there are API changes - which I suspect there will be. After you have upgraded the library, can you also check if those tests are working again then?

Let us know if you encounter any issue or problem and we'll help you out!

ahmedmq commented 4 years ago

@TYsewyn - Any idea why the test cases are failing for missing class definitions:

INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests
[ERROR] Tests run: 8, Failures: 0, Errors: 8, Skipped: 0, Time elapsed: 0.068 s <<< FAILURE! - in org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests
[ERROR] addTokenUri  Time elapsed: 0.059 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addTokenUri(VcapServiceCredentialsEnvironmentPostProcessorTests.java:77)
Caused by: java.lang.ClassNotFoundException: org.springframework.core.metrics.ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addTokenUri(VcapServiceCredentialsEnvironmentPostProcessorTests.java:77)
[ERROR] addUserInfoUri  Time elapsed: 0 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addUserInfoUri(VcapServiceCredentialsEnvironmentPostProcessorTests.java:99)
[ERROR] addClientIdUnderscores  Time elapsed: 0.001 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addClientIdUnderscores(VcapServiceCredentialsEnvironmentPostProcessorTests.java:66)
[ERROR] addServiceId  Time elapsed: 0 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addServiceId(VcapServiceCredentialsEnvironmentPostProcessorTests.java:111)
[ERROR] addClientId  Time elapsed: 0 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addClientId(VcapServiceCredentialsEnvironmentPostProcessorTests.java:56)
[ERROR] noop  Time elapsed: 0 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.noop(VcapServiceCredentialsEnvironmentPostProcessorTests.java:45)
[ERROR] addJwtKeyUri  Time elapsed: 0 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addJwtKeyUri(VcapServiceCredentialsEnvironmentPostProcessorTests.java:121)
[ERROR] addTokenUriAuthDomain  Time elapsed: 0 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/springframework/core/metrics/ApplicationStartup
    at org.springframework.cloud.cloudfoundry.environment.VcapServiceCredentialsEnvironmentPostProcessorTests.addTokenUriAuthDomain(VcapServiceCredentialsEnvironmentPostProcessorTests.java:88)
[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.addClientId:56 » NoClassDefFound
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.addClientIdUnderscores:66 » NoClassDefFound
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.addJwtKeyUri:121 » NoClassDefFound
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.addServiceId:111 » NoClassDefFound
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.addTokenUri:77 » NoClassDefFound
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.addTokenUriAuthDomain:88 » NoClassDefFound
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.addUserInfoUri:99 » NoClassDefFound
[ERROR]   VcapServiceCredentialsEnvironmentPostProcessorTests.noop:45 » NoClassDefFound ...
[INFO] 
[ERROR] Tests run: 8, Failures: 0, Errors: 8, Skipped: 0
[INFO] 

Module: spring-cloud-cloudfoundry-web

TYsewyn commented 4 years ago

Interesting, I don't see those issues. Can you run ./mvnw clean test in your terminal to check if that works?

ahmedmq commented 4 years ago

@TYsewyn - Yea seems like I had an issue with maven and it was not downloading all dependencies. This was sorted after doing a force maven install. So was able to run all tests successfully.

I changed the version to cf-java-client.version version to 4.9.0 in the root POM. This caused a compilation error in a couple of tests

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile (default-testCompile) on project spring-cloud-cloudfoundry-discovery: Compilation failure: Compilation failure:
[ERROR] /Users/amushtaq/workspace/contributions/spring-cloud-cloudfoundry/spring-cloud-cloudfoundry-discovery/src/test/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryAppServiceDiscoveryClientTest.java:[66,33] method url in class org.cloudfoundry.operations.applications.ApplicationDetail.Builder cannot be applied to given types;
[ERROR]   required: java.lang.String
[ERROR]   found: java.lang.String,java.lang.String
[ERROR]   reason: actual and formal argument lists differ in length
[ERROR] /Users/amushtaq/workspace/contributions/spring-cloud-cloudfoundry/spring-cloud-cloudfoundry-discovery/src/test/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryAppServiceDiscoveryClientTest.java:[89,33] method url in class org.cloudfoundry.operations.applications.ApplicationDetail.Builder cannot be applied to given types;
[ERROR]   required: java.lang.String
[ERROR]   found: java.lang.String,java.lang.String
[ERROR]   reason: actual and formal argument lists differ in length

Looking at the Test case it seems the method signature for creating a org.cloudfoundry.operations.applications.ApplicationDetail has been changed in this version of the Cloud Foundry Java Client . The below is the method to create the ApplicationDetail object in which the url() method has wrong number of arguments.


ApplicationDetail applicationDetail = ApplicationDetail.builder().id("billing-id")
                .name("billing").instances(3).memoryLimit(1024).stack("cflinux2")
                .diskQuota(1024).requestedState("Running").runningInstances(3)
                .url("billing.apps.example.com", "billing.apps.internal").build();
These are the two builder methods that is available in ```org.cloudfoundry.operations.applications.ApplicationDetail``` to add to the URLs list object ```/** * Adds one element to {@link ApplicationDetail#getUrls() urls} list. * @param element A urls element * @return {@code this} builder for use in a chained invocation */ public final Builder url(String element) { this.urls.add(Objects.requireNonNull(element, "urls element")); return this; } /** * Adds elements to {@link ApplicationDetail#getUrls() urls} list. * @param elements An array of urls elements * @return {@code this} builder for use in a chained invocation */ public final Builder urls(String... elements) { for (String element : elements) { this.urls.add(Objects.requireNonNull(element, "urls element")); } return this; } ``` So modified the code to use the 2nd method above which accepts multiple String arguments.

ApplicationDetail applicationDetail = ApplicationDetail.builder().id("billing-id")
                .name("billing").instances(3).memoryLimit(1024).stack("cflinux2")
                .diskQuota(1024).requestedState("Running").runningInstances(3)
                .urls("billing.apps.example.com", "billing.apps.internal").build();
Also uncommented the test cases in ```CloudFoundryClientAutoConfigurationTest.autoConfiguresBeansWithAllProperties``` and ```CloudFoundryClientAutoConfigurationTest.autoConfiguresBeansWithMinimalProperties``` which were previously commented out. Now all test cases are passing after the CF Java Client version upgrade.
TYsewyn commented 4 years ago

Cool! Could you create a pull request with your changes and add Closes gh-65 in the description? Then we can continue to discuss code changes inline. Thanks for your work already!

ahmedmq commented 4 years ago

@TYsewyn - Raised a Pull Request #73