This pull request is failing on build because it depends on the following pull requests: docker-archive-public/docker.engine-api#352
moby/swarmkit#1318

I could fix the failing-ci if I change hack/vendor.sh to download the swarmkik and engine-api dependencies from my forked versions of these repos. How to proceed? Should I wait for the for the engine-api and swarmkit pull requests to be accepted, or to change the vendor.sh?

@vasil-yordanov only the vendor check should fail because of vendoring (which is ok). The other ones fails because of gofmt :

16:24:08 ---> Making bundle: validate-gofmt (in bundles/1.13.0-dev/validate-gofmt)
16:24:08 These files are not properly gofmt'd:
16:24:08  - api/client/service/opts.go
16:24:08  - daemon/cluster/convert/container.go
16:24:08 
16:24:08 Please reformat the above files using "gofmt -s -w" and commit the result.

@vdemeester I have fixed the formatting, but some check fails. I saw in the logs "Variable with name 'BUILD_DISPLAY_NAME' already exists, current value: '#25437', new value: '#25437'". Is this some issue from build system side or there is something from my side that I have to do?

@vasil-yordanov looks like the error is;

11:45:53 # github.com/docker/docker/api/client/service
11:45:53 api/client/service/opts.go:445: unknown swarm.ContainerSpec field 'Hostname' in struct literal

@thaJeztah this error is because the code depends on github.com/docker/engine-api/types/swarm, which is in another github repo, where I have waiting pull request.
docker-archive-public/docker.engine-api#352
Actually this pull request depend on pull requests on two other repos: docker/engine-api and docker/swamkit, I'm not sure if this is the same team that is responsible for accepting the pull request on these three repos or it is separated teams with good synchronization.
@vdemeester told me earlier that "only the vendor check should fail because of vendoring (which is ok).", so I believe that now this error is due to vendoring only.

@vasil-yordanov what we usually do, is to temporarily include the changes in the vendored code in this PR (you can do separate commits for that); that way the code works, and CI can run. The vendor-CI will complain that you made changes to vendored code that don't match the commit/tag specified in hack/vendor.sh, but that's ok; once this PR is in code review, and the related PRs are merged, you can update the hack/vendor.sh and vendored code, so that the vendor CI goes green.

Thank you @theJeztah for explaining me the process. I will commit vendor. Actually now I noticed that there is such folder in this repo. I was expecting that it is created by hach/vendor.sh during build time, and the dependencies are not kept here, but fetched when needed from the corresponding repo.

@GordonTheTurtle I have checked that I have signed all my commits with git -s except the merges to the master of docker/docker repo which generates automatic commit message.
I also checked that "git rebase -i HEAD~5" does not show commits that I haven't signed, It do not show the merge commits.

These are my commits from "git reabase -i" command
pick 2a659dd Added option hostname to docker service create. This option will allow setting the same hostname on all containers spawned for given service
pick 3c4358d Reformated files with gofmt
pick 0009a68 Add vendor changes, to build the code and make CI to pass
In the git-rebase-todo I can not see merge commits.

All my commits have been signed when I did the commits, so it seems I do not need to amend the commits.

@vasil-yordanov you should not have merge commits of master in your PR. You should remove the merge commit and rebase, and that will fix the signing issue.

Does anyone has idea why this CI fails?
docker_cli_build_test.go is not related to my change for docker service create --hostname option.

win2lin is currently having issues (there's a PR to fix that), not sure about windowsRS1, but we can restart. During design review, it's not a problem anyway

@thaJeztah I have implemented the --hostname option to be a template, like --hostname=srv-*, in this case if we create a service with replicas>1 then the hostname will be srv-1, srv-2, ...
Only single replacement of * will happen. If there is not * at all then no replacement will be done and the hostname will be set the same on all containers for the given service.
Do you think this design is acceptable for you?
In my case I will not use * at all, I have implemented this because some people wanted it in #24877

I have removed my last commit after some discussion with stevvooe. "We need to be careful about creating a "template" system without thinking about consistent user experience."

Not really sure why hostname would really matter in services (or even normal containers)... but I guess people find reasons.
In the linked issue it looks like there was still some discussion going on.
@mavenugo Are you ok with this?

@stevvooe
I think you're confusing the hostname seen within the container process with the hostname exported to the network.

Indeed, I though they should be the same!

The main issue is whether or not bind these two concepts. While what you are demonstrating is important, it is largely orthogonal to this discussion. In fact, showing that these are easily confused is an argument on the side of considering this parameter in the context of a DNS mapping scheme.

Totally agree.
Going moby/swarmkit#1242

Indeed, I though they should be the same!

@mostolog If only life were so simple!!!

Thanks for the feedback! Let's keep bouncing this around so we can get this right.

ping @stevvooe @mrjana what's the status here? Is this going to be something for the templating feature that @stevvooe is working on?

@thaJeztah We merged the functionality in swarmkit. This PR will likely need to be carried for integration into docker.

@stevvooe, @thaJeztah so since moby/swarmkit#1688 was merged I assume we have to also do our job and merge that one in vendor.
Or since actually @vasil-yordanov has referenced his repo in vendor.sh we simply just need to refer again back to docker/swarmkit and that would be sufficient.
Also since vendor is under source control I assume that I will have to commit the changes and rebase again?

@niau That is the process. Let us know if you need help!

uha @stevvooe I assume that I had really lagged the dogs here.
This is basically the result of updating vendor.sh, executing it, and committing the various changes to vendor folders. It was a strange that the new run of vendor.sh caused also a lot of changes not only to swarmkit but also to other vendor folders. Is this how it is and should I commit also those? If not the changes are in separate commits to facilitate easy reverting.

@niau You can do something like ./hack/vendor.sh github.com/docker/swarmkit to only update that dependency. Let's keep the updates scoped to swarmkit.

Ok, this PR wasn't actually merged, but possibly @vasil-yordanov rebased/updated his master, and GitHub shows the exact same tree as 80d6d2e, so automatically marks it as "merged", even though nothing was actually merged;

@thaJeztah very true I simply rebased it. In docker/master is still missing the hostname

For example here
https://github.com/docker/docker/blob/master/cli/command/service/opts.go among the consts should be also "hostname" which is missing.

I think what @yongtang did and @vdemeester was only to revendor swarmkit

@niau looks like something went wrong during the rebase, because the change set in this PR is not empty https://github.com/docker/docker/pull/25437/files

Thanks @thaJeztah I assume so. We are missing one commit. I have recommitted it in Vasil's repo, but it does not appear here. It looks like the only option is a new PR. Is it so?

@niau yep, and it would be best to open the PR using a branch (that way, if you change/rebase master, it shouldn't happen again 👼)

@vdemeester @thaJeztah sorry for all the noise I hope that the latest PL is exactly as it should be. It is from a branch, it is rebased and hopefully all tests pass :)