Skip to content

some cleanups #985

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Mar 14, 2022
Merged

some cleanups #985

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ba6af42
see if it fails on the pipeline
wind57 Mar 14, 2022
0b7e8ab
add dependency
wind57 Mar 14, 2022
455bf15
re-trigger
wind57 Mar 14, 2022
6fe8738
re-trigger
wind57 Mar 14, 2022
877ae66
non-deprecated image
wind57 Mar 14, 2022
f33ba2e
non-deprecated image
wind57 Mar 14, 2022
ef9f400
re-trigger
wind57 Mar 14, 2022
46d6c0c
re-trigger
wind57 Mar 14, 2022
9578007
re-trigger
wind57 Mar 14, 2022
89f1f53
re-trigger
wind57 Mar 14, 2022
ea711c1
1.69 version
wind57 Mar 14, 2022
a95586b
no explicit dep
wind57 Mar 14, 2022
c64b388
trigger
wind57 Mar 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
test:
parallelism: 5 # parallel containers to split the tests among
machine:
image: circleci/classic:201707-01
image: ubuntu-2004:202201-02
Copy link
Contributor Author

@wind57 wind57 Mar 14, 2022

Choose a reason for hiding this comment

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

newer images, these will not work anymore from May 22 (and until then there will be random times when they will not be available), so let's just upgrade

Copy link
Contributor

@ryanjbaxter ryanjbaxter Mar 14, 2022

Choose a reason for hiding this comment

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

Ah I changed this on main but forgot to cherry pick it, thanks!

environment:
_JAVA_OPTIONS: "-Xms1024m -Xmx2048m"
_SERVICE_OCCURENCE: 5
Expand Down Expand Up @@ -140,7 +140,7 @@ jobs:
destination: testartifacts
build:
machine:
image: circleci/classic:201707-01
image: ubuntu-2004:202201-02
environment:
_JAVA_OPTIONS: "-Xms2g -Xmx2g"
_SERVICE_OCCURENCE: 5
Expand Down
12 changes: 12 additions & 0 deletions spring-cloud-kubernetes-dependencies/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<wiremock.version>2.26.3</wiremock.version>
<spring-retry.version>1.3.1</spring-retry.version>
<commons.collections4.version>4.4</commons.collections4.version>
<bouncycastle.version>1.69</bouncycastle.version>
Copy link
Contributor Author

@wind57 wind57 Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

this is the highest version where both clients are compatible, as both need it. I tried 1.70, fabric8 works, but native one fails with a missing method. I will take a look closer into that error and see if an upgrade will help, but that is out of scope here. So for now, this version will do

</properties>
<dependencyManagement>
<dependencies>
Expand Down Expand Up @@ -86,6 +87,17 @@
<version>${spring-retry.version}</version>
</dependency>

<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
<version>${bouncycastle.version}</version>
</dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcpkix-jdk15on</artifactId>
<version>${bouncycastle.version}</version>
</dependency>

<!-- Own dependencies -->
<dependency>
<groupId>org.springframework.cloud</groupId>
Expand Down
8 changes: 8 additions & 0 deletions spring-cloud-kubernetes-fabric8-autoconfig/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@
<scope>test</scope>
</dependency>

<dependency>
Copy link
Contributor Author

@wind57 wind57 Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

fabric-autoconfig tests where failing for me in a diff PR, and I have noticed that this dependency is needed, but it's not in place anymore. I guess it was coming from some spring-boot dependency and now it is not there anymore.

Copy link
Contributor

@ryanjbaxter ryanjbaxter Mar 14, 2022

Choose a reason for hiding this comment

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

All of our builds internally are passing both in the 2.1.x and main branches, so I don't think anything changes that would cause us to need this dependency now

Copy link
Contributor Author

@wind57 wind57 Mar 14, 2022

Choose a reason for hiding this comment

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

Im really sorry - the error was on my laptop, only. :(

Copy link
Contributor

@ryanjbaxter ryanjbaxter Mar 14, 2022

Choose a reason for hiding this comment

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

No worries glad you got if figured it. Still worth making the other changes though.

Copy link
Contributor Author

@wind57 wind57 Mar 14, 2022

Choose a reason for hiding this comment

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

right, build is green now

<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
</dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcpkix-jdk15on</artifactId>
</dependency>

</dependencies>

Expand Down
3 changes: 0 additions & 3 deletions ...fig/src/test/java/org/springframework/cloud/kubernetes/Fabric8AutoConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.runner.RunWith;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
Expand All @@ -32,11 +31,9 @@
import org.springframework.cloud.kubernetes.fabric8.Fabric8InfoContributor;
import org.springframework.cloud.kubernetes.fabric8.Fabric8PodUtils;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.test.context.junit4.SpringRunner;

import static org.assertj.core.api.Assertions.assertThat;

@RunWith(SpringRunner.class)
Copy link
Contributor Author

@wind57 wind57 Mar 14, 2022

Choose a reason for hiding this comment

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

minor clean-up from junit-4

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = App.class,
properties = { "spring.cloud.kubernetes.client.password=mypassword",
"spring.cloud.kubernetes.client.proxy-password=myproxypassword" })
Expand Down