Added support for openSUSE MicroOS #5998

andre161292 · 2023-02-16T09:23:37Z

SUMMARY

Added condition to check for /usr/sbin/transactional-update binary, which is the way to go for openSUSE MicroOS.

Fixes #5615

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

plugins/modules/zypper.py

ansibullbot · 2023-02-16T09:27:56Z

cc @AnderEnder @alxgu @andytom @commel @dcermak @evrardjp @lrupp @sealor @toabctl
click here for bot help

… to support microos closes #5615

commel · 2023-02-17T06:43:03Z

Looks good to me, thank you!

shipit

felixfontein

Just a tiny editorial change for the changelog fragment :)

changelogs/fragments/5615-zypper-transactional-update.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

andre161292 · 2023-02-17T07:13:06Z

tiny editorial change

Done :)

Akasurde · 2023-02-17T10:50:43Z

plugins/modules/zypper.py

@@ -512,7 +512,8 @@ def repo_refresh(m):


 def transactional_updates():
-    return os.path.exists('/var/lib/misc/transactional-update.state')
+    return os.path.exists('/var/lib/misc/transactional-update.state') \


Suggested change

return os.path.exists('/var/lib/misc/transactional-update.state') \

return any(map(os.path.exists, ['/var/lib/misc/transactional-update.state', '/usr/sbin/transactional-update']))

another way to write this

I assume the suggested change makes the boolean operation non-trivial anymore, in the way that it allocates an array and has to reflectively apply the list items to the supplied function. For more items i'd be with you, but for two..?

Thx for the suggestion nevertheless! Phyton's not my native language and i'm happy to learn about new ways to do stuff :)

On Python 3 it the map result is a generator, but on Python 2 this causes two calls to os.path.exists, while the original statement only makes two calls if the first one returns False.

But using a generator expression would make it rather efficient (though still slower than the original version) even on Python 2.6 and 2.7: any(os.path.exists(path) for path in ('/var/lib/misc/transactional-update.state', '/usr/sbin/transactional-update')).

Thank you for the explanation 🤗 Makes sense.

So should i change it, or leave it as is? What's preferred?

Python 2 is dead and is probably irrelevant if something there is slower than the supported release. What does /var/lib/misc/transactional-update.state contain? And is the system in a valid state if this file is missing?

Hi, thanks for pinging me.

Is the pure existence of the executable enough to check if transactional updates are actually in use? It may be true for MicroOS, which only exists in the transaction-variant. In openSUSE (e.g. Tumbleweed) I can as well install transactional-update and have the executable on the disk. That does not mean I actually activated transactional updates. That's the reason why I checked for the state file.

If there's a better way to check if transactional updates are really in use (and not only "possible"), I'm for the change. But not when we break the usage of the module on other systems.

Is there a way to identify MicroOS? Maybe looking at /etc/os-release or /etc/lsb-release?

@felixfontein in my MicroOS installation there is no /etc/lsb-release file, but /etc/os-release contains:

NAME="openSUSE MicroOS" # VERSION="20230130" ID="opensuse-microos" ID_LIKE="suse opensuse opensuse-tumbleweed" VERSION_ID="20230130" PRETTY_NAME="openSUSE MicroOS" ANSI_COLOR="0;32" CPE_NAME="cpe:/o:opensuse:microos:20230130" BUG_REPORT_URL="https://bugs.opensuse.org" HOME_URL="https://www.opensuse.org/" DOCUMENTATION_URL="https://en.opensuse.org/Portal:MicroOS" LOGO="distributor-logo-MicroOS"

Maybe looking at /etc/os-release or /etc/lsb-release?

It's id is opensuse-microos, so yes.
But we shouldn't only rely on that, as the state-file doesn't exist on a transactional Tumbleweed/Leap install.

I think we should replicate the checks zypper uses, if suitable. And it doesn't seem to use black magic, so we should be able to do so: https://github.com/openSUSE/zypper/blob/master/src/commands/conditions.cc#L44

andre161292 · 2023-02-22T10:04:43Z

I just pushed an update that checks if the filesystem is btrfs and readonly and contains the binary. Only if all of these are true, the transactional-update method will be used.

Still, there's one caveat: I don't quite know if it's okay to "just use" the pip dependency psutil in here. Where's the correct location to document it, or should i replace it (if there's an alternative)?

~~Edit: I guess the CI answered at least part of my question. :)~~

…g /proc/mount

plugins/modules/zypper.py

commel · 2023-02-22T13:55:33Z

Looks good to me! Thank you for getting into detail.

felixfontein · 2023-02-22T14:15:22Z

I'll merge this this weekend if nobody objects.

commel · 2023-02-22T14:22:28Z

I'm fine here.
shipit

sebix · 2023-02-22T14:44:35Z

Thank you @andre161292 for your time and efforts!

andre161292 · 2023-02-22T15:40:22Z

You're welcome!

patchback · 2023-02-25T09:58:08Z

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/2c762c475386b0eb322e685716219750e3dc3dd4/pr-5998

Backported as #6077

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* fix(zypper): Added condition to check for transactional-update binary to support microos closes #5615 * style(changelog): Made zypper-change uppercase Co-authored-by: Felix Fontein <felix@fontein.de> * fix(zypper): Removed check for /var/lib/misc/transactional-update.state * feat(zypper): Aligned transactional-update checks with zypper's * refactor(zypper): Removed dependency to psutil and made use of parsing /proc/mount * refactor(zypper): Removed need for regex, plus small refactoring --------- Co-authored-by: André Dörscheln <ad@itesign.de> Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit 2c762c4)

patchback · 2023-02-25T09:58:17Z

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/2c762c475386b0eb322e685716219750e3dc3dd4/pr-5998

Backported as #6078

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* fix(zypper): Added condition to check for transactional-update binary to support microos closes #5615 * style(changelog): Made zypper-change uppercase Co-authored-by: Felix Fontein <felix@fontein.de> * fix(zypper): Removed check for /var/lib/misc/transactional-update.state * feat(zypper): Aligned transactional-update checks with zypper's * refactor(zypper): Removed dependency to psutil and made use of parsing /proc/mount * refactor(zypper): Removed need for regex, plus small refactoring --------- Co-authored-by: André Dörscheln <ad@itesign.de> Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit 2c762c4)

felixfontein · 2023-02-25T09:58:57Z

@andre161292 thanks a lot for your contribution!
@commel @sebix @bartmichu thank you all for reviewing and commenting!

…roOS (#6077) Added support for openSUSE MicroOS (#5998) * fix(zypper): Added condition to check for transactional-update binary to support microos closes #5615 * style(changelog): Made zypper-change uppercase Co-authored-by: Felix Fontein <felix@fontein.de> * fix(zypper): Removed check for /var/lib/misc/transactional-update.state * feat(zypper): Aligned transactional-update checks with zypper's * refactor(zypper): Removed dependency to psutil and made use of parsing /proc/mount * refactor(zypper): Removed need for regex, plus small refactoring --------- Co-authored-by: André Dörscheln <ad@itesign.de> Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit 2c762c4) Co-authored-by: andre161292 <andre161292@users.noreply.github.com>

…roOS (#6078) Added support for openSUSE MicroOS (#5998) * fix(zypper): Added condition to check for transactional-update binary to support microos closes #5615 * style(changelog): Made zypper-change uppercase Co-authored-by: Felix Fontein <felix@fontein.de> * fix(zypper): Removed check for /var/lib/misc/transactional-update.state * feat(zypper): Aligned transactional-update checks with zypper's * refactor(zypper): Removed dependency to psutil and made use of parsing /proc/mount * refactor(zypper): Removed need for regex, plus small refactoring --------- Co-authored-by: André Dörscheln <ad@itesign.de> Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit 2c762c4) Co-authored-by: andre161292 <andre161292@users.noreply.github.com>

github-actions · 2023-02-25T10:49:35Z

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

* fix(zypper): Added condition to check for transactional-update binary to support microos closes ansible-collections#5615 * style(changelog): Made zypper-change uppercase Co-authored-by: Felix Fontein <felix@fontein.de> * fix(zypper): Removed check for /var/lib/misc/transactional-update.state * feat(zypper): Aligned transactional-update checks with zypper's * refactor(zypper): Removed dependency to psutil and made use of parsing /proc/mount * refactor(zypper): Removed need for regex, plus small refactoring --------- Co-authored-by: André Dörscheln <ad@itesign.de> Co-authored-by: Felix Fontein <felix@fontein.de>

ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor os packaging plugins plugin (any type) labels Feb 16, 2023

fix(zypper): Added condition to check for transactional-update binary…

821d030

… to support microos closes #5615

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Feb 17, 2023

felixfontein reviewed Feb 17, 2023

View reviewed changes

changelogs/fragments/5615-zypper-transactional-update.yml Outdated Show resolved Hide resolved

style(changelog): Made zypper-change uppercase

4924bfd

Co-authored-by: Felix Fontein <felix@fontein.de>

Akasurde reviewed Feb 17, 2023

View reviewed changes

André Dörscheln added 2 commits February 20, 2023 10:11

fix(zypper): Removed check for /var/lib/misc/transactional-update.state

a880100

feat(zypper): Aligned transactional-update checks with zypper's

db0b4bd

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Feb 22, 2023

This comment was marked as outdated.

Sign in to view

refactor(zypper): Removed dependency to psutil and made use of parsin…

9763f52

…g /proc/mount

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Feb 22, 2023

felixfontein reviewed Feb 22, 2023

View reviewed changes

plugins/modules/zypper.py Outdated Show resolved Hide resolved

plugins/modules/zypper.py Outdated Show resolved Hide resolved

refactor(zypper): Removed need for regex, plus small refactoring

1003df2

felixfontein merged commit 2c762c4 into ansible-collections:main Feb 25, 2023

patchback bot mentioned this pull request Feb 25, 2023

[PR #5998/2c762c47 backport][stable-5] Added support for openSUSE MicroOS #6077

Merged

patchback bot mentioned this pull request Feb 25, 2023

[PR #5998/2c762c47 backport][stable-6] Added support for openSUSE MicroOS #6078

Merged

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Feb 25, 2023

andre161292 deleted the bugfix/5615-transactional-updates-microos branch April 3, 2023 11:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for openSUSE MicroOS #5998

Added support for openSUSE MicroOS #5998

andre161292 commented Feb 16, 2023

ansibullbot commented Feb 16, 2023

commel commented Feb 17, 2023

felixfontein left a comment

andre161292 commented Feb 17, 2023

Akasurde Feb 17, 2023

andre161292 Feb 17, 2023

felixfontein Feb 17, 2023

andre161292 Feb 18, 2023

commel Feb 18, 2023

sebix Feb 20, 2023

felixfontein Feb 21, 2023

bartmichu Feb 21, 2023

andre161292 Feb 21, 2023

andre161292 Feb 21, 2023

andre161292 commented Feb 22, 2023 •

edited

Loading

This comment was marked as outdated.

commel commented Feb 22, 2023

felixfontein commented Feb 22, 2023

commel commented Feb 22, 2023

sebix commented Feb 22, 2023

andre161292 commented Feb 22, 2023

patchback bot commented Feb 25, 2023 •

edited

Loading

patchback bot commented Feb 25, 2023 •

edited

Loading

felixfontein commented Feb 25, 2023

github-actions bot commented Feb 25, 2023

	return os.path.exists('/var/lib/misc/transactional-update.state') \
	return any(map(os.path.exists, ['/var/lib/misc/transactional-update.state', '/usr/sbin/transactional-update']))

Added support for openSUSE MicroOS #5998

Added support for openSUSE MicroOS #5998

Conversation

andre161292 commented Feb 16, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

ansibullbot commented Feb 16, 2023

commel commented Feb 17, 2023

felixfontein left a comment

Choose a reason for hiding this comment

andre161292 commented Feb 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andre161292 commented Feb 22, 2023 • edited Loading

This comment was marked as outdated.

commel commented Feb 22, 2023

felixfontein commented Feb 22, 2023

commel commented Feb 22, 2023

sebix commented Feb 22, 2023

andre161292 commented Feb 22, 2023

patchback bot commented Feb 25, 2023 • edited Loading

Backport to stable-5: 💚 backport PR created

patchback bot commented Feb 25, 2023 • edited Loading

Backport to stable-6: 💚 backport PR created

felixfontein commented Feb 25, 2023

github-actions bot commented Feb 25, 2023

Docs Build 📝

andre161292 commented Feb 22, 2023 •

edited

Loading

patchback bot commented Feb 25, 2023 •

edited

Loading

patchback bot commented Feb 25, 2023 •

edited

Loading