X Tutup
Skip to content

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

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

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

Conversation

@jimlinntu
Copy link
Contributor

@jimlinntu jimlinntu commented Jan 12, 2021

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)
🦁

Copy link
Member

@thaJeztah thaJeztah left a comment

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)

Signed-off-by: Jim Lin <b04705003@ntu.edu.tw>
@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
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)

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@thaJeztah thaJeztah left a comment

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

X Tutup