-
Notifications
You must be signed in to change notification settings - Fork 751
adds config.json for C-Chain during antithesis - json logs #3968
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
adds config.json for C-Chain during antithesis - json logs #3968
Conversation
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.
Pull Request Overview
This PR adds a configuration file for the C-Chain to ensure logs are output in JSON format during antithesis tests with docker-compose.
- Adds logic in tests/antithesis/compose.go to create a config.json when the volume path indicates the C-Chain configuration directory.
- Updates the docker-compose project to mount the new configuration directory.
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.
Pull Request Overview
This PR ensures C-Chain logs are output in JSON format during Antithesis tests by adding a config.json
for the C-Chain directory and mounting it in Docker Compose.
- Write
config.json
in the C-Chain configs directory to set"log-json-format": true
- Mount the local C-Chain configs directory into the container
- Add
isCChainConfigDir
helper to identify the C-Chain config directory
Comments suppressed due to low confidence (2)
tests/antithesis/compose.go:128
- [nitpick] Add a unit test in the Antithesis suite to verify that
config.json
is created in the C-Chain config directory and contains the expected setting.
if isCChainConfigDir(volumePath) {
tests/antithesis/compose.go:184
- [nitpick] Add a brief comment above this volume mount to explain that it maps the local C-Chain config directory so that
config.json
is applied inside the container.
{ Type: types.VolumeTypeBind, Source: fmt.Sprintf("./volumes/%s/configs/chains/C", serviceName), Target: "/root/.avalanchego/configs/chains/C", },
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.
Pull Request Overview
This PR adds a configuration file (config.json) for the C-Chain during antithesis tests to enforce JSON-formatted logging.
- Adds a new cChainConfig type and logic to generate and write config.json when the volume path matches the expected C-Chain config directory
- Updates the docker-compose volume mappings in tests/antithesis/compose.go to include the C-Chain configuration directory
Comments suppressed due to low confidence (1)
tests/antithesis/compose.go:133
- Consider adding unit tests to verify that isCChainConfigDir correctly identifies the expected C-Chain configuration directory paths and that config.json is generated as expected.
if isCChainConfigDir(volumePath) {
Your initiative is appreciated! Given that we currently supply node configuration via flags, I would ask that you similarly provide the C-Chain configuration via a json-encoded flag rather than as a file on disk. For future, it may make sense to discuss details like this with maintainers in advance of implementation. Example of composing chain configuration for provision as a flag value: https://github.com/ava-labs/avalanchego/blob/master/tests/fixture/tmpnet/network.go#L859 |
@maru-ava Thank you for review. If I understand well configurations for networks are already stored on disk. Also latest tests are being run in docker compose. Docker compose uses images built from the default Dockerfile. Building them here or here (Depending if it is avalanchego or xsvm). I am not sure if changing flags could configure it, would appreciate suggestion. Here is originally built docker-compose.yaml: services:
avalanche-bootstrap-node:
container_name: avalanche-bootstrap-node
environment:
AVAGO_API_ADMIN_ENABLED: "true"
AVAGO_HEALTH_CHECK_FREQUENCY: 2s
AVAGO_HTTP_HOST: 0.0.0.0
AVAGO_INDEX_ENABLED: "true"
AVAGO_LOG_DISPLAY_LEVEL: TRACE
AVAGO_LOG_FORMAT: json
AVAGO_LOG_LEVEL: DEBUG
AVAGO_NETWORK_ID: local
AVAGO_NETWORK_MAX_RECONNECT_DELAY: 1s
AVAGO_NETWORK_PEER_LIST_PULL_GOSSIP_FREQUENCY: 250ms
AVAGO_PUBLIC_IP: 10.0.20.3
AVAGO_STAKING_SIGNER_KEY_FILE_CONTENT: QXZhbGFuY2hlTG9jYWxOZXR3b3JrVmFsaWRhdG9yMDE=
AVAGO_STAKING_TLS_CERT_FILE_CONTENT:content==
AVAGO_STAKING_TLS_KEY_FILE_CONTENT:content
hostname: avalanche-bootstrap-node
image: antithesis-xsvm-node:993ae109
networks:
avalanche-testnet:
ipv4_address: 10.0.20.3
volumes:
- type: bind
source: ./volumes/avalanche-bootstrap-node/logs
target: /root/.avalanchego/logs
avalanche-node-1:
container_name: avalanche-node-1
environment:
AVAGO_API_ADMIN_ENABLED: "true"
AVAGO_BOOTSTRAP_IDS: NodeID-7Xhw2mDxuDS44j42TCB6U5579esbSt3Lg
AVAGO_BOOTSTRAP_IPS: 10.0.20.3:9651
AVAGO_HEALTH_CHECK_FREQUENCY: 2s
AVAGO_HTTP_HOST: 0.0.0.0
AVAGO_INDEX_ENABLED: "true"
AVAGO_LOG_DISPLAY_LEVEL: TRACE
AVAGO_LOG_FORMAT: json
AVAGO_LOG_LEVEL: DEBUG
AVAGO_NETWORK_ID: local
AVAGO_NETWORK_MAX_RECONNECT_DELAY: 1s
AVAGO_NETWORK_PEER_LIST_PULL_GOSSIP_FREQUENCY: 250ms
AVAGO_PUBLIC_IP: 10.0.20.4
AVAGO_STAKING_SIGNER_KEY_FILE_CONTENT: QXZhbGFuY2hlTG9jYWxOZXR3b3JrVmFsaWRhdG9yMDQ=
AVAGO_STAKING_TLS_CERT_FILE_CONTENT: content==
AVAGO_STAKING_TLS_KEY_FILE_CONTENT:content
AVAGO_TRACK_SUBNETS: BKBZ6xXTnT86B4L5fp8rvtcmNSpvtNz8En9jG61ywV2uWyeHy
hostname: avalanche-node-1
image: antithesis-xsvm-node:993ae109
networks:
avalanche-testnet:
ipv4_address: 10.0.20.4
volumes:
- type: bind
source: ./volumes/avalanche-node-1/logs
target: /root/.avalanchego/logs
avalanche-node-2:
container_name: avalanche-node-2
environment:
AVAGO_API_ADMIN_ENABLED: "true"
AVAGO_BOOTSTRAP_IDS: NodeID-7Xhw2mDxuDS44j42TCB6U5579esbSt3Lg
AVAGO_BOOTSTRAP_IPS: 10.0.20.3:9651
AVAGO_HEALTH_CHECK_FREQUENCY: 2s
AVAGO_HTTP_HOST: 0.0.0.0
AVAGO_INDEX_ENABLED: "true"
AVAGO_LOG_DISPLAY_LEVEL: TRACE
AVAGO_LOG_FORMAT: json
AVAGO_LOG_LEVEL: DEBUG
AVAGO_NETWORK_ID: local
AVAGO_NETWORK_MAX_RECONNECT_DELAY: 1s
AVAGO_NETWORK_PEER_LIST_PULL_GOSSIP_FREQUENCY: 250ms
AVAGO_PUBLIC_IP: 10.0.20.5
AVAGO_STAKING_SIGNER_KEY_FILE_CONTENT: QXZhbGFuY2hlTG9jYWxOZXR3b3JrVmFsaWRhdG9yMDI=
AVAGO_STAKING_TLS_CERT_FILE_CONTENT: content==
AVAGO_STAKING_TLS_KEY_FILE_CONTENT: content==
AVAGO_TRACK_SUBNETS: BKBZ6xXTnT86B4L5fp8rvtcmNSpvtNz8En9jG61ywV2uWyeHy
hostname: avalanche-node-2
image: antithesis-xsvm-node:993ae109
networks:
avalanche-testnet:
ipv4_address: 10.0.20.5
volumes:
- type: bind
source: ./volumes/avalanche-node-2/logs
target: /root/.avalanchego/logs
avalanche-node-3:
container_name: avalanche-node-3
environment:
AVAGO_API_ADMIN_ENABLED: "true"
AVAGO_BOOTSTRAP_IDS: NodeID-7Xhw2mDxuDS44j42TCB6U5579esbSt3Lg
AVAGO_BOOTSTRAP_IPS: 10.0.20.3:9651
AVAGO_HEALTH_CHECK_FREQUENCY: 2s
AVAGO_HTTP_HOST: 0.0.0.0
AVAGO_INDEX_ENABLED: "true"
AVAGO_LOG_DISPLAY_LEVEL: TRACE
AVAGO_LOG_FORMAT: json
AVAGO_LOG_LEVEL: DEBUG
AVAGO_NETWORK_ID: local
AVAGO_NETWORK_MAX_RECONNECT_DELAY: 1s
AVAGO_NETWORK_PEER_LIST_PULL_GOSSIP_FREQUENCY: 250ms
AVAGO_PUBLIC_IP: 10.0.20.6
AVAGO_STAKING_SIGNER_KEY_FILE_CONTENT: QXZhbGFuY2hlTG9jYWxOZXR3b3JrVmFsaWRhdG9yMDM=
AVAGO_STAKING_TLS_CERT_FILE_CONTENT: content==
AVAGO_STAKING_TLS_KEY_FILE_CONTENT: content
AVAGO_TRACK_SUBNETS: BKBZ6xXTnT86B4L5fp8rvtcmNSpvtNz8En9jG61ywV2uWyeHy
hostname: avalanche-node-3
image: antithesis-xsvm-node:993ae109
networks:
avalanche-testnet:
ipv4_address: 10.0.20.6
volumes:
- type: bind
source: ./volumes/avalanche-node-3/logs
target: /root/.avalanchego/logs
avalanche-node-4:
container_name: avalanche-node-4
environment:
AVAGO_API_ADMIN_ENABLED: "true"
AVAGO_BOOTSTRAP_IDS: NodeID-7Xhw2mDxuDS44j42TCB6U5579esbSt3Lg
AVAGO_BOOTSTRAP_IPS: 10.0.20.3:9651
AVAGO_HEALTH_CHECK_FREQUENCY: 2s
AVAGO_HTTP_HOST: 0.0.0.0
AVAGO_INDEX_ENABLED: "true"
AVAGO_LOG_DISPLAY_LEVEL: TRACE
AVAGO_LOG_FORMAT: json
AVAGO_LOG_LEVEL: DEBUG
AVAGO_NETWORK_ID: local
AVAGO_NETWORK_MAX_RECONNECT_DELAY: 1s
AVAGO_NETWORK_PEER_LIST_PULL_GOSSIP_FREQUENCY: 250ms
AVAGO_PUBLIC_IP: 10.0.20.7
AVAGO_STAKING_SIGNER_KEY_FILE_CONTENT: QXZhbGFuY2hlTG9jYWxOZXR3b3JrVmFsaWRhdG9yMDU=
AVAGO_STAKING_TLS_CERT_FILE_CONTENT: content==
AVAGO_STAKING_TLS_KEY_FILE_CONTENT: content
AVAGO_TRACK_SUBNETS: BKBZ6xXTnT86B4L5fp8rvtcmNSpvtNz8En9jG61ywV2uWyeHy
hostname: avalanche-node-4
image: antithesis-xsvm-node:993ae109
networks:
avalanche-testnet:
ipv4_address: 10.0.20.7
volumes:
- type: bind
source: ./volumes/avalanche-node-4/logs
target: /root/.avalanchego/logs
workload:
container_name: workload
environment:
AVAWL_CHAIN_IDS: 2fRVf81H2AvG8qicdez81T8pJJiQCz5nRZ49otVK3VEDZjLgGM
AVAWL_URIS: http://10.0.20.3:9650,http://10.0.20.4:9650,http://10.0.20.5:9650,http://10.0.20.6:9650,http://10.0.20.7:9650
hostname: workload
image: antithesis-xsvm-workload:993ae109
networks:
avalanche-testnet:
ipv4_address: 10.0.20.129
networks:
avalanche-testnet:
driver: bridge
ipam:
config:
- subnet: 10.0.20.0/24 As a conclusion I am not sure if we need to set any flags in docker image build from the original Dockerfile. On another side, I am not aware if it is possible to set config parameter(s) for C-Chain using environment variables. Per C-chain documentation and chain-config documentation we should add config.json to configure C-chain. Also, some volumes (logs and db) + docker-compose.yml are already being written to the disk. Maybe @StephenButtolph has some inputs here? |
@aleksandarknezevic I think we typically try to pass all arguments as environment variables. In this case, I think that means we should use the "chain-config-content" flag (which should be the AVAGO_CHAIN_CONFIG_CONTENT environment variable). |
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.
Pull Request Overview
This PR adds a new configuration file for the C-Chain to switch its log format to JSON during antithesis tests, avoiding the mix of json and text log formats.
- Introduces new types (CChainConfig and ChainConfig) and the function encodeChainConfig() to build and base64‑encode the configuration.
- Updates the antithesis compose project to include the encoded chain configuration in the service environment.
6c7ed84
to
e028f0e
Compare
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.
Pull Request Overview
This PR introduces a new configuration mechanism for the C-Chain by adding a config.json file to enable JSON log formatting during Antithesis tests with docker-compose.
- Added types and a helper function (encodeChainConfig) to generate a nested, base64-encoded chain configuration.
- Modified environment variable setup in newComposeProject to include the encoded chain config when available.
tests/antithesis/compose.go
Outdated
|
||
chainConfigB64, err := encodeChainConfig() | ||
if err != nil { | ||
fmt.Printf("Failed to encode chain config: %v\n", err) |
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.
The error returned by encodeChainConfig is only printed and not propagated. Consider returning the error or handling it more robustly to avoid silent failures during configuration setup.
fmt.Printf("Failed to encode chain config: %v\n", err) | |
return nil, fmt.Errorf("failed to encode chain config: %w", err) |
Copilot uses AI. Check for mistakes.
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.
chainConfigB64 is optional, don't want to break tests and docker-compose.yaml creation. It is fine to omit chain configuration.
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.
It should actually be mandatory (i.e. a failure to compose the key value should be propagated) otherwise we aren't guaranteed of the c-chain emitting logs in json format.
Apologies for not responding sooner, I somehow missed notification of your reply. Also apologies for the lack of clarity in my original response, it shouldn't be necessary to duplicate the creation of the flag value since the network already has this ability: // outside the loop (once for all nodes)
// Configure the network
if network.PrimaryChainConfigs["C"] == nil {
network.PrimaryChainConfigs["C"] = tmpnet.ConfigMap{}
}
network.PrimaryChainConfigs["C"]["log-json-format"] = true
// Use the network to create the flag value
chainConfigContent, err := network.GetChainConfigContent()
if err != nil {
return fmt.Errorf("failed to get chain config content: %w", err)
}
...
// inside the loop (for each node)
env := types.Mapping{
...
// The value should always be non-empty here since we are setting the format and checking the error
// that could result from composing the flag value
config.ChainConfigContentKey: chainConfigContent,
} Note that this will require updating Edit1: Updated to use correct c-chain flag key and value |
Thank you @maru-ava for the awesome explanation. I've just updated PR with your recommendations (tested locally and confirmed that C-chain logs are in json format. Would appreciate your review. |
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.
One nit and then LGTM
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.
LGTM, thank you!
Rescinding approval pending resolution of test failure
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 your patience, the tests look happy now. Will just need to wait for a second approval to merge.
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.
Deluje dobro, hvala za doprinos!
Translation from Serbian: Looks good, thank you for your contribution!
@maru-ava It seems, I am missing permissions to merge:
Could you merge it please? |
Nobody is allowed to merge to master, we use a merge queue. I previously put it in the merge queue, but since you pushed a merge commit it got kicked out and we have to re-run CI and re-trigger merge. For future, please avoid pushing anything to avoid interrupting merge. It's fine if the branch isn't current with master so long as there are no conflicts. |
Thank you for explanation. Didn't notice information about merge queue. Sorry about that. |
Oh, I see what the problem is... You'll need to sign your commits. We have this enabled as a safety measure. |
Head branch was pushed to by a user without write access
ec80c71
to
74ecf1f
Compare
@maru-ava Now I force pushed in one signed commit. Hope it is fine. Thank you! |
Why this should be merged
Stop mixing json and text log formats during antithesis test in docker-compose.
Closes #3933
How this works
Per documentation adding
config.json
file in the$HOME/.avalanchego/configs/chains/C
with content:Converts C-Chain logs to json format during Antithesis tests by adding configuration file for docker-compose.
How this was tested
Commands:
task test-build-antithesis-images-avalanchego
task test-build-antithesis-images-xsvm
Before change logs were in json while C-Chain logs were in text:
After changes:
Need to be documented in RELEASES.md?
N/A