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.

Sign up for GitHub

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

Jump to bottom

[ws-daemon] Support config reload for IO limits #9440

Merged
merged 1 commit into from
Apr 20, 2022
Merged

[ws-daemon] Support config reload for IO limits #9440

merged 1 commit into from
Apr 20, 2022

Conversation

csweichel
Copy link
Contributor

Description

This PR adds support for hot config reloading of the IO bandwidth limiting in ws-daemon.

How to test

  • change the config
  • check if the v1 IO limits change

Release Notes

NONE

@csweichel csweichel marked this pull request as ready for review April 20, 2022 23:36
@csweichel csweichel requested a review from a team April 20, 2022 23:36
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Apr 20, 2022
@roboquat roboquat merged commit 8dd76ed into main Apr 20, 2022
@roboquat roboquat deleted the cw/dmn-cfg-reload branch April 20, 2022 23:38
@sagor999
Copy link
Contributor

why not use fsnotify to watch config file for changes?

@kylos101 kylos101 mentioned this pull request Apr 21, 2022
Closed
@csweichel
Copy link
Contributor Author

csweichel commented Apr 21, 2022 •
edited by werft-gitpod-dev-com bot
Loading

Kubernetes configMap changes aren't just a file modification, but a delete/move of a symlink. This is hard to keep track of using fsnotify. That said, 💯 agree: fsnotify is the way to go.

We're not using fsnotify here because:

  • we wanted to get this change out, and registry-facade has proven working code which we took over,
  • in registry-facade prior attempts at making this work with fsnotify had failed, hence this implementation.

@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aledbf aledbf aledbf approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@csweichel @sagor999 @aledbf @roboquat