Skip to content

[ws-manager] Explicitly proxy image-builder #9714

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

Merged
merged 1 commit into from
May 10, 2022
Merged

Conversation

csweichel
Copy link
Contributor

Description

The prior implementation used a generic gRPC proxy package to make ws-manager proxy to image-builder. This package failed in some edge cases which broke image builds.

In this PR we explicitly proxy image-builder.
Hope is that this is a short-lived workaround and we'll get to move the image build to the workspace side properly soon enough.

Release Notes

NONE

@csweichel csweichel requested a review from a team May 3, 2022 09:41
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label May 3, 2022
@jenting jenting changed the title [ws-manager] Explicitely proxy image-builder [ws-manager] xplicitly proxy image-builder May 3, 2022
@jenting jenting changed the title [ws-manager] xplicitly proxy image-builder [ws-manager] Explicitly proxy image-builder May 3, 2022
comparable
}

func forwardStream[R ProtoMessage](ctx context.Context, recv func() (R, error), send func(R) error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

🏅

@princerachit
Copy link
Contributor

/hold

Copy link
Contributor

@princerachit princerachit left a comment

Choose a reason for hiding this comment

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

Code changes look good to me but I don't see anything related to testing so putting this on hold. Has this been tested @csweichel

@kylos101
Copy link
Contributor

kylos101 commented May 3, 2022

/hold We've decided to delay shipping this to production this week, therefore please do not remove the hold.

@csweichel
Copy link
Contributor Author

/hold cancel

Merging this chance is ok because no-one is using it yet.

@roboquat roboquat merged commit 59c10c6 into main May 10, 2022
@roboquat roboquat deleted the cw/fix-imgbldr-proxy branch May 10, 2022 10:32
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/L team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants