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

Migrate update logic to Debian package #1076

Merged
merged 11 commits into from
Mar 2, 2022
Merged

Migrate update logic to Debian package #1076

merged 11 commits into from
Mar 2, 2022

Conversation

mrpalide
Copy link
Contributor

@mrpalide mrpalide commented Jan 31, 2022

Did you run make format && make check? Yes

Fixes #1070

Changes:

  • Remove old logic, based on downloading compressed version from github release page
  • Add new logic to update from Debian package repository

How to test this PR:

  • Burn fresh skybian image on SD card
  • Clone this PR, and build binaries by GOOS=linux GOARCH=arm64 GOARM=6 make build BUILDTAG=linux_arm64. (I use arm64 because of my board RPi4, you should use your own board architect).
  • Replace SD binaries files with new built binaries. Following files should replaced in /usr/bin:
    • skywire-cli
    • skywire-visor
    • setup-node
    • apps/*
  • Start board
  • Open hypervisor UI and Click on update
  • After complete and restarting service, check version of visor, should be 0.5.1.

Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

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

I would also like to move the config migration code to the debian package.

u.log.Warnf("%v is not found in update, skipping", binary)
return nil
func (u *Updater) addRepo() error {
if err := exec.Command("sudo", "add-apt-repository", "deb http://176.9.28.105 sid main").Run(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the domain here? It is deb.skywire.skycoin.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mh, I used URLs in wiki page that Moses created. I'll change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


if _, err := io.Copy(out, resp.Body); err != nil {
return "", err
func (u *Updater) restartBoard() {
Copy link
Member

Choose a reason for hiding this comment

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

Is a restart needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I thought, based on logic of Moses Debian package. I'll check it with him again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I change it to restart service, not the board at all.

@0pcom
Copy link
Collaborator

0pcom commented Jan 31, 2022

I would also like to move the config migration code to the debian package.

@jdknives you are referring to issue #1070?
Is there something I am missing with the current skywire-autoconfig script? The configs are generated or regenerated on every update.

@mrpalide why are we doing repository configuration here? Configuring the repository initially is probably not desirable for users if it was not already configured. They probably downloaded it directly.

the apt command itself will tell you that it is not stable to use in this way..

    WARNING : apt does not have a stable CLI interface. Use with caution in scripts.

In the second place
here:
https://github.com/mrpalide/skywire/blob/88fd12196f3b45aa0fea98c7a2a73cf746da167d/pkg/util/updater/updater.go#L132

nothing else will run after that. The function won't return anything, the process will be killed. A new instance may start but there is no way to know and not a good way to test.

https://github.com/mrpalide/skywire/blob/88fd12196f3b45aa0fea98c7a2a73cf746da167d/pkg/util/updater/updater.go#L125

you are uninstalling the program that is currently executing this command. the process will be killed.

@mrpalide mrpalide changed the title [WIP] Migrate update logic to Debian package Migrate update logic to Debian package Feb 3, 2022
Copy link
Contributor

@ersonp ersonp left a comment

Choose a reason for hiding this comment

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

The code looks good. Can't test it as I have misplaced my RPi.

@jdknives jdknives merged commit 5182761 into skycoin:develop Mar 2, 2022
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.

4 participants