-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Update test-network chaincode container versions #1266
Conversation
@bestbeforetoday I noticed that this update changes the indentation in the YAML files from 2 spaces to 4. Is there a specific reason for this change? Other files appear to be consistently using 2 spaces, so it might be better to align with that convention for consistency. What do you think? |
I think this PR changes the indentation from 4 spaces to the same 2 spaces used in other Yaml files. |
@bestbeforetoday |
No problem! I didn't set out to change the whitespace intentionally. I just have my editor set to autoformat on save. If you are happy with the 2 space indentation, the changes are (much) easier to review if you set GitHub's diff view settings to hide whitespace. |
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.
@bestbeforetoday
The updates look good to me. I have corrected the indentation for some of the commented-out configuration values, so please review those adjustments.
# Parameters on creating docker container. | ||
# Container may be efficiently created using ipam & dns-server for cluster | ||
# NetworkMode - sets the networking mode for the container. Supported | ||
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | ||
# Dns - a list of DNS servers for the container to use. | ||
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | ||
# Docker Host Config are not supported and will not be used if set. | ||
# LogConfig - sets the logging driver (Type) and related options | ||
# (Config) for Docker. For more info, | ||
# https://docs.docker.com/engine/admin/logging/overview/ | ||
# Note: Set LogConfig using Environment Variables is not supported. | ||
hostConfig: |
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.
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. | |
hostConfig: | |
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. | |
hostConfig: |
hostConfig: | ||
NetworkMode: host | ||
Dns: | ||
# - 192.168.0.1 |
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.
# - 192.168.0.1 | |
# - 192.168.0.1 |
# settings for docker vms | ||
# docker: | ||
# tls: | ||
# enabled: false | ||
# ca: | ||
# file: docker/ca.crt | ||
# cert: | ||
# file: docker/tls.crt | ||
# key: | ||
# file: docker/tls.key |
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.
# settings for docker vms | |
# docker: | |
# tls: | |
# enabled: false | |
# ca: | |
# file: docker/ca.crt | |
# cert: | |
# file: docker/tls.crt | |
# key: | |
# file: docker/tls.key | |
# settings for docker vms | |
# docker: | |
# tls: | |
# enabled: false | |
# ca: | |
# file: docker/ca.crt | |
# cert: | |
# file: docker/tls.crt | |
# key: | |
# file: docker/tls.key |
# hostConfig: | ||
# NetworkMode: host | ||
# Dns: | ||
# # - 192.168.0.1 | ||
# LogConfig: | ||
# Type: json-file | ||
# Config: | ||
# max-size: "50m" | ||
# max-file: "5" | ||
# Memory: 2147483648 |
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.
# hostConfig: | |
# NetworkMode: host | |
# Dns: | |
# # - 192.168.0.1 | |
# LogConfig: | |
# Type: json-file | |
# Config: | |
# max-size: "50m" | |
# max-file: "5" | |
# Memory: 2147483648 | |
# hostConfig: | |
# NetworkMode: host | |
# Dns: | |
# # - 192.168.0.1 | |
# LogConfig: | |
# Type: json-file | |
# Config: | |
# max-size: "50m" | |
# max-file: "5" | |
# Memory: 2147483648 |
# hostConfig: | |
# NetworkMode: host | |
# Dns: | |
# # - 192.168.0.1 | |
# LogConfig: | |
# Type: json-file | |
# Config: | |
# max-size: "50m" | |
# max-file: "5" | |
# Memory: 2147483648 | |
# hostConfig: | |
# NetworkMode: host | |
# Dns: | |
# # - 192.168.0.1 | |
# LogConfig: | |
# Type: json-file | |
# Config: | |
# max-size: "50m" | |
# max-file: "5" | |
# Memory: 2147483648 |
# Enables/disables the standard out/err from chaincode containers for | ||
# debugging purposes | ||
# attachStdout: false | ||
|
||
# Parameters on creating docker container. | ||
# Container may be efficiently created using ipam & dns-server for cluster | ||
# NetworkMode - sets the networking mode for the container. Supported | ||
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | ||
# Dns - a list of DNS servers for the container to use. | ||
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | ||
# Docker Host Config are not supported and will not be used if set. | ||
# LogConfig - sets the logging driver (Type) and related options | ||
# (Config) for Docker. For more info, | ||
# https://docs.docker.com/engine/admin/logging/overview/ | ||
# Note: Set LogConfig using Environment Variables is not supported. |
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.
# Enables/disables the standard out/err from chaincode containers for | |
# debugging purposes | |
# attachStdout: false | |
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. | |
# Enables/disables the standard out/err from chaincode containers for | |
# debugging purposes | |
# attachStdout: | |
# false | |
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. |
# If you utilize external chaincode builders and don't need the default Docker chaincode builder, | ||
# the endpoint should be unconfigured so that the peer's Docker health checker doesn't get registered. | ||
endpoint: | ||
unix:///var/run/docker.sock | ||
|
||
# settings for docker vms |
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.
# settings for docker vms | |
# settings for docker vms |
# Enables/disables the standard out/err from chaincode containers for | ||
# debugging purposes | ||
attachStdout: | ||
false | ||
|
||
# Parameters on creating docker container. | ||
# Container may be efficiently created using ipam & dns-server for cluster | ||
# NetworkMode - sets the networking mode for the container. Supported | ||
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | ||
# Dns - a list of DNS servers for the container to use. | ||
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | ||
# Docker Host Config are not supported and will not be used if set. | ||
# LogConfig - sets the logging driver (Type) and related options | ||
# (Config) for Docker. For more info, | ||
# https://docs.docker.com/engine/admin/logging/overview/ | ||
# Note: Set LogConfig using Environment Variables is not supported. |
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.
# Enables/disables the standard out/err from chaincode containers for | |
# debugging purposes | |
attachStdout: | |
false | |
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. | |
# Enables/disables the standard out/err from chaincode containers for | |
# debugging purposes | |
attachStdout: | |
false | |
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. |
# settings for docker vms | ||
# docker: | ||
# tls: | ||
# enabled: false | ||
# ca: | ||
# file: docker/ca.crt | ||
# cert: | ||
# file: docker/tls.crt | ||
# key: | ||
# file: docker/tls.key | ||
|
||
# Enables/disables the standard out/err from chaincode containers for | ||
# debugging purposes | ||
# attachStdout: false | ||
|
||
# Parameters on creating docker container. | ||
# Container may be efficiently created using ipam & dns-server for cluster | ||
# NetworkMode - sets the networking mode for the container. Supported | ||
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | ||
# Dns - a list of DNS servers for the container to use. | ||
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | ||
# Docker Host Config are not supported and will not be used if set. | ||
# LogConfig - sets the logging driver (Type) and related options | ||
# (Config) for Docker. For more info, | ||
# https://docs.docker.com/engine/admin/logging/overview/ | ||
# Note: Set LogConfig using Environment Variables is not supported. | ||
# hostConfig: | ||
# NetworkMode: host | ||
# Dns: | ||
# # - 192.168.0.1 | ||
# LogConfig: | ||
# Type: json-file | ||
# Config: | ||
# max-size: "50m" | ||
# max-file: "5" | ||
# Memory: 2147483648 | ||
|
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.
# settings for docker vms | |
# docker: | |
# tls: | |
# enabled: false | |
# ca: | |
# file: docker/ca.crt | |
# cert: | |
# file: docker/tls.crt | |
# key: | |
# file: docker/tls.key | |
# Enables/disables the standard out/err from chaincode containers for | |
# debugging purposes | |
# attachStdout: false | |
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. | |
# hostConfig: | |
# NetworkMode: host | |
# Dns: | |
# # - 192.168.0.1 | |
# LogConfig: | |
# Type: json-file | |
# Config: | |
# max-size: "50m" | |
# max-file: "5" | |
# Memory: 2147483648 | |
# settings for docker vms | |
# docker: | |
# tls: | |
# enabled: false | |
# ca: | |
# file: docker/ca.crt | |
# cert: | |
# file: docker/tls.crt | |
# key: | |
# file: docker/tls.key | |
# Enables/disables the standard out/err from chaincode containers for | |
# debugging purposes | |
# attachStdout: | |
# false | |
# Parameters on creating docker container. | |
# Container may be efficiently created using ipam & dns-server for cluster | |
# NetworkMode - sets the networking mode for the container. Supported | |
# standard values are: `host`(default),`bridge`,`ipvlan`,`none`. | |
# Dns - a list of DNS servers for the container to use. | |
# Note: `Privileged` `Binds` `Links` and `PortBindings` properties of | |
# Docker Host Config are not supported and will not be used if set. | |
# LogConfig - sets the logging driver (Type) and related options | |
# (Config) for Docker. For more info, | |
# https://docs.docker.com/engine/admin/logging/overview/ | |
# Note: Set LogConfig using Environment Variables is not supported. | |
# hostConfig: | |
# NetworkMode: host | |
# Dns: | |
# # - 192.168.0.1 | |
# LogConfig: | |
# Type: json-file | |
# Config: | |
# max-size: "50m" | |
# max-file: "5" | |
# Memory: 2147483648 | |
@bestbeforetoday So, I plan to submit a separate PR to the Fabric core repository to change this to 2 spaces. |
Thank you for your comment! I just saw it, and I appreciate the suggestion—I wasn't aware of that option, so it's very helpful.
Using this option, the indentation differences may indeed be less noticeable when comparing with the original file. That said, I think I'll still go ahead and submit a PR to adjust the indentation in the original file, leaving it to the maintainers to decide. |
The test-network peer configuration specifies $(TWO_DIGIT_VERSION) as the tag for the Node and Java chaincode containers. For Fabric v3.0, this requests fabric-nodeenv:3.0 and fabric-javaenv:3.0 Docker images to host Node and Java chaincode respectively. These images do not exist, which causes deployment of Node and Java chaincode to fail when using Fabric v3.0. Fabric v3.0 continues to use fabric-nodeenv:2.5 and fabric-javaenv:2.5. This change updates the test-network peer configuration to explicitly specify `2.5` as the Node and Java chaincode Docker image tags. This is (currently) the correct version for both Fabric v2.5 and Fabric v3.0. Signed-off-by: Mark S. Lewis <[email protected]>
d9cc00f
to
41ce1e1
Compare
Thank you for taking the time to go through all the formatting. Unfortunately, applying them all can't be done in one go and they are a pain to work through in the GitHub diff editor - I lost my batch updates several times. I have updated this PR to make the minimal changes required to resolve the actual problem being addressed. Formatting can be tackled in a separate PR when I or somebody else has time. |
Given that these files are very similar to the sampleconfig in the main Fabric repo, but are already slightly out-of-date compared to Fabric's version, I wonder if it is worth removing all the problematic commented out sections and comments. Instead, a comment at the top of each file could reference the Fabric sampleconfig. Entirely up to you though. |
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.
Thank you for taking the time to go through all the formatting. Unfortunately, applying them all can't be done in one go and they are a pain to work through in the GitHub diff editor - I lost my batch updates several times. I have updated this PR to make the minimal changes required to resolve the actual problem being addressed. Formatting can be tackled in a separate PR when I or somebody else has time.
@bestbeforetoday
Understood! I’ve reviewed the newly updated commit, and there are no issues with the changes. Thank you for handling this.
Thank you for your comment. As noted below, I’ve submitted a patch to fix the indentation in the original files: Once this is merged, I think it would be a good approach to update the various files in test-network based on the original file, taking your suggestion into consideration as well. |
The test-network peer configuration specifies $(TWO_DIGIT_VERSION) as the tag for the Node and Java chaincode containers. For Fabric v3.0, this requests fabric-nodeenv:3.0 and fabric-javaenv:3.0 Docker images to host Node and Java chaincode respectively. These images do not exist, which causes deployment of Node and Java chaincode to fail when using Fabric v3.0. Fabric v3.0 continues to use fabric-nodeenv:2.5 and fabric-javaenv:2.5.
This change updates the test-network peer configuration to explicitly specify
2.5
as the Node and Java chaincode Docker image tags. This is (currently) the correct version for both Fabric v2.5 and Fabric v3.0.Closes #1267