Skip to content
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 #4496: removing list handling from watching #4506

Merged
merged 1 commit into from Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,7 @@
* Fix #4478: Removing the resourceVersion bump with null status
* Fix #4482: Fixing blocking behavior of okhttp log watch
* Fix #4487: Schema for multimaps is now generated correctly
* Fix #4496: Removing watch handling of lists

#### Improvements
* Fix #4471: Adding KubernetesClientBuilder.withHttpClientBuilderConsumer to further customize the HttpClient for any implementation.
Expand Down
Expand Up @@ -20,7 +20,6 @@
import com.fasterxml.jackson.databind.node.ObjectNode;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.KubernetesResource;
import io.fabric8.kubernetes.api.model.KubernetesResourceList;
import io.fabric8.kubernetes.api.model.ListOptions;
import io.fabric8.kubernetes.api.model.Status;
import io.fabric8.kubernetes.api.model.WatchEvent;
Expand All @@ -39,7 +38,6 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -250,33 +248,9 @@ private WatchEvent contextAwareWatchEventDeserializer(String messageSource)
}
}

protected WatchEvent readWatchEvent(String messageSource) throws JsonProcessingException {
WatchEvent event = contextAwareWatchEventDeserializer(messageSource);
KubernetesResource object = null;
if (event != null) {
object = event.getObject();
}
// when watching API Groups we don't get a WatchEvent resource
// so the object will be null
// so lets try parse the message as a KubernetesResource
// as it will probably be a list of resources like a BuildList
if (object == null) {
object = Serialization.unmarshal(messageSource, KubernetesResource.class);
if (event == null) {
event = new WatchEvent(object, "MODIFIED");
} else {
event.setObject(object);
}
}
Comment on lines -259 to -270
Copy link
Member

@manusa manusa Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you watch API Groups in a real cluster?

From yesterday's discussion, the assumption was that receiving a list as an event was not possible, but the former comment here says otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you watch API Groups in a real cluster?

I couldn't quite determine what that comment meant. You cannot watch APIGroup directly, and we have no example, including Builds, where a watch returns a list rather than a WatchEvent. My only guess is that this was from early behavior in OpenShift and the comment could be referencing not using oapi for the watch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I recall now you did provide that explanation.

if (event.getType() == null) {
event.setType("MODIFIED");
}
return event;
}

protected void onMessage(String message) {
try {
WatchEvent event = readWatchEvent(message);
WatchEvent event = contextAwareWatchEventDeserializer(message);
Object object = event.getObject();
Action action = Action.valueOf(event.getType());
if (action == Action.ERROR) {
Expand All @@ -291,21 +265,7 @@ protected void onMessage(String message) {
} else if (object instanceof HasMetadata) {
HasMetadata hasMetadata = (HasMetadata) object;
updateResourceVersion(hasMetadata.getMetadata().getResourceVersion());

if (object instanceof KubernetesResourceList) {
// Dirty cast - should always be valid though
@SuppressWarnings({ "rawtypes" })
KubernetesResourceList list = (KubernetesResourceList) hasMetadata;
@SuppressWarnings("unchecked")
List<HasMetadata> items = list.getItems();
if (items != null) {
for (HasMetadata item : items) {
eventReceived(action, item);
}
}
} else {
eventReceived(action, hasMetadata);
}
eventReceived(action, hasMetadata);
} else {
final String msg = String.format("Invalid object received: %s", message);
close(new WatcherException(msg, null, message));
Expand Down