Refactor k8s client reload #1902
Conversation
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
| protected void putPathConfig(CompositePropertySource composite) { | ||
|
|
||
| if (!properties.paths().isEmpty()) { | ||
| Set<String> uniquePaths = new LinkedHashSet<>(properties.paths()); |
There was a problem hiding this comment.
we do this for configmaps, but not for secrets, so I decided to add it here
| // we know that the type is correct here | ||
| managedSources.add((S) mountConfigMapPropertySource); | ||
| } | ||
| else if (propertySource instanceof SecretsPropertySource secretsPropertySource) { |
There was a problem hiding this comment.
I am fixing a bug here with this one.
In case of configmaps, when we create a property source from paths, we create an instance of MountConfigMapPropertySource. Then, in ConfigReloadUtil, there is such a check:
if (source instanceof MountConfigMapPropertySource mountConfigMapPropertySource) { ... }
which means that reloads will take it into consideration.
While refactoring one integration test, I saw that it is failing, and after some debugging, realized that we have no code like the above for secrets. As such, I added that simple condition, because when we create a property source from paths in case of secrets, we create an instance of SecretsPropertySource
|
|
||
| // sometimes we get errors like : | ||
|
|
||
| // "message": "Discovery failed for some groups, |
There was a problem hiding this comment.
we have already this logic in fabric8 client, so I added it in here too
Signed-off-by: wind57 <[email protected]>
|
@ryanjbaxter this was supposed to be just one more IT refactor, but it turned out a bit more complicated. I added comments to the relevant parts that I need your eyes on, the rest is refactoring on an existing integration test. thank you |
|
Can we separate the bug fix out into its own PR? |
|
I thought about it too, but it gets kind of ugly. If you say its a must do, I will, of course. |
|
Why is it ugly? Would it help to merge the fix before the refactor (or the other way around)? |
|
hmm, you're probably right. Let me try that |
3 participants
No description provided.