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

fix: scheduler init data dir #1430

Closed
wants to merge 3 commits into from

Conversation

mikeee
Copy link
Member

@mikeee mikeee commented Jul 19, 2024

Description

Creates the directory to mount by default

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1416

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee requested review from a team as code owners July 19, 2024 00:13
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 21.83%. Comparing base (ddf43a5) to head (c0d30fa).
Report is 3 commits behind head on release-1.14.

Files Patch % Lines
pkg/standalone/standalone.go 0.00% 11 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-1.14    #1430      +/-   ##
================================================
- Coverage         21.94%   21.83%   -0.11%     
================================================
  Files                40       40              
  Lines              4913     4937      +24     
================================================
  Hits               1078     1078              
- Misses             3754     3778      +24     
  Partials             81       81              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yaron2
Copy link
Member

yaron2 commented Jul 19, 2024

cc @JoshVanL

@mikeee
Copy link
Member Author

mikeee commented Jul 19, 2024

Appears to work on mac but is having issues on Linux. Converting to draft

@mikeee mikeee marked this pull request as draft July 19, 2024 07:57
Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee marked this pull request as ready for review July 19, 2024 09:48
Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee mentioned this pull request Jul 19, 2024
3 tasks
@JoshVanL
Copy link
Contributor

JoshVanL commented Jul 19, 2024

It's very rare that a user would want to create local directory which is mounted into scheduler. There is very little the user can do with the data as it's raw ETCD & proto. Container runtime local volumes are the canonical approach, which are still accessible via the filesystem, but have no user permission issues- we are even doing a 777 here which we never want to be doing.

If a user did want to do this for whatever reason- they can per-create the local volume to a local relative path and use the --scheduler-volume to mount that.

@mikeee mikeee marked this pull request as draft July 19, 2024 11:02
@artursouza artursouza closed this Jul 19, 2024
@mikeee mikeee deleted the fix-scheduler-init branch July 19, 2024 23:12
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.

4 participants