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

don't fail on clusters with missing proxies #4060

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

fspmarshall
Copy link
Contributor

Allows clusters to be loaded by web UI even if no proxies are currently known.

An error during the loading of a single cluster can cause the whole web UI to err out. Auth server version defaults to "" if no auth servers can currently be loaded, but errors were still being generated if no proxies could be loaded. This PR allows the proxy address and version to also default to "" if they are not loadable.

Fixes #4005

Copy link
Contributor

@awly awly left a comment

Choose a reason for hiding this comment

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

Can we use some placeholder strings, like unknown?

@fspmarshall
Copy link
Contributor Author

Can we use some placeholder strings, like unknown?

I'm hesitant to put placeholder messages in potentially meaningful values, esp since things like unknown are technically valid hostnames. @kimlisa is this something that would be easy to handle at the frontent? Provide some kind of placeholder or message when some cluster info is missing?

@kimlisa
Copy link
Contributor

kimlisa commented Jul 16, 2020

Can we use some placeholder strings, like unknown?

I'm hesitant to put placeholder messages in potentially meaningful values, esp since things like unknown are technically valid hostnames. @kimlisa is this something that would be easy to handle at the frontent? Provide some kind of placeholder or message when some cluster info is missing?

I agree with @fspmarshall that any value can be a valid hostname.

For the UI handling, @alex-kovoy what do you think? I think it is okay to leave these fields blank. Missing proxy isn't the only scenario that leaves it blank, clusters using older proxies will also display blank (b/c these fields don't exist on versions <4.3). In the client side, we can maybe put a placeholder like "not available"?

What it looks like if proxy url and version returns empty:
image
image

@alex-kovoy
Copy link
Contributor

@kimlisa

In the client side, we can maybe put a placeholder like "not available"?

Handling of empty strings on UI side needs to be discussed separately. It needs a broader context. As for this ticket, I agree with Forest, let webapi return an empty string.

@russjones
Copy link
Contributor

russjones commented Jul 16, 2020

@fspmarshall Just a heads up, I just ran into this issue. This line took a few minutes to show up and until then I was getting the same error as above.

DEBU [PROXY:1]   List of known proxies updated: ["7b0db878-ca20-4d70-bcee-3252a327145f"]. services/proxywatcher.go:148

Shouldn't this occur right away at startup? I think this would also cause other problems in term of cluster registering themseleves after process restart.

I think that's what' we're seeing here.

@russjones russjones self-requested a review July 16, 2020 21:16
Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

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

@fspmarshall will create another ticket to address a potential underlying issue causing this.

@fspmarshall fspmarshall force-pushed the fspmarshall/fix-cluster-loading branch from 818f4cc to bf32f05 Compare July 17, 2020 18:41
@fspmarshall fspmarshall merged commit b500262 into master Jul 17, 2020
@fspmarshall fspmarshall deleted the fspmarshall/fix-cluster-loading branch July 17, 2020 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error with trusted cluster prevents accessing
5 participants