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

Updating a service does not preserve Arguments #169

Closed
ghost opened this issue Nov 1, 2017 · 3 comments
Closed

Updating a service does not preserve Arguments #169

ghost opened this issue Nov 1, 2017 · 3 comments
Assignees
Labels
kind/bug Something isn't working
Projects
Milestone

Comments

@ghost
Copy link

ghost commented Nov 1, 2017

Steps to reproduce:

  1. Create a service with an image, passing arguments i.e. - docker service create --replicas 1 --name pinger alpine ping docker.com
  2. In Swarmpit, update the service, set the replicas to 2, and Save the service

What happens:

  • service fails to restart because arguments are no longer being passed (compare .Spec and .PreviousSpec to see clearly)
          ...
          "TaskTemplate": {
                "ContainerSpec": {
                    "Image": "alpine:latest@sha256:b40e202395eaec699f2d0c5e01e6d6cb8e6b57d77c0e0221600cf0b5940cf3ab",
                    "Args": [
                        "ping",
                        "docker.com"
                    ],
                    "DNSConfig": {},
                    ...

vs

            ...
            "TaskTemplate": {
                "ContainerSpec": {
                    "Image": "alpine:latest",
                    "StopGracePeriod": 10000000000,       //// missing 'Args' Node+children
                    "DNSConfig": {},
                     ...

What should happen:

  • arguments for the service ( .Spec.TaskTemplate.ContainerSpec.Args) should be preserved
@nohaapav
Copy link
Member

nohaapav commented Nov 1, 2017

This is mainly because swarmpit does not support arguments yet. Will add support as well ;)

@nohaapav nohaapav self-assigned this Nov 1, 2017
@nohaapav nohaapav added the kind/bug Something isn't working label Nov 1, 2017
@nohaapav nohaapav added this to the 1.3 milestone Nov 1, 2017
@nohaapav
Copy link
Member

nohaapav commented Nov 2, 2017

@mpsmith Hey Matthew, should be ok now. Please check latest version.

This finding was actually more than just argument issue.
There was a premise in the beginning that services should be created only via swarmpit. If this is the case service data should be preserved and working well during update. But if service is created via cli with unknown data for swarmpit(e.g. arguments) these were cut off during update.

Thanks to your discovery i've added merge function which basically take original service(before update) and apply update delta. This should prevent data loss if service is created within cli or any other tool with service data not supported in swarmpit yet.

@nohaapav nohaapav added this to In Progress in 1.3 Nov 2, 2017
@nohaapav nohaapav moved this from In Progress to Done in 1.3 Nov 2, 2017
@ghost
Copy link
Author

ghost commented Nov 2, 2017

@nohaapav Thanks! I like your overall approach of applying a delta to the existing state. That seems like it should cover other things that might pop-up.

@nohaapav nohaapav closed this as completed Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
No open projects
1.3
Done
Development

No branches or pull requests

1 participant