Close fabric8 client#1773
Close fabric8 client#1773wind57 wants to merge 193 commits intospring-cloud:mainfrom wind57:close-fabric8-client
Conversation
Configure Renovate
|
First thing was for me to reproduce this one locally, which I did. The 502 Bad Gateway was really coming from ingress, it could not connect to the underlying Spring app that we were running for the test. The test we have in place goes like this:
I simplified the test for myself a bit, so that I would not run in k3s, but connect to my local running cluster. This way I could debug the application far easier. This did not help much :) but I noticed something interesting: when I issue a From here, it was easy, implement |
|
@ryanjbaxter fixes the main build failure |
| return new Fabric8PodUtils(client); | ||
| } | ||
|
|
||
| @PreDestroy |
There was a problem hiding this comment.
Can we just add @Bean(destroyMethod = "close") to the KubernetesClient bean above?
There was a problem hiding this comment.
I'm guessing that didn't work?
There was a problem hiding this comment.
hey, sorry, it worked partially. It did not work when we use config data api and register this bean via ConfigurableBootstrapContext. Your idea is much nicer and more correct then mine, its just that I don't know how to register the same destroyMethod in this case, but I'm reading the code a bit. Let me see if I find a way to do it
There was a problem hiding this comment.
eh no, it did not work :( I don't know why though... may be we can push this fix, and if I figure out your suggestion, amend it later
There was a problem hiding this comment.
If I read the documentation of destroyMethod on a @Bean, we should not even have to add it explicitly and it must work, but my tests, and pipeline failures, show that its not really the case.
|
I took another pass for a few hours today in the morning to see if I understand why your suggesting does not work, and I did not succeeded. I am still curious what is going on, but as long as we have a solution in place, I think we are good. wdyt? |
|
actually this should go into |
3 participants
No description provided.