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

bump springboot to 2.2.7 #742

Closed
wants to merge 1 commit into from

Conversation

nimakaviani
Copy link
Member

@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

  • bfec15c: bump springboot to 2.2.7

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@luispollo
Copy link
Contributor

@cfieber as part of the SpringBoot stuff you've been working on, have you had any issues with upgrading to later versions? We'd like to bump up the version slightly here, but this got me wondering if there's anything blocking us from upgrading to the next major.

@luispollo
Copy link
Contributor

Ah. Spoke too soon. Found the PR where we had previously had to revert the exact same upgrade, @nimakaviani: #665

@robfletcher would have more context.

@nimakaviani
Copy link
Member Author

hmmm interesting. would be good to know how we can move forward and resolve the conflict with internal metrics.

@cfieber
Copy link
Contributor

cfieber commented Aug 12, 2020

@luispollo I feel like its worth a try again to see if this is in a happy place for us, a lot has changed since the previous attempt got rolled back.

I'd suggest applying this locally and seeing if everything starts up fine running our internal wrapper

@luispollo
Copy link
Contributor

I'd suggest applying this locally and seeing if everything starts up fine running our internal wrapper

Thanks @cfieber, will do. I think my concern is that this affects all services, not just keel, but I can try with a couple.

@nimakaviani
Copy link
Member Author

@luispollo did you by any chance get to try this version bump to see how things work?

@luispollo
Copy link
Contributor

@luispollo did you by any chance get to try this version bump to see how things work?

Sorry, not yet. Will move that up in the queue next week.

@robzienert
Copy link
Member

Ping.

@robzienert robzienert closed this Sep 30, 2020
@luispollo
Copy link
Contributor

luispollo commented Oct 3, 2020

Sorry I missed the ping @robzienert and @nimakaviani. Will give this a try next week, so reopening it to track.

@luispollo luispollo reopened this Oct 3, 2020
@luispollo
Copy link
Contributor

luispollo commented Oct 3, 2020

OK... I gave this a shot today. Things were looking good until I tried our orca wrapper (I tested keel, echo and orca). It failed to start with this exception:

Caused by: java.lang.invoke.LambdaConversionException: Invalid receiver type class org.springframework.boot.actuate.autoconfigure.metrics.export.atlas.AtlasProperties; not a subtype of implementation type class org.springframework.boot.actuate.autoconfigure.metrics.export.properties.PushRegistryProperties
	at java.base/java.lang.invoke.AbstractValidatingLambdaMetafactory.validateMetafactoryArgs(AbstractValidatingLambdaMetafactory.java:254)
	at java.base/java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:328)
	at java.base/java.lang.invoke.BootstrapMethodInvoker.invoke(BootstrapMethodInvoker.java:127)
	... 65 common frames omitted

There was a change to the AtlasProperties class hierarchy per spring-projects/spring-boot#20843, however this breaks this app for us. I was unable to determine whether this is actually a bug in Spring Boot itself (since all other monitoring system properties classes seem to still extend PushRegistryProperties in 2.2.7) or a version incompatibility with internal Netflix libraries that setup Spectator. I don't have more bandwidth to look at this at this time, but wanted to let you know what I found.

I'm guessing this is the same problem @robfletcher ran into and why he had to revert the upgrade in #665

In any case, @nimakaviani, based on our last conversation in the SIG, it didn't sound like you were blocked by this anymore, right?

@luispollo luispollo closed this Oct 3, 2020
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.

Springboot 2.2.5 used as part of Kork 7.57 has a bug in loading properties.
5 participants