Unable to listen for file changes

[tianmingxing]

Describe the bug

When the application starts, it reads the configuration from the specified file path and creates a MapPropertySource.

But in the logic of listening, KubernetesClientConfigMapPropertySource. class is compared to it.

View the real situation of Environment. getPropertySources() in the JVM.

So the mismatch between these two will result in the application not being hot loaded when the file content changes.

[wind57]

You are obviously taking your time to open these issues and I, personally, very much appreciate this.

But it's complicated for me to navigate and fully understand what you mean, at times.

That is why we usually suggest for issues to have a minimal, reproducible example with clear steps on how to re-produce. For example, look at this sample and steps that were provided to us, which lead to me understanding the issue, being able to re-produce and fix it.

I will gladly look at this one too, but can you please provide such details?

[tianmingxing]

Sorry, I didn't describe the problem clearly. In fact, the above problem can be condensed into one problem, which is the following code:

composite.addFirstPropertySource(new MapPropertySource(name, map));

Should it become like this?

composite.addFirstPropertySource(new KubernetesClientConfigMapPropertySource(name, map));

They are located in: org.springframework.cloud.kubernetes.commons.config.ConfigMapPropertySourceLocator#addPropertySourceIfNeeded

Because during the process of listening to files, two classes will be compared，sourceClass=KubernetesClientConfigMapPropertySource.class, but source=MapPropertySource.class, The two are not equal!

else if (sourceClass.isInstance(source)) {
    managedSources.add(sourceClass.cast(source));
}

An example project for the minimum reproducible problem can be provided, even though it may be a bit troublesome, I think it is necessary.

[wind57]

I think you are on to something here...

Should it become like this?

composite.addFirstPropertySource(new KubernetesClientConfigMapPropertySource(name, map));

that's not an option since this code is in commons and we have two clients that use it. I'll take a closer look and write some tests to see that it reproduces. So far, I agree, it's a bug; but I'll spent some time in the upcoming days to confirm and potentially fix. The same problem happens with secrets I assume, and we need it fixed (again, if I confirm this is a bug) in both 2.1.x and 3.x.x branches.

[wind57]

we currently only "listen" when a ConfigMap changes, not when a path does. it looks like an older issue, see here. The solution is to have this in spring-cloud-commons, but its not currently implemented.

[wind57]

In general, I think I might try to implement such a feature in the future, but I need some green light from Ryan first.

I have not thought about it too much, but seems doable. @ryanjbaxter share your wisdom please, thank you.

[tianmingxing]

Listening to ConfigMap updates can add a burden to Api Server nodes, especially when the cluster is very large and there are too many application nodes that need to request it. For this reason, this plan will not be approved by the operation and maintenance engineer.

I personally also prefer the solution of listening to files. When I update ConfigMap, the files in the container will be updated synchronously after about 10 seconds. At this time, the application reads the latest configuration and hot loads the application, which is the best.

[ryanjbaxter]

This listening issue seems separate to the type problem originally reporting.

Listening to changes in config maps mounted as volume and telling the app to refresh is handled by the config map watcher
https://docs.spring.io/spring-cloud-kubernetes/docs/3.0.2/reference/html/#spring-cloud-kubernetes-configuration-watcher

[wind57]

So listening for file changes is a bug for grabs, right?

[ryanjbaxter]

No. The configuration watcher already provides support for this.

[wind57]

(scratches head) so where is the bug then? Cause the label "bug" exists.

[ryanjbaxter]

The way I interpreted this issue was there is a bug here
https://github.com/spring-cloud/spring-cloud-kubernetes/blob/main/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapPropertySourceLocator.java#L142

Because we create a MapPropertySource the check here
https://github.com/spring-cloud/spring-cloud-kubernetes/blob/main/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/reload/ConfigReloadUtil.java#L89

will never be true because we only pass Fabric8ConfigMapPropertySource.class or KubernetesClientConfigMapPropertySource.class to this method

But I could be misunderstanding something too.

[wind57]

I'm not 100% sure either, I'll write a small integration test to see what is going on and reply back here

[tianmingxing]

If it is confirmed to be a bug, please take some time to rest. Thank you very much!

[wind57]

I am a little stuck with the actual job that I have that pays the bills :), but once I'm done with that, this will be the next thing I'm going to take a look at. FYI

[wind57]

I can reproduce this one via an integration test, it's a bug... I'll fix it and present the comments in the PR when its ready.