explorer/machine_type breaks existing configurations #3

New Issue

ungleich-gitea · 2021-11-20T11:24:42Z

ungleich-gitea commented

2021-11-20 11:24:42 +00:00

As with !1010 to master, a new format is introduced that will break the existing usage of the explorer.

Output of the old explorer are:

physical
openvz
lxc
virtual_by_*
unknown

Now it will output up to multiple lines for each layer it detects in following format:

TYPE[ VERB VENDOR]

Which can result in the following output for example:

container
container on lxc
virtual by kvm

It's primarily not bad; through the new output we can gather more information about the state but the new format requires a new way to process it - e.g. something like [ "$(cat "$__explorer/machine_type")" = "lxc" ] isn't possible any more. We need rather something like this:

# this
if grep -Fwq "container" "$__explorer/machine_type"; then
  echo yay
fi

# or more excact
if grep -Fxq "container on lxc" "$__explorer/machine_type"; then
  echo more yay
fi

But we must think about how to migrate to the new format as gentle as possible. The easiest way is to write a new migration explorer (which outputs the breaking format), and after a while (at best at a major or after a longer time so they can migrate), the migration explorer replaces the old one but also links to the migration explorer name. So you have time to switch back to the original explorer name again.

This involves two steps and needs a core change to make it possible to deprecate explorers, but would be the gentle way of doing. Instead of this, we can also make a hard change where the user setup breaks and he must fix it immediately (hopefully with some notes to migrate). This should happen on a major release.

All effort will be useless if in the end I'm the only user of this type (and it already broke for me), but in the end you don't know how many out there uses this explorer, so it's probably a hard guessing game saying nobody uses it and it's wasted effort but I don't know neither :-)

As with !1010 to master, a new format is introduced that will break the existing usage of the explorer. Output of the old explorer are: - `physical` - `openvz` - `lxc` - `virtual_by_*` - `unknown` Now it will output up to multiple lines for each layer it detects in following format: > TYPE[ VERB VENDOR] Which can result in the following output for example: ``` container container on lxc virtual by kvm ``` It's primarily not bad; through the new output we can gather more information about the state but the new format requires a new way to process it - e.g. something like `[ "$(cat "$__explorer/machine_type")" = "lxc" ]` isn't possible any more. We need rather something like this: ```sh # this if grep -Fwq "container" "$__explorer/machine_type"; then echo yay fi # or more excact if grep -Fxq "container on lxc" "$__explorer/machine_type"; then echo more yay fi ``` But we must think about how to migrate to the new format as gentle as possible. The easiest way is to write a new migration explorer (which outputs the breaking format), and after a while (at best at a major or after a longer time so they can migrate), the migration explorer replaces the old one but also links to the migration explorer name. So you have time to switch back to the original explorer name again. This involves two steps and needs a core change to make it possible to deprecate explorers, but would be the gentle way of doing. Instead of this, we can also make a hard change where the user setup breaks and he must fix it immediately (hopefully with some notes to migrate). This should happen on a major release. All effort will be useless if in the end I'm the only user of this type (and it already broke for me), but in the end you don't know how many out there uses this explorer, so it's probably a hard guessing game saying nobody uses it and it's wasted effort but I don't know neither :-)

ungleich-gitea commented

2021-11-20 15:24:44 +00:00

thanks @ssrq, it just happened that I was completely unable to configure hosts cause I depend on the check if it's a container or not (input format is not relevant for this but must be known :-) ). It involved a bit time of searching till I found out with sh -x that there is now a multi-line string instead of just "lxc". And I didn't pulled master a longer time before, but did it cause I wanted to tinker a bit with cdist.

Your changes are good and I don't say anything against them, it's only the question how to handle that "API break". Normally, it happens at a major release or for bigger ones with a migration path. I understand that the change is small in means of affected users (but also don't know if that many use master instead of the latest version), which makes less sense for a "soft change". You can also make a backwards-compatible change (at least I could do it if I wouldn't have made an exact match), which could have been a problem if this change will be released as bugfix/minor release.

It's not about you forgot something, it's rather about such changes in general. That they should be handled more special. But I don't know if this "breaking change" wasn't considered that problematic or it's something more general, but it looks not bad as you can e.g. deprecate parameters for types. I was just curious of handling such changes as I've experienced it through this (and yes, I just had to change one line :P).

But in the end, we can check at which version (major/minor/bugfix) we should release this change and write some note (which isn't guaranteed that the affected user reads this before it happens) about this or a migration guide how to update. And I noticed with it that we don't have any documentation for global explorers except a list of them in the docs and the source itself. Looks like nobody has a problem with looking up the source ;-).

thanks @ssrq, it just happened that I was completely unable to configure hosts cause I depend on the check if it's a container or not (input format is not relevant for this but must be known :-) ). It involved a bit time of searching till I found out with `sh -x` that there is now a multi-line string instead of just "lxc". And I didn't pulled master a longer time before, but did it cause I wanted to tinker a bit with cdist. Your changes are good and I don't say anything against them, it's only the question how to handle that "API break". Normally, it happens at a major release or for bigger ones with a migration path. I understand that the change is small in means of affected users (but also don't know if that many use master instead of the latest version), which makes less sense for a "soft change". You can also make a backwards-compatible change (at least I could do it if I wouldn't have made an exact match), which could have been a problem if this change will be released as bugfix/minor release. It's not about you forgot something, it's rather about such changes in general. That they should be handled more special. But I don't know if this "breaking change" wasn't considered that problematic or it's something more general, but it looks not bad as you can e.g. deprecate parameters for types. I was just curious of handling such changes as I've experienced it through this (and yes, I just had to change one line :P). But in the end, we can check at which version (major/minor/bugfix) we should release this change and write some note (which isn't guaranteed that the affected user reads this before it happens) about this or a migration guide how to update. And I noticed with it that we don't have any documentation for global explorers except a list of them in the docs and the source itself. Looks like nobody has a problem with looking up the source ;-).

ungleich-gitea commented

2021-11-20 15:24:45 +00:00

Please excuse my late reaction to this issue (as the author of the rewritten explorer).

Yes, the rewritten explorer does change the output format. Saying that breaking the output format was "intentional" is probably the wrong way of phrasing it, but when I was drafting the rewrite of the explorer I came to the conclusion that trying to stay compatible with the old format would be complicated and limiting.
The old output was inconsistent (VMs were prefixed with virtual_by_ while containers had no prefix at all) and did not support describing nested chroot/container/VM combinations.
Moreover, when drafing the new output format I tried to design it in a way that allows further extensions without breaking well-behaved consumers of its output (e.g. if you want to know you run inside LXC you can test against ^container [^ ]* lxc, but if you have some special code for libvirt-lxc you can test against ^container [^ ]* lxc-libvirt$ specifically).

Processing the output is different, but I don't think it's that complicated.

If you just want to know what you are dealing with:

read -r machtype <"${__global:?}/explorer/machine_type"
case ${machtype}
in
	('chroot') ;;
	('container') ;;
	('physical') ;;
	('virtual machine') ;;
esac

If you want to test against a specific vendor, e.g. one of:

grep -q '^chroot' "${__global:?}/explorer/machine_type"
grep -q '^container [^ ]* lxc' "${__global:?}/explorer/machine_type"
grep -q '^container [^ ]* lxc-libvirt$' "${__global:?}/explorer/machine_type"
grep -q '^virtual [^ ]* kvm' "${__global:?}/explorer/machine_type"
grep -q '^virtual [^ ]* vmware' "${__global:?}/explorer/machine_type"

@matze The new explorer has been completed for almost two months, being in master for a month before you opened this issue.
With this being said, we cannot conclude that you are (apart from me…) the only user of this explorer, but I don't think it's heavily used, either.
It's certainly not used by any core types.

So, since migration will need to occur anyway (by reading the issue I take it that you are not opposing the rewritten explorer altogether) and a migration script to the old format would need to be written first, I'm suggesting that it makes more sense to adapt types to the new format than to invest possibly lots of time into a slower transition.

If you heavily depend on the old output, it would also be possible as a workaround, to copy the old code as an explorer into your .cdist or into your types.

Please excuse my late reaction to this issue (as the author of the rewritten explorer). Yes, the rewritten explorer does change the output format. Saying that breaking the output format was "intentional" is probably the wrong way of phrasing it, but when I was drafting the rewrite of the explorer I came to the conclusion that trying to stay compatible with the old format would be complicated and limiting. The old output was inconsistent (VMs were prefixed with `virtual_by_` while containers had no prefix at all) and did not support describing nested chroot/container/VM combinations. Moreover, when drafing the new output format I tried to design it in a way that allows further extensions without breaking well-behaved consumers of its output (e.g. if you want to know you run inside LXC you can test against `^container [^ ]* lxc`, but if you have some special code for libvirt-lxc you can test against `^container [^ ]* lxc-libvirt$` specifically). Processing the output is different, but I don't think it's that complicated. If you just want to know what you are dealing with: ```sh read -r machtype <"${__global:?}/explorer/machine_type" case ${machtype} in ('chroot') ;; ('container') ;; ('physical') ;; ('virtual machine') ;; esac ``` If you want to test against a specific vendor, e.g. one of: ```sh grep -q '^chroot' "${__global:?}/explorer/machine_type" grep -q '^container [^ ]* lxc' "${__global:?}/explorer/machine_type" grep -q '^container [^ ]* lxc-libvirt$' "${__global:?}/explorer/machine_type" grep -q '^virtual [^ ]* kvm' "${__global:?}/explorer/machine_type" grep -q '^virtual [^ ]* vmware' "${__global:?}/explorer/machine_type" ``` @matze The new explorer has been completed for almost two months, being in master for a month before you opened this issue. With this being said, we cannot conclude that you are (apart from me…) the only user of this explorer, but I don't think it's heavily used, either. It's certainly not used by any core types. So, since migration will need to occur anyway (by reading the issue I take it that you are not opposing the rewritten explorer altogether) and a migration script to the old format would need to be written first, I'm suggesting that it makes more sense to adapt types to the new format than to invest possibly lots of time into a slower transition. If you heavily depend on the old output, it would also be possible as a workaround, to copy the old code as an explorer into your `.cdist` or into your types.

ungleich-gitea commented

2021-11-20 15:24:47 +00:00

changed the description

Sign in to join this conversation.