Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Add update service node page for docker in Lisk Service v0.7.0 #1850

Merged

Conversation

chris529
Copy link
Contributor

@chris529 chris529 commented Sep 14, 2023

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Merging #1850 (68b7ab3) into release/0.7.0 (ccb2964) will decrease coverage by 0.09%.
Report is 96 commits behind head on release/0.7.0.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                @@
##           release/0.7.0    #1850      +/-   ##
=================================================
- Coverage          66.90%   66.82%   -0.09%     
=================================================
  Files                300      298       -2     
  Lines               5140     5112      -28     
  Branches             927      922       -5     
=================================================
- Hits                3439     3416      -23     
+ Misses              1701     1696       -5     

see 6 files with indirect coverage changes

@chris529 chris529 changed the base branch from release/0.7.0 to 1751-add-steps-to-import-DB-snapshots September 18, 2023 13:06
@chris529 chris529 changed the base branch from 1751-add-steps-to-import-DB-snapshots to release/0.7.0 September 18, 2023 13:07
This reverts commit 7307f65.
sameersubudhi
sameersubudhi approved these changes Sep 28, 2023
@chris529 chris529 force-pushed the 1570-add-update-service-node-page-for-docker branch from 9daf6e2 to 25a3308 Compare October 5, 2023 12:26
== Start

.Inside the lisk-service root folder
----
cp ./docker/example.env .env
Copy link
Contributor

Choose a reason for hiding this comment

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

but this is not about starting it for the first time, it is about how to start it in general. I don't think you need to do it every time you start Lisk Service. So it shouldn't be mentioned here

@@ -12,10 +12,23 @@ Mona Bärenfänger <[email protected]>
:url_snapshot_config: configuration/index.adoc
:url_backups: management/snapshots.adoc

NOTE: To upgrade Lisk Service to a desired version, please check out the specific version with *`git checkout <tag>`*, and then create the Docker image, followed by starting Lisk Service as shown in the commands below.
Copy link
Contributor

Choose a reason for hiding this comment

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

How to update Lisk Service should not be a note, it should at least be a separate section, and every step necessary to update Lisk Service should be explained it that very section, and supported, if possible, by code snippets the user can simply copy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, there is now a section showing how to update Lisk Service 👍

@chris529 chris529 requested a review from Tschakki October 17, 2023 14:44
Copy link
Contributor

@sameersubudhi sameersubudhi left a comment

Choose a reason for hiding this comment

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

Looks good overall. However, please consider the following recommendations as well.

docs/antora/modules/ROOT/pages/management/docker.adoc Outdated Show resolved Hide resolved
docs/antora/modules/ROOT/pages/management/docker.adoc Outdated Show resolved Hide resolved
Copy link
Contributor

@Tschakki Tschakki left a comment

Choose a reason for hiding this comment

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

Imho it would be beneficial to have separate pages for the update guides, as initially intended, as the process includes several steps and also there are multiple options how to update.

Also, should we include steps to backup the existing data before the update, or is that not necessary?


To upgrade the Docker container to a desired version, please follow one of the two options below. To find all the tagged versions for Lisk Service, please check the available tags (https://github.com/LiskHQ/lisk-service/tags) on GitHub.

==== Option A - Download pre-built images from DockerHub
Copy link
Contributor

Choose a reason for hiding this comment

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

Option A should be the recommended way to upgrade Lisk Service, imho Option A and B should be switched, because B looks easier

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, imho it would be nice to use tabs to switch between Option A & B

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I have now added tabs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, imho it would be nice to use tabs to switch between Option A & B

I'd prefer that Download pre-built images from DockerHub remain the first option. Building images locally can be problematic at times if the dependencies are stale and we did not publish an update for any reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

docs/antora/modules/ROOT/pages/management/docker.adoc Outdated Show resolved Hide resolved

==== Option B - Build images locally

Build the images locally using the following steps.
Copy link
Contributor

Choose a reason for hiding this comment

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

At what point does it describe how to download and install the new version?

Copy link
Contributor

@sameersubudhi sameersubudhi Oct 19, 2023

Choose a reason for hiding this comment

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

Seems like the step to run git checkout <tag> in between the Stop Lisk Service and Build the required updated Docker images steps are missing. Have left a suggestion 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.

Done as suggested 👍

@chris529 chris529 requested a review from Tschakki October 19, 2023 11:47
Copy link
Contributor

@Tschakki Tschakki left a comment

Choose a reason for hiding this comment

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

Thank you! 🎉

@sameersubudhi sameersubudhi merged commit 2ca2af5 into release/0.7.0 Oct 20, 2023
7 of 8 checks passed
@sameersubudhi sameersubudhi deleted the 1570-add-update-service-node-page-for-docker branch October 20, 2023 13:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants