Skip to content

Deterministic hash#731

Merged
NicolasMahe merged 45 commits intodevfrom
feature/determistic-hash
Mar 12, 2019
Merged

Deterministic hash#731
NicolasMahe merged 45 commits intodevfrom
feature/determistic-hash

Conversation

@NicolasMahe
Copy link
Copy Markdown
Member

@NicolasMahe NicolasMahe commented Jan 24, 2019

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
Copy Markdown
Member

antho1404 commented Jan 24, 2019

same hashes for me for the 3 deployment methods :)

krhubert
krhubert reviewed Jan 24, 2019
View reviewed changes
Comment thread service/service.go Outdated
@antho1404
Copy link
Copy Markdown
Member

antho1404 commented Jan 24, 2019

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
Copy Markdown
Member

mesg-bot commented Jan 25, 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/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
Comment thread service/service.go Outdated
@NicolasMahe NicolasMahe added the poc label Feb 1, 2019
@mesg-bot
Copy link
Copy Markdown
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
Comment thread service/namespace.go Outdated
@mesg-foundation mesg-foundation deleted a comment Feb 8, 2019
@krhubert krhubert force-pushed the feature/determistic-hash branch from de2d453 to 52af70e Compare February 8, 2019 12:22
@antho1404
Copy link
Copy Markdown
Member

antho1404 commented Mar 9, 2019

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
Comment thread service/service.go Outdated
NicolasMahe
NicolasMahe commented Mar 12, 2019
View reviewed changes
Comment thread service/service.go
@NicolasMahe NicolasMahe mentioned this pull request Mar 12, 2019
Closed
@NicolasMahe
Copy link
Copy Markdown
Member Author

NicolasMahe commented Mar 12, 2019

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 <krhubert@gmail.com>
@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
Copy Markdown
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
Copy Markdown
Member Author

NicolasMahe commented Mar 12, 2019

Oh yeah 🍾 🍾 🍾 🍾

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

mesg-bot commented Mar 15, 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/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

@antho1404 antho1404 antho1404 left review comments

+2 more reviewers

@krhubert krhubert krhubert approved these changes

@ilgooz ilgooz ilgooz approved these changes

Reviewers whose approvals may not affect merge requirements

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