-
Notifications
You must be signed in to change notification settings - Fork 880
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(wsl): Put back the "path" argument to wsl_path in ds-identify #5537
Conversation
Got swallowed by canonical@da6b5c4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CarlosNihelton code looks good and I can see the oversight in the original use of duplicated wslpath $params $1. Given the time it takes to setup and launch these instances for me on a paid account, can you attach the before/after of the /run/cloud-init/ds-identify.log
here to establish both failure path and success.
Before:
After the fix:
|
And augmenting the information we log with: diff --git a/tools/ds-identify b/tools/ds-identify
index f6a97461..9dd1ff3b 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -1776,7 +1776,7 @@ dscheck_WSL() {
# Then we can check for any .cloud-init folders for the user
if [ ! -d "$profile_dir/.cloud-init/" ] && [ ! -d "$profile_dir/.ubuntupro/.cloud-init/" ]; then
- debug 1 "No .cloud-init directories found"
+ debug 1 "No .cloud-init directories found in $profile_dir"
return "${DS_NOT_FOUND}"
fi
we would see
(notice the usage error message printed in place of the |
Excellent @CarlosNihelton LGTM. Please add the suspplemental parameterized log message anyway as that is a good breadcrumb anyway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One log parameterization is added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…nonical#5537) Got swallowed by https://github.com/canonical/cloud-init/pull/5116/commits/ da6b5c4 The former commit resulted in usage error from the wslpath command thus we never found WSL specific data, disabling cloud-init.
) Got swallowed by https://github.com/canonical/cloud-init/pull/5116/commits/ da6b5c4 The former commit resulted in usage error from the wslpath command thus we never found WSL specific data, disabling cloud-init.
Proposed Commit Message
Additional Context
Test Steps
datasources_list=[WSL, NoCloud] # could be something else,as long as there is one more real datasource meant to be not activated
/run/ds-identify.log
. It should contain the message "No .cloud-init directories found".We compute the
.cloud-init
directory correctly but when translating that into a Linux path usingwslpath
then things don't go well.Unfortunately that's a function built to be mocked, so I don't see how to properly test this in the current repository practices.
Merge type