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

Add Docker Swarm config file, and Swarm & compose config file support #1540

Merged

Conversation

SvenDowideit
Copy link

@SvenDowideit SvenDowideit commented Mar 9, 2018

for #1362

see README.md changes for how&what it does

can be tested using the image at svendowideit/docker-gitlab

@SvenDowideit SvenDowideit force-pushed the docker-config-and-secrets-file-env branch 3 times, most recently from 7f8c397 to b137dd2 Compare March 9, 2018 02:28
@SvenDowideit SvenDowideit changed the title Add Docker Swarm config file, and Swar&compose config file support Add Docker Swarm config file, and Swarm&compose config file support Mar 9, 2018
@SvenDowideit SvenDowideit changed the title Add Docker Swarm config file, and Swarm&compose config file support Add Docker Swarm config file, and Swarm & compose config file support Mar 9, 2018
README.md Outdated
file: ./gitlab.secrets
```

If you're only using one of these files, don't include its entries in the docker-compose file.
Copy link

Choose a reason for hiding this comment

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

Does this mean "If you're not using one of these files, then don't include its entry in the docker-compose file"? It's a little confusing as currently worded.

@SvenDowideit SvenDowideit force-pushed the docker-config-and-secrets-file-env branch from b137dd2 to a2d7efb Compare March 19, 2018 00:02
@SvenDowideit SvenDowideit force-pushed the docker-config-and-secrets-file-env branch from a2d7efb to 4542234 Compare April 10, 2018 06:53
@SvenDowideit SvenDowideit force-pushed the docker-config-and-secrets-file-env branch from 4542234 to d244882 Compare April 18, 2018 06:38
@SvenDowideit
Copy link
Author

what do you think @solidnerd ?

@solidnerd
Copy link
Collaborator

Hey @SvenDowideit,

thanks for your work. I think it is a nice cool idea to support that.

I would like to see this as an additional content so for this case I would like to have it as feature. That means I think we should also need to set an environment variable that allows us to use this feature something like SUPPORT_DOCKER_SWARM_CONFIGS. Also I don't like the change in the standard docker-compose.yml because this a breaking change for the project because compose in version 2 is for single hosts and everything higher than 3.x is for the cluster approach. Also this feature could be used by kubernetes configmaps. I think it would be nice for the user to have an example with that. So i think we could gonna put this in contrib/docker-swarm. In future there will be also support for helm and pure k8s.

WDYT ?

@SvenDowideit SvenDowideit force-pushed the docker-config-and-secrets-file-env branch from d244882 to 18da2d2 Compare May 14, 2018 05:41
@SvenDowideit
Copy link
Author

@solidnerd idk - SUPPORT_DOCKER_SWARM_CONFIGS seems redundant - if you're not using Docker Swarm secrets, then there are no swarm files - so the new code in runtime/functions won't trigger.

wrt moving the contrib - can do...

@SvenDowideit SvenDowideit force-pushed the docker-config-and-secrets-file-env branch 2 times, most recently from ce18316 to 244eed7 Compare May 14, 2018 05:54
@SvenDowideit SvenDowideit force-pushed the docker-config-and-secrets-file-env branch from 244eed7 to 474fe9f Compare May 22, 2018 04:23
@solidnerd
Copy link
Collaborator

solidnerd commented May 22, 2018

I think we will support this for 10.8.0 and yes you have right an extra env will make this redundant .

I tested it and it's working for me. Thanks for the work ;)

@SvenDowideit
Copy link
Author

@solidnerd awesome! looking forward to it :)

@solidnerd solidnerd merged commit 8b638b3 into sameersbn:master May 26, 2018
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.

3 participants