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

[workspacekit] improve logging when not able to connect to daemon #9951

Merged
merged 1 commit into from
May 12, 2022

Conversation

sagor999
Copy link
Contributor

Description

Improves logging to better understand when we fail to connect to daemon from workspacekit.

Related Issue(s)

Related to #9835

How to test

Release Notes

none

Documentation

@sagor999 sagor999 requested a review from a team May 11, 2022 22:52
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label May 11, 2022
@@ -397,7 +397,7 @@ var ring1Cmd = &cobra.Command{

client, err := connectToInWorkspaceDaemonService(ctx)
if err != nil {
log.WithError(err).Error("cannot connect to daemon")
log.WithError(err).Error("cannot connect to daemon from ring1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be ring2?

Copy link
Contributor

@utam0k utam0k May 12, 2022

Choose a reason for hiding this comment

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

I think it is correct because ring2 starts here as another process from ring1. This code calls itself (i.e., workspacekit) in a separate process. Double-fork is required before the container is created.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your kindly explanation @utam0k, attach the diagram for future reference
image

@utam0k
Copy link
Contributor

utam0k commented May 12, 2022

/hold until @jenting approve this PR

@utam0k
Copy link
Contributor

utam0k commented May 12, 2022

/werft run

👍 started the job as gitpod-build-pavel-ring-logging.1
(with .werft/ from main)

Copy link
Contributor

@jenting jenting left a comment

Choose a reason for hiding this comment

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

/lgtm

@jenting
Copy link
Contributor

jenting commented May 12, 2022

/unhold

@roboquat roboquat merged commit 791163b into main May 12, 2022
@roboquat roboquat deleted the pavel/ring-logging branch May 12, 2022 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none size/S team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants