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

Kubernetes and Registry fixes #7368

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
5 changes: 3 additions & 2 deletions pkg/kubernetes/scripts/app.js
Expand Up @@ -198,7 +198,6 @@
* Openshift:
* - Have Project objects
* - Project objects are listable by any user, only accessilbe returned
* - Project objects are not watchable
*
* Kubernetes and Openshift
* - Namespace objects are only accessible to all users
Expand All @@ -210,8 +209,10 @@

var promise = discoverSettings().then(function(settings) {
var ret = [];
if (settings.flavor === "openshift")
if (settings.flavor === "openshift") {
ret.push(loader.watch("projects", $rootScope));
Copy link
Member

Choose a reason for hiding this comment

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

Does this require a certain minimal Kubernetes version to work? This watch isn't being used (yet) in our code (unless the statusPhase("Active") from the next commit relies on that?), so our tests wouldn't notice if that fails?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the watch populates the loader automatically, so the data will be used, if that's what you meant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the watch fails the loader loads the projects and the functionality should be the same as it is currently.

Having the watch just better helps the UI pick up changes that we were sometimes missing before and hopefully will help clean up some of the flakes.

Copy link
Member

Choose a reason for hiding this comment

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

OK. It felt a bit like an opportunistic thing that we neither rely on nor test, but it seems that's pretty much exactly its intention :-) At least I don't see how it could make matters worse.

ret.push(loader.load("projects"));
}
if (settings.isAdmin)
ret.push(loader.watch("namespaces", $rootScope));
return $q.all(ret);
Expand Down
1 change: 1 addition & 0 deletions pkg/kubernetes/scripts/kube-client.js
Expand Up @@ -487,6 +487,7 @@
link = decodeURIComponent(resourcePath([resource]));
if (drain[i].type == "DELETED") {
delete objects[link];
delete present[link];
removed[link] = resource;
} else if (drain[i].checkResourceVersion) {
/* There is a race between items loaded from
Expand Down