Read actual system configs during k8s upgrade #579
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Because under k8s the actual real system is mounted into a dir, the configs that we are reading by default are not the ones in the system but the ones on the upgrade container, which is running under / This patch introduces a check to see if we are running under k8s, and if we are it tries to get the host dir where the system is mounted. then it will prepend that host dir to the config scan so we can actually scan the system configs. Int he case we are out of k8s, this should not affect the upgrade as it will return an empty string resulting in the ususal dirs being scanned Signed-off-by: Itxaka <[email protected]>
Signed-off-by: Itxaka <[email protected]>
thanks tests Signed-off-by: Itxaka <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #579 +/- ##
==========================================
- Coverage 49.96% 49.94% -0.03%
==========================================
Files 48 48
Lines 4653 4655 +2
==========================================
Hits 2325 2325
- Misses 2051 2054 +3
+ Partials 277 276 -1 ☔ View full report in Codecov by Sentry. |
jimmykarily
reviewed
Oct 16, 2024
pkg/config/spec.go
Outdated
| hostDir = "/host" | ||
| } | ||
| hostDir := k8sutils.GetHostDirForK8s() | ||
| config.Logger.Logger.Debug().Bool("status", underKubernetes).Str("hostdir", hostDir).Msg("Running under kubernetes host directory") |
There was a problem hiding this comment.
There is no if anymore. This will always read "Running under kubernetes host directory" no? Or is Bool() a conditions on whether to print this or not?
There was a problem hiding this comment.
yes, indeed, I will change the msg.
this is just to show the values of the kubernetes and hostdirectory for debug purposes as it can happen that its doing it worngly and we have no visibility onto it
jimmykarily
reviewed
Oct 16, 2024
pkg/utils/k8s/common.go
Outdated
| return "/host" | ||
| } | ||
| } else { | ||
| // We return an empty string so any filepath.join does nto alter the paths |
There was a problem hiding this comment.
Suggested change
| // We return an empty string so any filepath.join does nto alter the paths | |
| // We return an empty string so any filepath.join does not alter the paths |
jimmykarily
approved these changes
Oct 16, 2024
Signed-off-by: Itxaka <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because under k8s the actual real system is mounted into a dir, the configs that we are reading by default are not the ones in the system but the ones on the upgrade container, which is running under /
This patch introduces a check to see if we are running under k8s, and if we are it tries to get the host dir where the system is mounted. then it will prepend that host dir to the config scan so we can actually scan the system configs.
Int he case we are out of k8s, this should not affect the upgrade as it will return an empty string resulting in the ususal dirs being scanned
Fixes kairos-io/kairos#2944 and probably others lol