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 custom binds set as RW_PATHS #944

Merged
merged 6 commits into from
Feb 22, 2023
Merged

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented Feb 22, 2023

RW_PATHS are meant for overlay dirs which go away after a reboot. Custom binds/binds are mounted under COS_PERSISTENT, so they persist after reboot AND are RW by default.

This patch adds custom binds into the PERSISTENT_STATE_PATHS and ephemeral into RW_PATHS

Also stops adding custom values into the cos-layout.env file as they are not needed

Also removes the existing keys to re-add them with the new merged values, otherwise we could have duplicated keys, which should not matter as the last one wins, but still its ugly and could cause problems in the future.

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

RW_PATHS are meant for overlay dirs which go away after a reboot.
Custom binds/binds are mounted under COS_PERSISTENT, so they persist
after reboot AND are RW by default.

This patch removes adding the custom binds into the RW_PATHS on the
cos-layout file as that can lead to unintended consequences

Signed-off-by: Itxaka <[email protected]>
@Itxaka Itxaka requested review from oz123 and a team February 22, 2023 10:43
@netlify
Copy link

netlify bot commented Feb 22, 2023

Deploy Preview for kairos-io canceled.

Name Link
🔨 Latest commit d4fd8d5
🔍 Latest deploy log https://app.netlify.com/sites/kairos-io/deploys/63f61deaaf82ac0007452605

mauromorales
mauromorales previously approved these changes Feb 22, 2023
Copy link
Member

@mauromorales mauromorales left a comment

Choose a reason for hiding this comment

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

LGTM

@Itxaka
Copy link
Member Author

Itxaka commented Feb 22, 2023

This also sets the defaults for recovery to be sure that we have a proper cos-layout file AND prevents the custom layout for running on anything that its not active/passive

@Itxaka
Copy link
Member Author

Itxaka commented Feb 22, 2023

Umm this still doesnt fix the bind mounts being set properly

@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2023

Codecov Report

Merging #944 (5a4ae74) into master (498b8ec) will not change coverage.
The diff coverage is n/a.

❗ Current head 5a4ae74 differs from pull request most recent head 265e1a9. Consider uploading reports for the commit 265e1a9 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master     #944   +/-   ##
=======================================
  Coverage   22.79%   22.79%           
=======================================
  Files          22       22           
  Lines        1610     1610           
=======================================
  Hits          367      367           
  Misses       1179     1179           
  Partials       64       64           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Itxaka <[email protected]>
@mudler mudler merged commit 31d9491 into kairos-io:master Feb 22, 2023
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