-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix/740 #1067
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
Fix/740 #1067
Conversation
|
|
||
| private final ResourceEventHandler<V1ConfigMap> handler = new ResourceEventHandler<>() { | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just renames here - nothing else
| private final ResourceEventHandler<V1Secret> handler = new ResourceEventHandler<>() { | ||
|
|
||
| @Override | ||
| public void onAdd(V1Secret obj) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renames only
| @Override | ||
| public Mono<Void> triggerRefresh(KubernetesObject configMap) { | ||
| return busRefreshTrigger.triggerRefresh(configMap); | ||
| public Mono<Void> triggerRefresh(KubernetesObject configMap, String appName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can have multiple apps based in a configMap/secret now
| */ | ||
|
|
||
| package org.springframework.cloud.kubernetes.configuration.watcher; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the main changes are in this class.
| <artifactId>spring-cloud-kubernetes-client-configmap-event-reload-multiple-apps</artifactId> | ||
| <packaging>pom</packaging> | ||
|
|
||
| <modules> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an integration test where two apps are deployed, one configmap that has the new annotation in place: that refresh both of the apps
| }, refreshDelay, TimeUnit.MILLISECONDS); | ||
| Set<String> apps = apps(kubernetesObject, annotationName); | ||
|
|
||
| if (!apps.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the crox of the change is here: if the annotation that says we should restart multiple apps is present - try to get apps from it, and send events based on those names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could simplify this a bit if we add the name to the set if its empty
| <artifactId>spring-cloud-kubernetes-client-secrets-event-reload-multiple-apps</artifactId> | ||
| <packaging>pom</packaging> | ||
|
|
||
| <modules> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two apps that react as a result of a secret being deployed with proper label and annotation. Just proves that we expect both applications to be updated
|
|
||
| If a change is made to a ConfigMap or Secret with valid labels then Spring Cloud Kubernetes Configuration Watcher will take the name of the ConfigMap or Secret | ||
| and send a notification to the application with that name. | ||
| and send a notification to the application with that name. This might not be enough for your use-case though, you could for example what to: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added documentation here for the new properties
|
@ryanjbaxter ready to be looked at |
| }, refreshDelay, TimeUnit.MILLISECONDS); | ||
| Set<String> apps = apps(kubernetesObject, annotationName); | ||
|
|
||
| if (!apps.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could simplify this a bit if we add the name to the set if its empty
|
@ryanjbaxter take a look please to see if I got your point correct. thank you |
No description provided.