-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 context propagation for bootstrap and server checks #8924
Conversation
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.
With the change it seems to provide the right environment context to the threads that effectively perform the workspace start.
And multi-tenant rh-che
on Che6 is not broken anymore.
So approving even if I didn't catch fully how environment context propagation works.
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.
LGTM
serversAndProbesFuture.whenComplete((ok, ex) -> serversReadyFuture.cancel(true)); | ||
} catch (InfrastructureException ex) { | ||
serversAndProbesFuture.completeExceptionally(ex); | ||
EnvironmentContext.setCurrent(context); |
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.
What if there would be a method that wraps anything with context setting. Would the code be simpler with less indentation (which leads to complexity of code reading)?
Example:
.thenCompose(setContext(context, bootstrap(toCancelFutures, machine)))
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.
sure, that makes sense, I'll think about that
9908a31
to
eefca12
Compare
eefca12
to
b56001f
Compare
What does this PR do?
After the merge of #8836, we got a problem with start of workspaces in rh-che. The reason for these failures is there are no token in environment context while a machine is bootstrapping. The absence of the token in context causes the kubernetes client to attempt to make requests on behalf of an anonymous user.