Skip to content
This repository has been archived by the owner on May 20, 2022. It is now read-only.

Swarm file flavors #222

Merged
merged 5 commits into from
Jan 21, 2018
Merged

Swarm file flavors #222

merged 5 commits into from
Jan 21, 2018

Conversation

ulm0
Copy link
Contributor

@ulm0 ulm0 commented Jan 15, 2018

After reading the swarm file you guys offer, i saw it could be improved by "un-exposing" some ports for services that can talk to each other through the overlay network, and by adding some more documentation at the top of the file.

- APP_HOST=mm_app
# talk to the port within the overlay network
# without (over)exposing ports
- APP_PORT_NUMBER=80
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did some digging in the app container and it doesn't listen on port 8065 as stated in the current swarm file, but the port 80.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, don't know why it was like that...

Copy link
Contributor Author

@ulm0 ulm0 left a comment

Choose a reason for hiding this comment

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

Meant to be a single comment

deploy:
restart_policy:
condition: on-failure
web:
Copy link
Contributor Author

@ulm0 ulm0 Jan 15, 2018

Choose a reason for hiding this comment

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

I've also been able to make mattermost work without the web container provided here, and using traefik talking right to the app container instead. Could add an example if wanted.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay to drop this container on the stack file as we discussed here. I use (and prefer) Traefik too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned in that thread, this repo may keep flavors with the stack file, one with this web image, another with traefik.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we can keep both that's a good things, but I don't know how to do that without duplicating a lot of configuration on both files ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya, the config duplication is a problem, but I think maintaining both, the current one and the one with traefik will be enough support.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, good for me to get both is you want to provide a Traefik example :)

@pichouk
Copy link
Contributor

pichouk commented Jan 16, 2018

Looks good to me, waiting for @xcompass approval too :)

@pichouk pichouk requested a review from xcompass January 16, 2018 18:50
@ulm0
Copy link
Contributor Author

ulm0 commented Jan 16, 2018

Btw i renamed the file docker-compose.yml. should i name it docker-stack.yml instead?

@pichouk
Copy link
Contributor

pichouk commented Jan 16, 2018

I prefer to keep docker-stack.yml unless you have a good argument to use docker-compose.yml ?

@ulm0
Copy link
Contributor Author

ulm0 commented Jan 16, 2018

Nah, I just use docker-compose.yml for convention, since it uses the Compose file version 3 reference, although is aimed for swarm only, but i can name it back to docker-stack.yml in order to avoid confusion.

@ulm0 ulm0 changed the title Swarm file documentation [WIP] Swarm file documentation Jan 16, 2018
@ulm0 ulm0 changed the title [WIP] Swarm file documentation [WIP] Swarm file flavors Jan 16, 2018
@ulm0
Copy link
Contributor Author

ulm0 commented Jan 16, 2018

@pichouk, just added the file with traefik

@ulm0 ulm0 changed the title [WIP] Swarm file flavors Swarm file flavors Jan 16, 2018
@pichouk
Copy link
Contributor

pichouk commented Jan 21, 2018

LGTM, thanks @ulm0 :)

@pichouk pichouk merged commit 09d5955 into mattermost:master Jan 21, 2018
andruwa13 pushed a commit to andruwa13/mattermost-docker that referenced this pull request Jan 31, 2018
* Better documentation for swarm file, avoid exposing ports for no reason

* add swarm file using traefik
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants