Browse code
Fix issue in API `POST /services/(id or name)/update`
This fix tries to address the issue raised in 26090 where
remote API `POST /services/(id or name)/update` cannot
use `name` to update. This is not consistent with the
documentation of the remote API.
This fix fixes this issue by performing a lookup with `getService`
in case `name` instead of `id` is used in API.
This fix adds an integration test to cover the changes.
This fix fixes 26090.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Yong Tang authored on 2016/08/29 13:15:03Showing 2 changed files
| ... | ... |
@@ -892,7 +892,7 @@ func (c *Cluster) GetService(input string) (types.Service, error) {
|
| 892 | 892 |
} |
| 893 | 893 |
|
| 894 | 894 |
// UpdateService updates existing service to match new properties. |
| 895 |
-func (c *Cluster) UpdateService(serviceID string, version uint64, spec types.ServiceSpec, encodedAuth string) error {
|
|
| 895 |
+func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec types.ServiceSpec, encodedAuth string) error {
|
|
| 896 | 896 |
c.RLock() |
| 897 | 897 |
defer c.RUnlock() |
| 898 | 898 |
|
| ... | ... |
@@ -913,6 +913,11 @@ func (c *Cluster) UpdateService(serviceID string, version uint64, spec types.Ser |
| 913 | 913 |
return err |
| 914 | 914 |
} |
| 915 | 915 |
|
| 916 |
+ currentService, err := getService(ctx, c.client, serviceIDOrName) |
|
| 917 |
+ if err != nil {
|
|
| 918 |
+ return err |
|
| 919 |
+ } |
|
| 920 |
+ |
|
| 916 | 921 |
if encodedAuth != "" {
|
| 917 | 922 |
ctnr := serviceSpec.Task.GetContainer() |
| 918 | 923 |
if ctnr == nil {
|
| ... | ... |
@@ -922,10 +927,6 @@ func (c *Cluster) UpdateService(serviceID string, version uint64, spec types.Ser |
| 922 | 922 |
} else {
|
| 923 | 923 |
// this is needed because if the encodedAuth isn't being updated then we |
| 924 | 924 |
// shouldn't lose it, and continue to use the one that was already present |
| 925 |
- currentService, err := getService(ctx, c.client, serviceID) |
|
| 926 |
- if err != nil {
|
|
| 927 |
- return err |
|
| 928 |
- } |
|
| 929 | 925 |
ctnr := currentService.Spec.Task.GetContainer() |
| 930 | 926 |
if ctnr == nil {
|
| 931 | 927 |
return fmt.Errorf("service does not use container tasks")
|
| ... | ... |
@@ -936,7 +937,7 @@ func (c *Cluster) UpdateService(serviceID string, version uint64, spec types.Ser |
| 936 | 936 |
_, err = c.client.UpdateService( |
| 937 | 937 |
ctx, |
| 938 | 938 |
&swarmapi.UpdateServiceRequest{
|
| 939 |
- ServiceID: serviceID, |
|
| 939 |
+ ServiceID: currentService.ID, |
|
| 940 | 940 |
Spec: &serviceSpec, |
| 941 | 941 |
ServiceVersion: &swarmapi.Version{
|
| 942 | 942 |
Index: version, |
| ... | ... |
@@ -1168,3 +1168,21 @@ func (s *DockerSwarmSuite) TestApiSwarmRestartCluster(c *check.C) {
|
| 1168 | 1168 |
|
| 1169 | 1169 |
checkClusterHealth(c, nodes, mCount, wCount) |
| 1170 | 1170 |
} |
| 1171 |
+ |
|
| 1172 |
+func (s *DockerSwarmSuite) TestApiSwarmServicesUpdateWithName(c *check.C) {
|
|
| 1173 |
+ d := s.AddDaemon(c, true, true) |
|
| 1174 |
+ |
|
| 1175 |
+ instances := 2 |
|
| 1176 |
+ id := d.createService(c, simpleTestService, setInstances(instances)) |
|
| 1177 |
+ waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, instances) |
|
| 1178 |
+ |
|
| 1179 |
+ service := d.getService(c, id) |
|
| 1180 |
+ instances = 5 |
|
| 1181 |
+ |
|
| 1182 |
+ setInstances(instances)(service) |
|
| 1183 |
+ url := fmt.Sprintf("/services/%s/update?version=%d", service.Spec.Name, service.Version.Index)
|
|
| 1184 |
+ status, out, err := d.SockRequest("POST", url, service.Spec)
|
|
| 1185 |
+ c.Assert(err, checker.IsNil) |
|
| 1186 |
+ c.Assert(status, checker.Equals, http.StatusOK, check.Commentf("output: %q", string(out)))
|
|
| 1187 |
+ waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, instances) |
|
| 1188 |
+} |