-
Notifications
You must be signed in to change notification settings - Fork 1k
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
the fail-fast:true property is not triggered if the config map is not found #1016
Comments
you're right, we will not fail the app if the configmap or secrets is not found, but that was the behavior for a very long time. I debated this same issue in some PRs, but the response was always the same - this is expected. Over the time, this grew up on me and I agree. If you think about it, lack of some (or incorrect) property read, should result in a different exception in your business logic. I see this in various other places in spring eco-system too, spring projects rarely fail by default on such things. In general, having such a property here where it would generate a failure could get hairy fast. You can specify more than one configmap for example, and then what should happen? Allow failure of the app for specific ones? Fail if at least one fails? Have a sensible default for all of them? I don't know the correct answer; but having implemented some of these things, it will get complicated :) btw, you might be the first user reporting and wanting such a feature. If owners of this project think there is a simple way to do this (and if it's worth it), I could implement it. |
just dropping a note, if we get to this in the future. may be name such a property |
I have been thinking about this one over some weekends and this is not trivial at all. In general, we support two types of searches: by name and by labels. If we think about these two in isolation, then things are not that complicated and we could fail if we can't find what we are looking for. The (big) problem arises when you think about profiles and profiles are a big part of Spring DNA. For example As such, if we want to implement this like any other property we have, it could look like : You could then say, how about:
Well, in such a case, we will have 4 configmaps to read in the end: We could solve this via:
But what if there are 10 profiles total? We would need to repeat:
10 times in the configuration file. This is ugly, at best and it also couples the profiles way too much to the configuration. It becomes obvious that What if we will support only source based strictness? For example:
The question still remains, how to treat
In such a case, only
So enforce strict for all profiles? Let's say you have a profile
It can get even more hairy: what if we treat
In such a case, we would validate that:
I don't know why but this looks heavily involved, to me. |
I was thinking into a slightly different direction here: what if define profiles that you want to have strictness for? For example:
it will validate the presence of While I am inclined towards this one, the issue comes that we already have a property called
and there are two active profiles I don't mind trying to implement such an approach, but I need @ryanjbaxter thoughts here and yours @jbkervyn too, if you have the time. Thank you. |
note for myself: I started working on this one, and I'll fix one more issue along the way. I've created this fix that makes sure that
we do make any distinction (order wise) between What I will be fixing also is being in par with profiles |
I really like your ideas @wind57 . However, what I had in mind was a more simple use-case not going that much into depth with the profiles that you are rightfully pointing out. What I was thinking is : as long as you do have a configmap mentionned, there should be at least one hit. So if we do take this strict idea that I do like, it would mean : as long as strict = true, the validation would only succeed if a configmap having value either Just to ellaborate on the usecase I was initially having when I raised this issue : Simple spring boot app, having no spring.cloud.kubernetes.config.sources property explicitely set. And by the presense of the depencency, would look for a configmap matching the application.name. In this use-case, I thought that it would make sense to have a failing validation when no configmap would be found. That's why I thought having a at-least logic would make sense. But I must confess that I do not possess a decent knowledge on the topic, so I'm just sharing, but I do value more your expertise over mine. Thanks a lot for putting so many thoughts into this :) |
No worries at all, I liked the idea and I do admit I thought about such a failure of the app when support was added for imho, what I propose is nothing more then your proposal + some more use-cases, thus more configurations possible, thus more users that are happy. To your case btw, simply |
I completely agree with your proposal, I was just thinking, maybe we are making it now a bit too complex/flexible. But I eventually think you're right and that this is the way to go. |
I am kind of lost from the beginning here, the application should fail to start if
|
Currently, there is no way to fail for a missing source. |
OK now it makes more sense. The issue here is whether to fail if the configmap is missing completely. When I think about this, its just not something we do for the most part. As @wind57 points out, in a simple Spring app if there is no profile specific configuration file the app does not fail to start. Now there is one case where I can think of where we would fail if a configuration file does not exist and this is when using Now we do support It feels like maybe using |
I have tried to have a POC here, it's still not finished. The idea of how I was envisioning this is described above in my comments with I don't know what you think about it Ryan. If it's a direction you want to pursue, I would bring that to a stable state. |
Honestly it seems overly complicated, there are just too many variations. I also think that if the application is missing a property it requires than it should fail, the source of where that property comes from should not really matter, its the value that matters at the end of the day. |
I debated this with myself for quite a while. I had the same thought that the app should fail elsewhere, if you need a property. Then I decided - what if I try to implement it, how complicated can it get? The response is very, so much that I had a very hard time with my own thoughts. At this point in time, I am convinced that its not worth. Ill close my PR for the time being. |
Hi @wind57, thanks for putting so many efforts into this. I can quiet relate to your state of mind when you write that you had a hard time with your own thoughts :) The same parallelism can be found with : spring boot fails to start when you declare spring-data-jpa without having any jdbc driver in your classpath. It's for that reason that I raised this issue. Fail/Warn during startup if not a single configmap is found :) But as I wrote before, I'm not knowledgeable on all the implications, so I do trust your judgements over mine :) |
That is not true, you can use spring-cloud-kubernetes for a variety of things
To me this is a bit different. This is a dependency that needs to be present in order to function. The equivalent in Spring Cloud Kubernetes is if you tried to use it without Fabric8 or the Kubernetes Java Client on the classpath. I marked the issue as waiting for votes to see if other community members think they want this functionality. |
I would vote for failing if configmap is not found with fail-fast property. |
Even if we shut down the application because the config map was deleted wouldn't K8S keep trying to redeploy the pod and then run into the same problem? |
No, the difference is that @RefreshScope/@ConfigurationProperties beans are recreated lazily when these beans are actually used in code and, on the other hand, when application is starting it creates them on startup and shuts down if it can't create them. |
Right so if we provided an option to shut the application down if the config map is not found, Kubernetes will just restart it because it believes it should be running, and it will fail at startup again. I guess I don't see how this is helping with the issue. |
What if the bean with a property from k8s config is recreated lazily after some irreversible actions like sending an email? In this case fail fast option would prevent any attempts to process any requests. |
In the light of some work that I am doing right now, I think that this one is doable in a simple manner. I will present my ideas soon. |
I think it would be handy (except if there is another way to do this), to make your application fail when no matching configmap can be found in the namespace. I would expect this to be the behavior of fail-fast. However, this only happens when there is an exception while reading the configmap. If the configmap is not found, no exception is triggered.
The text was updated successfully, but these errors were encountered: