container: update documentation and remove unused tools

[algolucky]

Summary

addressing some left-over comments from #4927

• adds section explaining some solutions for handling volume permissions.
• adds /etc/algorand/logging.config to the list of special files.
• adds a blurb about how TELEMETRY_NAME takes precedence over the GUID set in /etc/algorand/logging.config.
• removes algoh, carpenter, catchupsrv, ddconfig.sh, find-node.sh, and tealcut from the final image.
• WORKDIR is /algod

Test Plan

• ci
• package testing

[codecov]

Codecov Report

Merging #4982 (4ed7614) into master (5689093) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4982      +/-   ##
==========================================
+ Coverage   53.61%   53.63%   +0.01%     
==========================================
  Files         432      432              
  Lines       54048    54048              
==========================================
+ Hits        28979    28989      +10     
+ Misses      22818    22812       -6     
+ Partials     2251     2247       -4     

Impacted Files	Coverage Δ
agreement/cryptoVerifier.go	67.60% <0.00%> (-2.12%)	⬇️
agreement/proposalManager.go	96.07% <0.00%> (-1.97%)	⬇️
network/wsNetwork.go	65.01% <0.00%> (+0.26%)	⬆️
util/rateLimit.go	82.35% <0.00%> (+0.98%)	⬆️
crypto/merkletrie/node.go	93.48% <0.00%> (+1.86%)	⬆️
crypto/merkletrie/trie.go	68.61% <0.00%> (+2.18%)	⬆️
ledger/roundlru.go	96.22% <0.00%> (+5.66%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[winder]

LGTM

[onetechnical]

Hmm, actually, why are we removing all of these? Container size? It might be nice to use this container to get a copy of compiled binaries.

[algolucky]

find-nodes.sh doesn't really make sense in this context, and neither does algoh? I'm not as familar with the other tools but that may be the case here.

[onetechnical]

I agree most of these don't make sense in the context of running a container, but they do in the sense of copying the binaries out of the container. I don't feel strongly on it, and they do reduce the size of the container to remove, but I could envision someone grabbing this just for the files themselves.

[winder]

Aren't these mostly legacy tools? I would be a little surprised if anyone was using them, and even more surprised if the people using them are also interested in the docker container.

[algolucky]

one idea here is to have an intermediate step. build and push the build container first, then we can extract things into more specific containers if necessary. then we have both, an artifact with all of the binaries that can be used in various ways and then containers for things like algod that have their own stand-alone APIs and storage.

[onetechnical]

I didn't test running it after these changes but they look good in principle!

[onetechnical]

I agree most of these don't make sense in the context of running a container, but they do in the sense of copying the binaries out of the container. I don't feel strongly on it, and they do reduce the size of the container to remove, but I could envision someone grabbing this just for the files themselves.

[winder]

nit: replace /dist with $GOPATH everywhere

[winder]

nit: /etc/algorand or /etc/algod seems like a better name for these sorts of files than /algod.

Couldn't this approach cause issues if the user has mounted a volume to /algod?

[algolucky]

good point. it may be best to keep data, bin, and config in separate directory trees.

since /algod is reserved for data, binaries can be placed in /bin and config in /etc/algorand.

[winder]

Why two steps here instead of letting goal network create put files in the correct location?

[algolucky]

goal network create was complaining that /algod was not empty, probably from moving things to /algod. without the --noclean option, it is a destructive command which will delete if not successful.

I figured that the safest option would be to execute this on a temporary directory to remove the chance of an accidental removal of any data as this is the default behavior.

[winder]

That warning is a feature not a bug. If the directory is not suitable for a private network it's important to understand why. At times I was careful to create the network before updating settings to avoid this error. We also updated "goal network create" to allow empty directories, are there other specific exceptions we should add?

[algolucky]

good point.

go-algorand/cmd/goal/network.go

Line 88 in 4631571

if util.FileExists(networkRootDir) && !util.IsEmpty(networkRootDir) {

is where it is getting caught up at. In terms of exceptions, I don't think any are warrented. probably best that the container errors if /algod/data exists already (pre-exiting data mount). I'll revert my changes.

[winder]

Nice sleuthing. That IsEmpty line was added for the docker container so that you could mount an empty directory: #3911

Maybe changing the config file override directory would resolve the problems you saw?

[winder]

I was testing the container and noticed TARGETARCH is now required.

If it works, it works (and it does, after specifying my architecture it works fine). But I wonder if it's really needed?
It looks like the golang docker container uses a different approach:
arch="$(dpkg --print-architecture)"; arch="${arch##*-}"; \

There is some sort of issue when using --build-arg=stable for the build and a public network:

docker run --rm -it -p 4190:8080 -e TOKEN=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -e NETWORK=mainnet algod_test

Has an error:

+ '[' -f /algod/data/../network.json ']'
+ '[' -f /algod/data/genesis.json ']'
+ '[' mainnet == '' ']'
+ start_new_public_network
+ cd /node
+ '[' '!' -d run/genesis/mainnet ']'
+ mkdir -p /algod/data
+ cd /algod/data
+ cp /node/run/genesis/mainnet/genesis.json genesis.json
+ cp /node/run/config.json.example config.json
+ configure_data_dir
+ cd /algod/data
+ algocfg -d . set -p GossipFanout -v 1
Warning: Error loading config file from '.'

It worked fine if I build the container with --build-arg CHANNEL=nightly, or build from source with URL/BRANCH.

[algolucky]

@winder I noticed that too, it copies the config.json.example from the repo. so I think that the config has changed since stable release which is causing that issue.

[winder]

@winder I noticed that too, it copies the config.json.example from the repo. so I think that the config has changed since stable release which is causing that issue.

That would explain it.

Here is a PR to clarify the error in the future: #5025

[winder]

LGTM. Tested and everything related to this PR looks fine.