Skip to content
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: try to fix default hostname #1550

Closed
wants to merge 3 commits into from
Closed

Conversation

NtskwK
Copy link
Contributor

@NtskwK NtskwK commented Sep 3, 2024

No description provided.

@pierotofy
Copy link
Member

Hey thanks @NtskwK, I understand what the intent here is, but can you provide some context about how to reproduce the problem this patch is solving?

@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 4, 2024

I noticed that the name of container are not always the same when I try to deploy the WebODM on WSL with ./webodm.sh start --dev --gpu(?) . Sometime, node-odm-1, webodm-node-odm-1, [DIR_NAME]-odm-1.

I don't fully understand the way nodeodm is named. But setting the hostname to the container name will allow them to connect successfully.

@pierotofy
Copy link
Member

pierotofy commented Sep 4, 2024

I wonder if explicitly setting the container_name (https://docker-docs.uclv.cu/compose/compose-file/compose-file-v2/#container_name) property to docker-compose.nodeodm.yml would make the hostname predictable and not require runtime checks.

If so, we could use it when --default-nodes is 1 in webodm.sh (but not when --default-nodes is > 1).

@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 4, 2024

That's weird. If there is already a container named webodm_db/webodm_webapp/broker/worker, docker-compose will throw an error xxx is already exist then stop batch. But it's different to node-odm-1, because docker-compose.nodeodm.yml(and .gpu.*) dose not appoint container_name with container_name: xxx. So [DIR_NAME]-node-odm-1 will be renamed by Docker automaticlly.

@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 4, 2024

> webodm.sh

down(){
	command="$docker_compose -f docker-compose.yml"

	if [ "${GPU_NVIDIA}" = true ]; then
		command+=" -f docker-compose.nodeodm.gpu.nvidia.yml"
	elif [ "${GPU_INTEL}" = true ]; then
		command+=" -f docker-compose.nodeodm.gpu.intel.yml"
	else
		command+=" -f docker-compose.nodeodm.yml"
	fi

	command+=" -f docker-compose.nodemicmac.yml down --remove-orphans"

	run "${command}"
}

docker-compose.nodeodm.gpu.* will not be shutdown if --gpu was not be used in ./webodm down. This can be an important cause of container residue. Then, a new WebODM program starter will launch a container which is named DIR_NAME]-node-odm-1.

Use container name to replace hostname is only functional when both they(Client/Server) are in the same docker-compose group and can't be captured by other container or even by host machine. Well, the new WebODM cannot capture the already existing nodes(node-odm-1)

@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 4, 2024

On user-defined networks, containers can not only communicate by IP address, but can also resolve a container name to an IP address.

Is can only works in user-defined bridge networks and disable in default bridge networks.

@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 4, 2024

I wonder if explicitly setting the container_name (https://docker-docs.uclv.cu/compose/compose-file/compose-file-v2/#container_name) property to docker-compose.nodeodm.yml would make the hostname predictable and not require runtime checks.

If so, we could use it when --default-nodes is 1 in webodm.sh (but not when --default-nodes is > 1).

It's theoretically possible, but not recommended. The logic of the function will become complex. If docker-compose starts successfully, but docker-compose.node is blocked due to duplicate names, it will be deployed in a state without nodes, which is not expected.

@pierotofy
Copy link
Member

https://docs.docker.com/compose/networking/

Mm, perhaps just setting COMPOSE_PROJECT_NAME to "webodm" would do?

@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 4, 2024 via email

@pierotofy pierotofy marked this pull request as draft September 4, 2024 16:54
@NtskwK NtskwK marked this pull request as ready for review September 4, 2024 17:44
@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 6, 2024

#1237


command+=" -f docker-compose.nodeodm.gpu.nvidia.yml"
command+=" -f docker-compose.nodeodm.gpu.intel.yml"
command+=" -f docker-compose.nodeodm.yml"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the change here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shutdown function now has to be entered after the --gpu to shut down the gpu node. But it's just the startup parameter in the manual. There may be users who get unexpected results. I would like to be able to make sure they shut down.

@@ -11,6 +11,7 @@ services:
- WO_DEFAULT_NODES
node-odm:
image: opendronemap/nodeodm:gpu
container_name: node-odm-1
Copy link
Member

@pierotofy pierotofy Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test with:

./webodm.sh restart --default-nodes 3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My apologies. I forgot there would be conflict here.

I still don't fully understand the way to the naming. Can you give me some advice?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to understand how docker-compose works.

This is a good start: https://docs.docker.com/compose/

Read it all.

@NtskwK
Copy link
Contributor Author

NtskwK commented Sep 8, 2024

You are right, maybe I should not affect the behavior of the other. Just focus on the try to fix default hostname like next function do.

Workaround for mysterious "webodm_node-odm-1" or "webodm-node-odm-1" hostname switcharoo on Mac

How do you think?

@pierotofy pierotofy closed this Sep 29, 2024
@NtskwK NtskwK deleted the patch-3 branch November 17, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants