Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(devnet): add substrate docker images to dockerfile #2263
feat(devnet): add substrate docker images to dockerfile #2263
Changes from 30 commits
6139712
8c3fb1d
c0be6dc
eda4e4a
dae59e2
6ccf8fb
1407b3a
0d59fac
5108412
9fa7ffb
71a8f5e
f1fea2d
9b485f6
cff9c8c
262b329
012b4b9
1d158a3
f0689ee
604ce4b
237647a
ae0429e
843d361
bcb134e
579136c
c49bcf0
f03093a
25946ca
e8c1e9d
15f1377
94c6a87
17fd1f2
aacb438
a382561
ca63faa
a4bd83c
e476e23
e12686d
03c89c6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we running prometheus in this docker-compose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added Prometheus while testing the exported metric data from authority nodes, I decided to keep it in the same file to easily start a Prometheus server with authorities nodes, should I remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the difference between the
gssmr
chain that's within our codebase and this one?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both have 3 authority nodes, however, the
cross-client
genesis file is accepted by the substrate node since it hasstaking
andsession
json keys whilegssmr
chain hasBabe
andGrandpa
json keys. Thecross-client
genesis file has the runtime version 09.10 that don't trigger errors like:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we should have one chain all cross client, and gossamer only setups. We may end up having multiple of these chains that differ based on number of authority nodes.
@kishansagathiya is working on secondary vrf slot mode for babe #2307. We need to ensure that the genesis file is using this mode and we should be updating to v0.9.16 of polkadot.
It would be nice to have some sort of readme or something in the devnet readme that describes how to generate these files. I know you were following the steps provided by Kishan, but where did the original genesis-spec.json come from? What pallets are in the runtime? These are questions we should have answers to. It may make sense to have another repo and associated github CI workflows to generate these genesis files that the devnets can pull down as part of the docker image build process. Rather than just putting arbitrary files in this repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
./polkadot build-spec --disable-default-bootnode --dev > genesis-spec.json
And then you will have to modify it according to your requirements, like adding authorities.
And then you run something like this to get a genesis.json file
./polkadot build-spec --chain genesis-spec.json --raw --disable-default-bootnode > genesis.json
Will make a pr to add this to read me.
New versions have a different genesis-spec file than 0.9.10.
Also running cross-client dev net with newer version of polkadot gives runtime errors. So, yeah we need to update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we rename this to something more descriptive.
3-auth-node-0.9.10
or something.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also we could have
0.9.10
as a build argument at the top global scope in the Dockerfile.i.e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qdm12 I was having problems trying to use arguments declared before
FROM
, following this issue (link) I could resolve and apply the suggestion.Basically, all the
ARG
's declared/initialized beforeFROM
needs to be redeclared afterFROM
to use its values otherwise will not be possible to get the value fromARG
. I your suggestion we need to addThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what port is prometheus running on?
update-dd-agent-config
is currently expecting the prometheus endpoint to behttp://127.0.0.1:9876/metrics
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed substrate nodes to expose Prometheus at port 9876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What port does substrate uses? Shouldn't we use the standard
9090
port for Gossamer as well 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by default the port 9090 is the one Prometheus docker image prom/prometheus uses to bootstrap its own server to collect metrics. I just standardized the port which substrate and gossamer will expose metrics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but by default what's the port substrate/polkadot uses for Prometheus? We force it to
9876
here it seems. I'm thinking we should change it in Gossamer (for end users experience).As far as I know, applications usually have their Prom registry server listening on
9090
, so maybe we want to use that instead? Although if polkadot uses 9876 for whatever weird reason, then sure we can match it as well I guess.