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

Zeitgeist Client Update Automation #944

Merged
merged 11 commits into from
Jul 20, 2023
Merged

Zeitgeist Client Update Automation #944

merged 11 commits into from
Jul 20, 2023

Conversation

samuelarogbonlo
Copy link
Contributor

@samuelarogbonlo samuelarogbonlo commented Jan 12, 2023

This should resolve (#924).

This PR contains the automation script to auto-update the zeitgeist clients on the self-hosted servers and OnFinality. @sea212 Can you possibly take a look at this whenever you can? Thanks.

Copy link
Member

@Chralt98 Chralt98 left a comment

Choose a reason for hiding this comment

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

Thanks @samuelarogbonlo !

Impressed by ChatGPTs response:

The most important mistake to fix would likely be the lack of error handling and validation. This script does not check for errors that might occur during the download or execution of the software, which can make it difficult to troubleshoot issues if they arise. Additionally, it does not validate if the variable RELEASE_VERSION is set or not, which can cause issues if not set. This can result in the script trying to download the wrong version of the software or trying to download a version that does not exist.

Another important mistake would be the lack of compatibility and security checks. The script does not check if the new version of Zeitgeist software is compatible with the current system, or if the new version of the software is signed and verified or not. This could potentially lead to issues if the software is not compatible or if the new version is not verified and could be a security risk.

Lastly, the script does not check if the new version of Zeitgeist software is already installed or not, and does not check if the new version of Zeitgeist software is tested before applying the update. This could lead to unnecessary updates and could cause issues if the new version is not fully tested.

gather_facts: no
become: true
environment:
RELEASE_VERSION: "{{ lookup('env', 'RELEASE_VERSION') }}"
Copy link
Member

Choose a reason for hiding this comment

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

Where do we set RELEASE_VERSION? In the environment variables? What happens, if it is not set? Can we print the RELEASE_VERSION in the Github Actions Debug Console?

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 RELEASE_VERSION is automatically set by the Github client-auto-update.yml workflow as an environment variable. So when the playbook is ran in, it looks for environment variable already set by the Github workflow. I may have to include some logic so the workflow can run if RELEASE_VERSION is not specified, just to be on the safer side.

@Chralt98 Chralt98 requested a review from sea212 January 12, 2023 13:54
@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2023

Codecov Report

Merging #944 (10f2e2d) into main (85cefa6) will decrease coverage by 0.98%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #944      +/-   ##
==========================================
- Coverage   95.67%   94.69%   -0.99%     
==========================================
  Files          90       92       +2     
  Lines       18779    19267     +488     
==========================================
+ Hits        17967    18245     +278     
- Misses        812     1022     +210     
Flag Coverage Δ
tests 94.69% <ø> (-0.99%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
zrml/court/src/mock.rs 36.84% <0.00%> (-24.93%) ⬇️
zrml/simple-disputes/src/mock.rs 10.00% <0.00%> (-13.53%) ⬇️
zrml/authorized/src/mock.rs 40.90% <0.00%> (-12.94%) ⬇️
zrml/authorized/src/lib.rs 82.35% <0.00%> (-12.10%) ⬇️
zrml/court/src/lib.rs 87.71% <0.00%> (-6.43%) ⬇️
zrml/simple-disputes/src/lib.rs 77.77% <0.00%> (-4.05%) ⬇️
zrml/prediction-markets/src/migrations.rs 90.69% <0.00%> (-1.59%) ⬇️
zrml/prediction-markets/src/lib.rs 90.56% <0.00%> (-1.41%) ⬇️
zrml/prediction-markets/src/weights.rs 93.96% <0.00%> (-1.16%) ⬇️
primitives/src/market.rs 70.49% <0.00%> (-0.35%) ⬇️
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@sea212 sea212 left a comment

Choose a reason for hiding this comment

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

Main needs to be merged into this branch.
How can we test this before merging? Maybe by adding manual dispatch and executing it.

scripts/ansible/client-auto-update-playbook.yml Outdated Show resolved Hide resolved
.github/workflows/client-auto-update.yml Outdated Show resolved Hide resolved
.github/workflows/docker-hub-parachain.yml Outdated Show resolved Hide resolved
.github/workflows/docker-hub-parachain.yml Outdated Show resolved Hide resolved
Comment on lines 34 to 40
- name: Create Backup from Previous Zeitgeist Client
no_log: true
shell: |
cd /services/zeitgeist/bin
PREV_VERSION=$(./zeitgeist --version | grep -Eo '([0-9]{1,}\.)+[0-9]{1,}' | tr -d '.')
mv zeitgeist zeitgeist_$PREV_VERSION.bak
when: not mnt.stat.exists and mnt.stat.isdir
Copy link
Member

Choose a reason for hiding this comment

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

We might want to remove old backups here to avoid bloating.

Comment on lines 28 to 32
shell: |
cd /mnt/*/services/zeitgeist/bin
PREV_VERSION=$(./zeitgeist --version | grep -Eo '([0-9]{1,}\.)+[0-9]{1,}' | tr -d '.')
mv zeitgeist zeitgeist_$PREV_VERSION.bak
when: mnt.stat.exists and mnt.stat.isdir
Copy link
Member

Choose a reason for hiding this comment

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

We might want to remove old backups here to avoid bloating.

- name: Download Zeitgeist Client to Mount Dir
no_log: true
shell: |
cd /mnt/*/services/zeitgeist/bin
Copy link
Member

Choose a reason for hiding this comment

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

This only works as long as there are not two folder trees of the following form:
/mnt/<folder1>/services
/mnt/<folder2>/services

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 for now there's no more than one sub-directory in the /mnt/ directory. These sub-directories contain the parachain and relay-chain data mounted in volumes for each zeitgeist node.

if [[ -v RELEASE_VERSION ]]; then
wget -O zeitgeist https://github.com/zeitgeistpm/zeitgeist/releases/download/$RELEASE_VERSION/zeitgeist_parachain
else
wget -O zeitgeist https://github.com/zeitgeistpm/zeitgeist/releases/download/v0.3.6/zeitgeist_parachain
Copy link
Member

Choose a reason for hiding this comment

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

v0.3.6 is bugged.

Suggested change
wget -O zeitgeist https://github.com/zeitgeistpm/zeitgeist/releases/download/v0.3.6/zeitgeist_parachain
wget -O zeitgeist https://github.com/zeitgeistpm/zeitgeist/releases/download/v0.3.4/zeitgeist_parachain

@sea212 sea212 added the s:revision-needed The pull requests must be revised label Jan 25, 2023
@sea212 sea212 added this to the v0.3.9 milestone Jan 25, 2023
@sea212 sea212 added s:review-needed The pull request requires reviews and removed s:revision-needed The pull requests must be revised labels Feb 9, 2023
@sea212 sea212 removed this from the v0.3.9 milestone Jun 22, 2023
@sea212 sea212 added this to the v0.4.0 milestone Jul 5, 2023
@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2023

This pull request is now in conflicts. Could you fix it @samuelarogbonlo? 🙏

@sea212 sea212 added s:in-progress The pull requests is currently being worked on s:revision-needed The pull requests must be revised and removed s:review-needed The pull request requires reviews s:in-progress The pull requests is currently being worked on labels Jul 7, 2023
@sea212 sea212 added s:blocked The pull requests awaits resolution of dependencies (state what it depends on) and removed s:revision-needed The pull requests must be revised labels Jul 14, 2023
@sea212
Copy link
Member

sea212 commented Jul 14, 2023

Blocked: Waiting for secrets to be added to GH

@sea212 sea212 modified the milestones: v0.3.10, v0.3.11 Jul 20, 2023
@sea212 sea212 added s:review-needed The pull request requires reviews and removed s:blocked The pull requests awaits resolution of dependencies (state what it depends on) labels Jul 20, 2023
@sea212 sea212 added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels Jul 20, 2023
@sea212 sea212 merged commit dedb812 into main Jul 20, 2023
20 checks passed
@sea212 sea212 deleted the client-auto-update branch July 20, 2023 23:05
@sea212 sea212 modified the milestones: v0.3.11, v0.3.10 Jul 20, 2023
@sea212 sea212 linked an issue Jul 21, 2023 that may be closed by this pull request
samuelarogbonlo added a commit that referenced this pull request Jul 24, 2023
sea212 pushed a commit that referenced this pull request Jul 25, 2023
fix: Set playbook name correctly (#944)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:accepted This pull request is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clients should be automatically upgraded on the servers
4 participants