Don't resolve image by contacting registry for docker stack deploy tests#33386
Don't resolve image by contacting registry for docker stack deploy tests#33386nishanttotla wants to merge 1 commit intomoby:masterfrom
Conversation
e7fe5d4 to
9a3433f
Compare
9a3433f to
f65f813
Compare
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
f65f813 to
b1e9dfd
Compare
|
Also updated tests that were failing because they didn't account for the default |
|
Ping @aaronlehmann @vdemeester @tiborvass this PR is ready now. I'm not sure if it's okay to update the CLI commit hash, but the tests should be updated (note that these fixes were already part of the cherry picks into RC2 and RC3 for 17.06). |
|
LGTM |
thaJeztah
left a comment
There was a problem hiding this comment.
I'd like to have consensus on our approach for bumping the CLI first; I'll see if we can discuss this ASAP
|
@thaJeztah sounds good. I updated CLI here to make sure tests pass. |
| # CLI | ||
| DOCKERCLI_REPO=https://github.com/docker/cli | ||
| DOCKERCLI_COMMIT=3dfb8343b139d6342acfd9975d7f1068b5b1c3d3 | ||
| DOCKERCLI_COMMIT=4d980880f30eddec01dbe4328e0cd656a52b502b |
There was a problem hiding this comment.
i'm not sure about this :S
tiborvass
left a comment
There was a problem hiding this comment.
I suggest changing the scripts to install docker cli by downloading the released 17.06 static binary. Probably in a separate PR to not conflate the two problems.
|
Let's not block this PR because we want to make changes to how the CLI is installed. Without these changes to the tests, the tests won't run correctly with the released 17.06 binary anyway. |
|
Has a decision been made about this one? |
|
I agree we should open a PR to pin the client at the 17.06 release version. These tests should be deleted, not updated. They're only testing the cli, so they are not providing any value in this repo. They won't catch any regressions to the cli because they are not run as part of cli changes. |
This PR turns off digest pinning for
docker stack deployintegration-cli tests.It should be merged as soon as the CLI changes of docker/cli#121 are merged and included.
We did a similar thing for
docker service createandupdatein #33274NOTE: The flag that's being added depends on docker/cli#121, so I've updated the CLI commit.