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

Jump to bottom

Deterministic hash #731

Merged
merged 45 commits into from
Mar 12, 2019
Merged

Deterministic hash #731

merged 45 commits into from
Mar 12, 2019

Conversation

NicolasMahe
Copy link
Member

Implementation of https://forum.mesg.com/t/services-hash-are-different-from-downloaded-tarball-and-remote-git/211/5

Docker hash are not deterministic.

This PR use the archive to create the service hash.
It very simple are works well:

From github archive

./dev-cli service deploy https://github.com/mesg-foundation/service-ethereum/archive/master.tar.gz
✔ Service deployed with hash: da39a3ee5e6b4b0d3255bfef95601890afd80709

From local git

./dev-cli service dev /Users/nico/Developments/MESG/services/service-ethereum
✔ Service deployed with ID: da39a3ee5e6b4b0d3255bfef95601890afd80709

From remote git

./dev-cli service dev https://github.com/mesg-foundation/service-ethereum
✔ Service deployed with ID: da39a3ee5e6b4b0d3255bfef95601890afd80709

Please try the remote ones on your computer and tell me if you have the same or different hash!

@antho1404
Copy link
Member

same hashes for me for the 3 deployment methods :)

krhubert
krhubert reviewed Jan 24, 2019
View reviewed changes
service/service.go Outdated Show resolved Hide resolved
@antho1404
Copy link
Member

We need to add the deployment variables in this hash too.
The same service deployed will have one hash but if we deploy it again with different env variables this should generate a new hash.

2 solutions:

  • Add a .env/.envrc file in the archive during deployment (but we need to support to source this file in docker...)
  • find a way to hash the tar + the new env variables that override the mesg.yml

@mesg-bot
Copy link
Member

This pull request has been mentioned on MESG Community. There might be relevant details there:

https://forum.mesg.com/t/services-hash-are-different-from-downloaded-tarball-and-remote-git/211/6

@antho1404 antho1404 mentioned this pull request Jan 27, 2019
Merged
ilgooz
ilgooz previously approved these changes Jan 28, 2019
View reviewed changes
ilgooz
ilgooz reviewed Jan 28, 2019
View reviewed changes
service/service.go Outdated Show resolved Hide resolved
@NicolasMahe NicolasMahe added the poc label Feb 1, 2019
@mesg-bot
Copy link
Member

mesg-bot commented Feb 1, 2019

This pull request has been mentioned on MESG Community. There might be relevant details there:

https://forum.mesg.com/t/services-hash-are-different-from-downloaded-tarball-and-remote-git/211/7

@mesg-foundation mesg-foundation deleted a comment from mesg-bot Feb 1, 2019
NicolasMahe
NicolasMahe commented Feb 8, 2019
View reviewed changes
service/namespace.go Outdated Show resolved Hide resolved
@mesg-foundation mesg-foundation deleted a comment Feb 8, 2019
@antho1404
Copy link
Member

Did some fixes here #804 @mesg-foundation/core can you have a look

antho1404
antho1404 previously approved these changes Mar 12, 2019
View reviewed changes
krhubert
krhubert previously approved these changes Mar 12, 2019
View reviewed changes
NicolasMahe
NicolasMahe commented Mar 12, 2019
View reviewed changes
service/service.go Outdated Show resolved Hide resolved
@NicolasMahe NicolasMahe mentioned this pull request Mar 12, 2019
Closed
@NicolasMahe
Copy link
Member Author

I cannot with GitHub because i am the creator on the PR but I still approve :)

@NicolasMahe @krhubert
Remove comment
5467056 
Co-Authored-By: krhubert <[email protected]>
@krhubert krhubert dismissed stale reviews from antho1404 and themself via 5467056 March 12, 2019 08:30
ilgooz
ilgooz approved these changes Mar 12, 2019
View reviewed changes
Copy link
Contributor

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

code lgtm, manual tests are ok

@NicolasMahe NicolasMahe merged commit f6626fa into dev Mar 12, 2019
@NicolasMahe NicolasMahe deleted the feature/determistic-hash branch March 12, 2019 09:50
@NicolasMahe
Copy link
Member Author

Oh yeah 🍾 🍾 🍾 🍾

@NicolasMahe NicolasMahe mentioned this pull request Mar 14, 2019
Closed
@mesg-bot
Copy link
Member

This pull request has been mentioned on MESG Community. There might be relevant details there:

https://forum.mesg.com/t/services-hash-are-different-from-downloaded-tarball-and-remote-git/211/12

@antho1404 antho1404 mentioned this pull request Mar 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krhubert krhubert krhubert approved these changes

@ilgooz ilgooz ilgooz approved these changes

@antho1404 antho1404 antho1404 left review comments

Assignees

@krhubert krhubert

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@NicolasMahe @antho1404 @mesg-bot @krhubert @ilgooz