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

(FACT-3023) xen command: avoid xen-toolstack warning #2357

Merged
merged 1 commit into from Apr 23, 2021

Conversation

lollipopman
Copy link
Contributor

xen-toolstack is deprecated and so on Puppet runs in Debian buster you
receive this nettlesome message:

warning: something called deprecated script /usr/lib/xen-common/bin/xen-toolstack

Prior to this change we used xen-toolstack if it is present, but that is
only necessary if more than one stack is installed. Instead check if we
have multiple tool stacks and if we do use xen-toolstack if it is
present.

@lollipopman lollipopman requested review from a team April 16, 2021 20:58
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@lollipopman lollipopman force-pushed the xen-toolstack-deprecated branch 3 times, most recently from ab9540b to 889f4dd Compare April 17, 2021 20:47
@GabrielNagy
Copy link
Contributor

Hi @lollipopman, thanks for the contribution! 🎉

I have a couple of questions, and I don't have much experience with Xen (and no hypervisor VM on hand), so please bear with me:

  • Is this warning shown on stdout/stderr when executing Facter without --debug? Normally, the execution API should redirect all external command output to debug.
  • I was able to find some context on this -- a thread on bugs.debian.org which mentions that:

The xm toolstack is long gone now, and xl is the only choice.

Initially I wanted to ask if there was an accepted way of getting the toolstack in case of multiple stacks, but from what I understand there's only xl now?

It would be good to update https://github.com/puppetlabs/facter/blob/main/lib/schema/facter.yaml#L1897 to reflect the new behavior.

Also, I opened FACT-3023 to track the work on this. Can you amend your commit title to include the ticket ID instead of maint?

Thanks again for your contribution!

@lollipopman
Copy link
Contributor Author

  • Is this warning shown on stdout/stderr when executing Facter without --debug? Normally, the execution API should redirect all external command output to debug.

The output goes to stderr, the full text of the script is:

#!/bin/sh
set -e
echo >&2 "warning: something called deprecated script $0"
exec xl "$@"
  • I was able to find some context on this -- a thread on bugs.debian.org which mentions that:

The xm toolstack is long gone now, and xl is the only choice.

This is mostly true, but it was still available in Debian Jessie, which has long term support until June of 2022, [1].

Initially I wanted to ask if there was an accepted way of getting the toolstack in case of multiple stacks, but from what I understand there's only xl now?

That is true now, but I assumed we wanted to be backward compatible, what are the compatibility guarantees for facter?

It would be good to update https://github.com/puppetlabs/facter/blob/main/lib/schema/facter.yaml#L1897 to reflect the new behavior.

Will do, once we agree upon the behavior

Also, I opened FACT-3023 to track the work on this. Can you amend your commit title to include the ticket ID instead of maint?

done!

Thanks again for your contribution!

Thank you!

@GabrielNagy
Copy link
Contributor

Yep @lollipopman I agree we should keep this backwards-compatible. The current approach of executing xen-toolstack only if multiple stacks are available seems fine to me.

xen-toolstack is deprecated and so on Puppet runs in Debian buster you
receive this nettlesome message:

  warning: something called deprecated script /usr/lib/xen-common/bin/xen-toolstack

Prior to this change we used xen-toolstack if it is present, but that is
only necessary if more than one stack is installed. Instead check if we
have multiple tool stacks and if we do use xen-toolstack if it is
present.
@lollipopman
Copy link
Contributor Author

thanks @GabrielNagy updated with your feedback, let me know if you want any other changes!

@GabrielNagy GabrielNagy changed the title bugfix xen command: avoid xen-toolstack warning (FACT-3023) xen command: avoid xen-toolstack warning Apr 23, 2021
@GabrielNagy GabrielNagy merged commit e4efb46 into puppetlabs:main Apr 23, 2021
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.

None yet

3 participants