Skip to content

fix --update-order and --rollback-order flags#2927

Merged
thaJeztah merged 1 commit into
docker:masterfrom
jimlinntu:fix_update_rollback_order
Jan 18, 2021
Merged

fix --update-order and --rollback-order flags#2927
thaJeztah merged 1 commit into
docker:masterfrom
jimlinntu:fix_update_rollback_order

Conversation

@jimlinntu

@jimlinntu jimlinntu commented Jan 12, 2021

Copy link
Copy Markdown
Contributor

fixes moby/moby#41581

- What I did
Add missing flags in anyChanged function in two places, resolving moby/moby#41581

- How I did it
Just add the missing flags.

- How to verify it

  • docker service create --update-order start-first --restart-condition none --no-healthcheck --name start-first-test alpine tail -f /dev/null
  • docker inspect start-first-test

(Before this commit)

...
            "UpdateConfig": {
                "Parallelism": 1,
                "FailureAction": "pause",
                "Monitor": 5000000000,
                "MaxFailureRatio": 0,
                "Order": "stop-first"
            },
            "RollbackConfig": {
                "Parallelism": 1,
                "FailureAction": "pause",
                "Monitor": 5000000000,
                "MaxFailureRatio": 0,
                "Order": "stop-first"
            },
            "EndpointSpec": {
                "Mode": "vip"
            }
        },
        "Endpoint": {
            "Spec": {}
        }
    }
]

(After this commit):

...
            "UpdateConfig": {
                "Parallelism": 1,
                "FailureAction": "pause",
                "Monitor": 5000000000,
                "MaxFailureRatio": 0,
                "Order": "start-first"
            },
            "RollbackConfig": {
                "Parallelism": 1,
                "FailureAction": "pause",
                "Monitor": 5000000000,
                "MaxFailureRatio": 0,
                "Order": "start-first"
            },
            "EndpointSpec": {
                "Mode": "vip"
            }
        },
        "Endpoint": {
            "Spec": {}
        }
    }
]

- Description for the changelog

fix --update-order and --rollback-order flags when only --update-order or --rollback-order is provided.

- A picture of a cute animal (not mandatory but encouraged)
🦁

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Change itself looks good, but perhaps you can add a test (see my comment)

Comment thread cli/command/service/opts.go
@jimlinntu
jimlinntu force-pushed the fix_update_rollback_order branch from 2a27929 to 26a6a72 Compare January 18, 2021 14:42
o.update = updateOptions{order: "start-first"}
o.rollback = updateOptions{order: "start-first"}

service, err := o.ToService(context.Background(), &fakeClient{}, flags)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit picking:

assert.NilError(t, err)
expected := "start-first"
assert.Equal(t, service.UpdateConfig.Order, expected)
assert.Equal(t, service.RollbackConfig.Order, expected)

@silvin-lubecki silvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docker service create ignores --update-order

3 participants