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.

Sign up for GitHub

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

Offer rollback option for interactive upgrading #11771

Merged
merged 19 commits into from
Feb 23, 2024
Merged

Offer rollback option for interactive upgrading #11771

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
894bc9a
Add rollback mechanism
nicok1 Jan 19, 2024
5f2fd7e
Fix non-interactive shell script behavior and improve error handling
nicok1 Jan 19, 2024
ae2e96c
correcting path
nicok1 Jan 19, 2024
87f42d6
Change preinstall.sh
nicok1 Jan 19, 2024
ac0ecf0
Small fix
nicok1 Jan 20, 2024
ce620d4
Change Line Endings from CRLF to LF
nicok1 Jan 21, 2024
23ab8fe
Apply suggestions from code review
nicok1 Jan 21, 2024
1b20af6
Small bugfixes
nicok1 Jan 24, 2024
518d01f
Update postinstall.sh script to provide more context when restoring t…
nicok1 Jan 24, 2024
0643027
Improve Indentation
nicok1 Jan 25, 2024
8e65a13
Applying suggestion from Review
nicok1 Jan 25, 2024
f7d687e
Revert "Improve Indentation"
nicok1 Jan 25, 2024
17f583a
Fix Indentation
nicok1 Jan 25, 2024
60fce98
Moved Code to (old)-postrm
nicok1 Jan 26, 2024
151376e
Removed Testcode
nicok1 Jan 26, 2024
9825043
Use dpkgs Error unwind and remove /tmp/oldevcc mechanism
nicok1 Jan 27, 2024
9790c8b
Improve code structure
nicok1 Jan 27, 2024
02b00e7
Restore old preinstall
nicok1 Jan 27, 2024
b97771c
Small correction
nicok1 Jan 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Revert "Improve Indentation" 
This reverts commit 0643027.
  • Loading branch information
@nicok1
nicok1 committed Jan 25, 2024
commit f7d687ea0b9cc30de7e1a66552ae65b3bcf53767
4 changes: 4 additions & 0 deletions .goreleaser.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@ builds:
- CGO_ENABLED=0
goos:
- linux
- darwin
- windows
goarch:
- amd64
- arm
- arm64
goarm:
- "6"
ignore:
Expand Down
108 changes: 0 additions & 108 deletions .goreleaser.yml.bak
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ ifeq ($(RELEASE),1)
COMMIT :=
endif
VERSION := $(if $(TAG_NAME),$(TAG_NAME),$(SHA))

VERSION :=0.123.10-next
BUILD_DATE := $(shell date -u '+%Y-%m-%d_%H:%M:%S')
BUILD_TAGS := -tags=release
LD_FLAGS := -X github.com/evcc-io/evcc/server.Version=$(VERSION) -X github.com/evcc-io/evcc/server.Commit=$(COMMIT) -s -w
Expand All @@ -29,9 +27,6 @@ PACKAGES = ./release
GOROOT := $(shell go env GOROOT)
CURRDIR := $(shell pwd)

deb::
goreleaser --snapshot --skip-publish --rm-dist --release-notes "Dummy release notes"

default:: ui build

all:: clean install install-ui ui assets lint test-ui lint-ui test build
Expand Down
73 changes: 37 additions & 36 deletions packaging/scripts/postinstall.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,44 +24,45 @@ if [ -f /tmp/oldevcc ] && [ $INTERACTIVE -eq 1 ]; then
checkConfigOutput=$(/usr/bin/evcc checkconfig 2>&1 || true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the point of the OR with true here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your feedback. The modification to this line is intended to prevent the package manager from halting the script execution if the subcommand returns a non-zero exit code. By appending '|| true'.

I have run some tests on the behaviour of the package manager that we can discuss (as soon as I have more time). This may result in a few more changes.

Copy link
Collaborator

@GrimmiMeloni GrimmiMeloni Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

OK interesting - especially the part about the subcommand / subshell RC influencing postinstall.sh. I would fail to see how postinstall.sh can even know about any RC from the variable substitution as it does not capture it. But I'll take your word for it.
Feel free to mark this as resolved and to ping me again on this subject once you had the time to fiddle around more, and want to bounce around ideas.

oldevccversion=$(/tmp/oldevcc -v)
newevccversion=$(/usr/bin/evcc -v)
if echo "$checkConfigOutput" | grep -q "config valid"; then
if [ "$oldevccversion" = "$newevccversion" ]; then
echo "--------------------------------------------------------------------------------"
echo "Old evcc version detected. To apply the new version, please reinstall evcc. (e.g. apt-get reinstall evcc)"
echo "--------------------------------------------------------------------------------"
else
if echo "$checkConfigOutput" | grep -q "config valid"; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

if [ "$checkConfigOutput" = "config valid" ]

Copy link
Member

Choose a reason for hiding this comment

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

Solche Änderungen kannst Du auch direkt inline zum applien vorschlagen ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes no sense at this point. If successful, the checkconfig command does not return exactly "config valid", but also dynamic information such as the version number. It should therefore be checked for contains and not for equals.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry about that. I checked what the command does, but apparently I did not look close enough. My bad.


if [ "$oldevccversion" = "$newevccversion" ]; then
echo "--------------------------------------------------------------------------------"
echo "Old evcc version detected. To apply the new version, please reinstall evcc. (e.g. apt-get reinstall evcc)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is somewhat confusing.
The check is for old and new version being equal, yet, the log message claims and old version was detected.
What does "old version" in this context mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I understand that. To explain it: If the version in /tmp/oldevcc is the same as /usr/bin/evcc, then a rollback was done before and then the apt-get install command was used (again). In order to really get the latest binary after correcting the config, either a newer release must be installed with apt-get install, or the current release must be reinstalled with apt-get reinstall. Perhaps you have an idea for a more suitable feedback?

Here is an example of dpkg -s evcc after a rollback:

klein@NPC:~/evcc$ dpkg -s evcc
Package: evcc
Status: install ok half-configured
Priority: optional
Installed-Size: 78064
Maintainer: andig <cpuidle@gmx.de>
Architecture: amd64
Version: 0.123.9~next ----> But Binary is 0.123.8
Config-Version: 0.123.8 
Description: EV Charge Controller
Homepage: https://evcc.io

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now I'm even more confused. 😅
Does the above dpkg output indicate, that the rollback does not actually roll back, in the sense that while it did restore the old version (0.123.8), the package manager now thinks 0.123.9 is already installed?
If so, that sounds like a total nightmare/deal breaker. 😢

echo "--------------------------------------------------------------------------------"
else
rm -rf /tmp/oldevcc
fi
fi
else
if [ "$oldevccversion" = "$newevccversion" ]; then
echo "--------------------------------------------------------------------------------"
echo "Old evcc version detected. To apply the new version, please reinstall evcc. (e.g. apt-get reinstall evcc)"
echo "--------------------------------------------------------------------------------"
else
echo "--------------------------------------------------------------------------------"
echo "ERROR: your evcc configuration is not compatible with the new version. Please consider reading the release notes: https://github.com/evcc-io/evcc/releases"
echo "checkconfig Output:"
echo "$checkConfigOutput"
echo "--------------------------------------------------------------------------------"

while true; do
echo "Do you want to keep your old (working) evcc version? [Y/n]: "
read choice
case "$choice" in
n*|N*|"")
echo "We will keep the new version. Your evcc configuration stays untouched!"
break
;;
y*|Y*)
echo "The old version will be restored. Your evcc configuration stays untouched! Consider reinstalling the new version after fixing your configuration. (e.g. apt-get reinstall evcc)"
cp -r /tmp/oldevcc /usr/bin/evcc
failInstallation=1
break
;;
*)
;;
esac
done
if [ "$oldevccversion" = "$newevccversion" ]; then
echo "--------------------------------------------------------------------------------"
echo "Old evcc version detected. To apply the new version, please reinstall evcc. (e.g. apt-get reinstall evcc)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.
Also - any chance to deduplicate?

echo "--------------------------------------------------------------------------------"
else
echo "--------------------------------------------------------------------------------"
echo "ERROR: your evcc configuration is not compatible with the new version. Please consider reading the release notes: https://github.com/evcc-io/evcc/releases"
echo "checkconfig Output:"
echo "$checkConfigOutput"
echo "--------------------------------------------------------------------------------"
while true; do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Either this while-block is incorrectly indented, or it is accidentally added to the else branch above. (Or Github UI is messing up my view...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The indentation issue has been resolved. However, I can confirm that the placement of the loop is correct.

echo "Do you want to keep your old (working) evcc version? [Y/n]: "
read choice
case "$choice" in
n*|N*|"")
echo "We will keep the new version. Your evcc configuration stays untouched!"
break
;;
y*|Y*)
echo "The old version will be restored. Your evcc configuration stays untouched! Consider reinstalling the new version after fixing your configuration. (e.g. apt-get reinstall evcc)"
cp -r /tmp/oldevcc /usr/bin/evcc
failInstallation=1
break
;;
*)
;;
esac
done
fi
fi
fi
Expand Down