Skip to content

[27.x backport] volume/update: require 1 argument/fix panic#5426

Merged
laurazard merged 2 commits intodocker:27.xfrom
thaJeztah:27.x_backport_fix-panic-volume-update
Sep 11, 2024
Merged

[27.x backport] volume/update: require 1 argument/fix panic#5426
laurazard merged 2 commits intodocker:27.xfrom
thaJeztah:27.x_backport_fix-panic-volume-update

Conversation

@thaJeztah
Copy link
Member

- What I did

This command was declaring that it requires at least 1 argument, when it needs exactly 1 argument. This was causing the CLI to panic when the command was invoked with no argument.

- How I did it

Require exactly 1 argument.

- How to verify it

  • docker volume update

- Description for the changelog

Fix issue where `docker volume update` command would cause the CLI to panic if no argument/volume was passed.

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

This command was declaring that it requires at least 1 argument, when it
needs exactly 1 argument. This was causing the CLI to panic when the
command was invoked with no argument:

`docker volume update`

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
(cherry picked from commit daea277)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 27.2.2 milestone Sep 11, 2024
@thaJeztah thaJeztah self-assigned this Sep 11, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.81%. Comparing base (90559a6) to head (965699b).
Report is 26 commits behind head on 27.x.

Additional details and impacted files
@@            Coverage Diff             @@
##             27.x    #5426      +/-   ##
==========================================
+ Coverage   59.77%   59.81%   +0.04%     
==========================================
  Files         345      345              
  Lines       23405    23396       -9     
==========================================
+ Hits        13990    13994       +4     
+ Misses       8445     8432      -13     
  Partials      970      970              
@thaJeztah
Copy link
Member Author

Ah! Failing because the 27.x branch doesn't have some of the UX improvements we made;

59.13 === FAIL: cli/command/volume TestUpdateCmd (0.00s)
59.13     update_test.go:21: assertion failed: expected error to contain "requires 1 argument", got "\"update\" requires exactly 1 argument.\nSee 'update --help'.\n\nUsage:  update [OPTIONS] [VOLUME] [flags]\n\nUpdate a volume (cluster volumes only)"
The error-message changed in newer versions, and no longer includes
"exactly".

This patch adjusts the test in the meantime.

    59.13 === FAIL: cli/command/volume TestUpdateCmd (0.00s)
    59.13     update_test.go:21: assertion failed: expected error to contain "requires 1 argument", got "\"update\" requires exactly 1 argument.\nSee 'update --help'.\n\nUsage:  update [OPTIONS] [VOLUME] [flags]\n\nUpdate a volume (cluster volumes only)"

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

I pushed a commit to adjust the test for now. We can revert that commit it we decide to backport the improvements.

@laurazard
Copy link
Member

Sounds good to me, thanks for taking care of this!

@laurazard laurazard merged commit fba240c into docker:27.x Sep 11, 2024
@thaJeztah thaJeztah deleted the 27.x_backport_fix-panic-volume-update branch September 11, 2024 15:14
renovate bot added a commit to earthly/dind that referenced this pull request Sep 23, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/docker](https://redirect.github.com/docker/docker) | minor |
`27.2.1` -> `27.3.1` |

---

### Release Notes

<details>
<summary>docker/docker (docker/docker)</summary>

###
[`v27.3.1`](https://redirect.github.com/moby/moby/releases/tag/v27.3.1)

[Compare
Source](https://redirect.github.com/docker/docker/compare/v27.3.0-rc.1...v27.3.1)

#### 27.3.1

For a full list of pull requests and changes in this release, refer to
the relevant GitHub milestones:

- [docker/cli, 27.3.1
milestone](https://redirect.github.com/docker/cli/issues?q=sort%3Aupdated-desc+is%3Aclosed+milestone%3A27.3.1)
- [moby/moby, 27.3.1
milestone](https://redirect.github.com/moby/moby/issues?q=sort%3Aupdated-desc+is%3Aclosed+milestone%3A27.3.1)

##### Bug fixes and enhancements

- CLI: Fix issue with command execution metrics not being exported due
to the CLI MeterProvider being shutdown too early.
[docker/cli#5457](https://redirect.github.com/docker/cli/pull/5457)

##### Packaging updates

- Update `Compose` to
[v2.29.7](https://redirect.github.com/docker/compose/releases/tag/v2.29.7)

###
[`v27.3.0`](https://redirect.github.com/moby/moby/releases/tag/v27.3.0)

[Compare
Source](https://redirect.github.com/docker/docker/compare/v27.2.1...v27.3.0-rc.1)

#### 27.3.0

For a full list of pull requests and changes in this release, refer to
the relevant GitHub milestones:

- [docker/cli, 27.3.0
milestone](https://redirect.github.com/docker/cli/issues?q=sort%3Aupdated-desc+is%3Aclosed+milestone%3A27.3.0)
- [moby/moby, 27.3.0
milestone](https://redirect.github.com/moby/moby/issues?q=sort%3Aupdated-desc+is%3Aclosed+milestone%3A27.3.0)

##### Bug fixes and enhancements

- containerd image store: Fix `docker image prune -a` untagging images
used by containers started from images referenced by a digested
reference.
[moby/moby#48488](https://redirect.github.com/moby/moby/pull/48488)
- Add a `--feature` flag to the daemon options.
[moby/moby#48487](https://redirect.github.com/moby/moby/pull/48487)
- Updated the handling of the `--gpus=0` flag to be consistent with the
NVIDIA Container Runtime.
[moby/moby#48483](https://redirect.github.com/moby/moby/pull/48483)

[https://github.com/docker/cli/pull/5432](https://redirect.github.com/docker/cli/pull/5432)5432)
- Support WSL2 mirrored-mode networking's use of interface `loopback0`
for packets from the Windows host.
[moby/moby#48514](https://redirect.github.com/moby/moby/pull/48514)
- Fix an issue that prevented communication between containers on an
IPv4 bridge network when running with `--iptables=false`,
`--ip6tables=true` (the default), a firewall with a DROP rule for
forwarded packets on hosts where the `br_netfilter` kernel module was
not normally loaded.
[moby/moby#48511](https://redirect.github.com/moby/moby/pull/48511)
- CLI: Fix issue where `docker volume update` command would cause the
CLI to panic if no argument/volume was passed.
[docker/cli#5426](https://redirect.github.com/docker/cli/pull/5426)
- CLI: Properly report metrics when run in WSL environment on Windows.
\[[docker/cli#5432](https://redirect.github.com/docker/cli/issues/5432)]

##### Packaging updates

- Update `containerd` (static binaries only) to
[v1.7.22](https://redirect.github.com/containerd/containerd/releases/tag/v1.7.22)
    [moby/moby#48468](https://redirect.github.com/moby/moby/pull/48468)
- Updated `Buildkit` to
[v0.16.0](https://redirect.github.com/moby/buildkit/releases/tag/v0.16.0)
- Update `Compose` to
[v2.29.6](https://redirect.github.com/docker/compose/releases/tag/v2.29.6)
- Update `Buildx` to
[v0.17.1](https://redirect.github.com/docker/buildx/releases/tag/v0.17.1)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge
- At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/earthly/dind).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUiXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment