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

feat: warn for signs of external mender installation #744

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TheMeaningfulEngineer
Copy link

mender-convert doesn't handle well if the input image has Mender installed already.
This could happen if someone did the installation of the Debian Mender package and then attempted to mender-convert that image.

This introduces a function that will detect detect the potential issue to aid troubleshooting from the logs once the error is hit.

With it you will see:

2025-02-18 18:06:44 [WARN] [mender-convert-modify] work/rootfs/var/lib/mender-monitor exists!
2025-02-18 18:06:44 [WARN] [mender-convert-modify] Input image contain traces Mender of previous Mender installation
2025-02-18 18:06:44 [WARN] [mender-convert-modify] The conversion might fail. Please provide a clean input image.

Extra WARN context is above
Below is old

2025-02-18 17:53:29 [INFO] [mender-convert-modify] Performing platform specific pre-modifications (if any)
2025-02-18 17:53:29 [INFO] [mender-convert-modify] Running hook: platform_pre_modify
2025-02-18 17:53:29 [INFO] [mender-convert-modify] Creating state folder in the data partition for Mender  add-ons
mender-convert-modify has finished. Cleaning up...
2025-02-18 17:53:29 [ERROR] [mender-convert] mender-convert failed
2025-02-18 17:53:29 [DEBUG] [mender-convert-modify] When running: (./mender-convert-modify:174): run_and_
log_cmd():

        sudo ln -sf /data/mender work/rootfs/var/lib
        ln: work/rootfs/var/lib/mender: cannot overwrite directory

2025-02-18 17:53:29 [ERROR] [mender-convert] mender-convert failed
2025-02-18 17:53:29 [ERROR] [mender-convert] mender-convert exit code: 1

External Contributor Checklist

🚨 Please review the guidelines for contributing to this repository.

  • Make sure that all commits follow the conventional commit specification for the Mender project.

The majority of our contributions are fixes, which means your commit should have
the form below:

fix: <SHORT DESCRIPTION OF FIX>

<OPTIONAL LONGER DESCRIPTION>

Changelog: <USER-FRIENDLY-CHANGE-DESCRIPTION> or <None>
Ticket: <TICKET NUMBER> or <None>
  • Make sure that all commits are signed with git --signoff. Also note that the signoff author must match the author of the commit.

Description

Please describe your pull request.

Thank you!

@mender-test-bot
Copy link
Contributor

@TheMeaningfulEngineer, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline".


my commands and options

You can trigger a pipeline on multiple prs with:

  • mentioning me and start pipeline --pr mender/127 --pr mender-connect/255

You can start a fast pipeline, disabling full integration tests with:

  • mentioning me and start pipeline --fast

You can trigger GitHub->GitLab branch sync with:

  • mentioning me and sync

You can cherry pick to a given branch or branches with:

  • mentioning me and:
 cherry-pick to:
 * 1.0.x
 * 2.0.x

@mender-test-bot
Copy link
Contributor

Merging these commits will result in the following changelog entries:

Changelogs

mender-convert (probe-tainted-input-image)

New changes in mender-convert since master:

Features
  • warn for signs of external mender installation

@TheMeaningfulEngineer TheMeaningfulEngineer force-pushed the probe-tainted-input-image branch 2 times, most recently from 01a77f9 to 3a30dc1 Compare February 20, 2025 14:21
mender-convert doesn't handle well if the input image has Mender
installed already.
This could happen if someone did the installation of the Debian Mender package
and then attempted to mender-convert that image.

This introduces a function that will detect detect the potential issue
to aid troubleshooting from the logs once the error is hit.

With it you will see:

```
2025-02-18 18:06:44 [WARN] [mender-convert-modify] work/rootfs/var/lib/mender-monitor exists!
2025-02-18 18:06:44 [WARN] [mender-convert-modify] Input image contain traces Mender of previous Mender installation
2025-02-18 18:06:44 [WARN] [mender-convert-modify] The conversion might fail. Please provide a clean input image.

Extra WARN context is above
Below is old

2025-02-18 17:53:29 [INFO] [mender-convert-modify] Performing platform specific pre-modifications (if any)
2025-02-18 17:53:29 [INFO] [mender-convert-modify] Running hook: platform_pre_modify
2025-02-18 17:53:29 [INFO] [mender-convert-modify] Creating state folder in the data partition for Mender  add-ons
mender-convert-modify has finished. Cleaning up...
2025-02-18 17:53:29 [ERROR] [mender-convert] mender-convert failed
2025-02-18 17:53:29 [DEBUG] [mender-convert-modify] When running: (./mender-convert-modify:174): run_and_
log_cmd():

        sudo ln -sf /data/mender work/rootfs/var/lib
        ln: work/rootfs/var/lib/mender: cannot overwrite directory

2025-02-18 17:53:29 [ERROR] [mender-convert] mender-convert failed
2025-02-18 17:53:29 [ERROR] [mender-convert] mender-convert exit code: 1
```

Changelog: title
Ticket: none

Signed-off-by: Alan <[email protected]>
Copy link
Contributor

@danielskinstad danielskinstad left a comment

Choose a reason for hiding this comment

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

Nice, generally it looks good, thank you 🚀
Some small nits for typos, and a suggestion for the probing.

@@ -13,6 +13,29 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# Checks there is no preexisting mender integration on the golden image
#
# mender-convert expects a vanilla image wihtout mender installed by other means.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# mender-convert expects a vanilla image wihtout mender installed by other means.
# mender-convert expects a vanilla image without mender installed by other means.

@@ -13,6 +13,29 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# Checks there is no preexisting mender integration on the golden image
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Checks there is no preexisting mender integration on the golden image
# Checks if there is a preexisting mender integration on the golden image

Comment on lines +24 to +36
directories=(
"work/rootfs/var/lib/mender-monitor"
"work/rootfs/var/lib/mender-configure"
"work/data/mender"
)

for dir in "${directories[@]}"; do
if [ -e "$dir" ]; then
log_warn "$dir exists!"
log_warn "Input image contain traces Mender of previous Mender installation"
log_warn "The conversion might fail. Please provide a clean input image."
fi
done
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest something like this instead to avoid hardcoding the paths:

Suggested change
directories=(
"work/rootfs/var/lib/mender-monitor"
"work/rootfs/var/lib/mender-configure"
"work/data/mender"
)
for dir in "${directories[@]}"; do
if [ -e "$dir" ]; then
log_warn "$dir exists!"
log_warn "Input image contain traces Mender of previous Mender installation"
log_warn "The conversion might fail. Please provide a clean input image."
fi
done
directories="$(sudo find work/ -type d -name "mender*")"
## None of these should be present in a clean rootfs
if [ -n "$directories" ]; then
log_warn "Found following directories:\n$directories"
log_warn "Input image contains traces of Mender"
log_warn "The conversion might fail. Please provide a clean input image."
fi

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.

3 participants