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

list the volumes to be deleted without forcing #8990

Closed
wants to merge 61 commits into from

Conversation

iwita
Copy link

@iwita iwita commented Jan 17, 2021

This commit lists the volumes that are going to be removed when the command podman volume prune is issued.
It is applied only in the cases when --force flag is not enabled.
Closes #8913

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: iwita
To complete the pull request process, please assign luap99 after the PR has been reviewed.
You can assign the PR to them by writing /assign @luap99 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@iwita iwita changed the title list the volumes to be deleted without forcing, Fixes: # list the volumes to be deleted without forcing Jan 17, 2021
@@ -58,7 +59,18 @@ func prune(cmd *cobra.Command, args []string) error {
}
if !force {
reader := bufio.NewReader(os.Stdin)
fmt.Println("WARNING! This will remove all volumes not used by at least one container.")
fmt.Println("WARNING! This will remove all volumes not used by at least one container. The following volumes will be removed:")
listOptions.Filter, err = filters.ParseFilterArgumentsIntoFilters(filter)
Copy link
Member

Choose a reason for hiding this comment

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

This would list all volumes, not just the ones that would be removed. podman volume prune is going to prune the volumes that are not in use.

I believe you would need to have a filter to show volumes that are not in use.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @rhatdan.
Yes, you are right. Didn't consider the actual use of the prune command.
Now I added a function in cmd/podman/volumes/prune.go in order to get the used volumes and remove them from the available ones for pruning.

@rhatdan
Copy link
Member

rhatdan commented Jan 17, 2021

I would prefer to add a filter command, Put this on the server side.

podman volume ls --filter unused

For example. Then use this to fix list volumes

@iwita
Copy link
Author

iwita commented Jan 17, 2021

I would prefer to add a filter command, Put this on the server side.

podman volume ls --filter unused

For example. Then use this to fix list volumes

@rhatdan Thank you very much for your guidance.
I applied the needed changes by adding another filter in pkg/domain/filters/volumes.go
However, when I tried to use this newly created filter for the purposes of this PR, I found out that the Volumes function in https://github.com/containers/podman/blob/master/libpod/runtime_volume.go#L113 uses an OR statement when validating a Volume. Thus, when the custom unused=1 filter is applied(internally) together with user-specified filters (e.g. volume labels), if either of them is true, the volume is printed (which is unwanted).

@lsm5
Copy link
Member

lsm5 commented Jan 26, 2021

@iwita all commits need to have DCO/commit signoff else Smoke Test will complain.

@iwita iwita force-pushed the ls-before-deleting branch 3 times, most recently from 10024f9 to 384b6f4 Compare January 26, 2021 22:37
Signed-off-by: Achilleas Tzenetopoulos <[email protected]>
@iwita iwita force-pushed the ls-before-deleting branch 2 times, most recently from 40750fe to c180fd5 Compare January 26, 2021 22:55
iwita and others added 17 commits January 27, 2021 01:00
Signed-off-by: Achilleas Tzenetopoulos <[email protected]>
This function is now used for the port and rename command.
Rename it to AutocompleteContainerOneArg.

Signed-off-by: Paul Holzinger <[email protected]>
When doing a podman images, manifests lists look just like images, so
it is logical that users would assume that they can just podman push them
to a registry.  The problem is we throw out weird errors when this happens
and users need to somehow figure out this is a manifest list rather then
an image, and frankly the user will not understand the difference.

This PR will make podman push just do the right thing, by failing over and
attempting to push the manifest if it fails to push the image.

Fix up handling of manifest push

Protocol should bring back a digest string, which can either be
printed or stored in a file.

We should not reimplement the manifest push setup code in the tunnel
code but take advantage of the api path, to make sure remote and local
work the same way.

Signed-off-by: Daniel J Walsh <[email protected]>
Signed-off-by: Daniel J Walsh <[email protected]>
Set `-mod=vendor` when generating the bindings.  We expect all
dependencies to be vendored already.  This should slightly speed
up the bindings generation and prevent redundant network accesses.

Signed-off-by: Valentin Rothberg <[email protected]>
Run `go generate ./pkg/bindings/...` once for all bindings instead of
generating them separately.  This should speed up bindings generation
as a given package is visited only once, and it fixes containers#8989 by dropping
the use of pushd and popd.

Fixes: containers#8989
Signed-off-by: Valentin Rothberg <[email protected]>
Install golangci-lint to `./bin` instead of `$GOBIN`.  The latter may be
shared with other projects who require a different version.  Having a
shared version of golangci-lint is a reoccurring source of red herrings
on my work station, so I think it's time to split them.

Signed-off-by: Valentin Rothberg <[email protected]>
Detect if the installed version of golangci-lint is outdated and update
it if needed.

Signed-off-by: Valentin Rothberg <[email protected]>
Install ginkgo on demand via `go get -u` rather than keeping a copy it's
entire source code in the vendor dirctory.  The main motivation for that
is to make `golangci-lint` happy which is continuously throwing up on
the import of a program (i.e., ginkgo).  The linter is broken and stupid
as it ignores flags to ignore dirs and ignores build tags (at least some
linters do) which is blocking us from updating to newer versions.

Signed-off-by: Valentin Rothberg <[email protected]>
It turns out an options was added to container exists so it makes sense
to have pods and container exists calls have an optional structure for
options.

Signed-off-by: baude <[email protected]>
Release trigger script failed[1] because the entire script
runs under 'set -e'; so a 'grep -- -dev' that finds no
results will cause a nonzero exit status and hence the
entire script to fail. Work around that.

 [1] https://cirrus-ci.com/task/4541290882793472

Signed-off-by: Ed Santiago <[email protected]>
I'm tired of seeing these every time I run 'make':

   WARNING: go-md2man does not handle node type HTMLSpan

Cause: left-angle-brackets ( < ) in document source

Solution:
  1) backquote-escape those that need to be shown, usually
     ones referring to an argument or email address; or
  2) Actual HTML ( <sup> and <a> ) which are meant to be
     shown in generated HTML docs but can't be shown in
     man pages, we filter out via a sed expression.

Signed-off-by: Ed Santiago <[email protected]>
Add podman network exists command with remote support.

Signed-off-by: Paul Holzinger <[email protected]>
On each PR (with a few exceptions), check the list of git-touched
files, and abort if no tests are added. Include instructions
on how to bypass the check if tests really aren't needed.

Include a hardcoded exception list for PRs that only touch a
well-known subset of "safe" files: docs, .cirrus.yml, vendor,
version, hack, contrib, or *.md. This list is likely to need
tuning over time.

Add a test suite, but not one recognized by the new script
(because it's a "*.t" file), so: [NO TESTS NEEDED]

Signed-off-by: Ed Santiago <[email protected]>
This reverts commit de05e58.

Running `go get -u` will change the local Go module causing CI to fail
as the local git tree is being changed.  Reverting the change for now
until we have a better idea.

Signed-off-by: Valentin Rothberg <[email protected]>
Paul Holzinger and others added 26 commits January 27, 2021 01:29
Add podman manifest exists command with remote support.

Signed-off-by: Paul Holzinger <[email protected]>
docker-client is a library written in Java and used in Eclipse to
speak with Docker API. When endpoint /images/search is called,
HTTP header attribute X-Registry-Auth has value "null". This is for
sure wrong but Docker tolerates this value, and call works. With this
patch call works also with Podman. containers#7857

Signed-off-by: Milivoje Legenovic <[email protected]>
Bump golang.org.x/cyrpto to the latest

Signed-off-by: root <[email protected]>
Signed-off-by: TomSweeneyRedHat <[email protected]>
if a CNI network is added to the container, use the IP address in that
network instead of hard-coding the slirp4netns default.

commit 5e65f0b introduced this
regression.

Closes: containers#9065

Signed-off-by: Giuseppe Scrivano <[email protected]>
Error looks like:

  # github.com/containers/podman/pkg/api/handlers/swagger
  src/github.com/containers/podman/pkg/api/handlers/swagger/swagger.go:169:3: undefined: libpod.InspectVolumeData

[NO TESTS NEEDED]

Signed-off-by: Reinhard Tartler <[email protected]>
when doing a network creation, the dnsname plugin should be disabled
when the --internal bool is set.  a warning is displayed if this
happens and docs are updated.

Signed-off-by: baude <[email protected]>
Update the completion script like spf13/cobra#1249.

[NO TESTS NEEDED]

Fixes containers#8829

Signed-off-by: Paul Holzinger <[email protected]>
Copied from @TeeVenDick patch containers#9072

Signed-off-by: Daniel J Walsh <[email protected]>
Bumps [github.com/google/uuid](https://github.com/google/uuid) from 1.1.5 to 1.2.0.
- [Release notes](https://github.com/google/uuid/releases)
- [Commits](google/uuid@v1.1.5...v1.2.0)

Signed-off-by: dependabot-preview[bot] <[email protected]>
Signed-off-by: Daniel J Walsh <[email protected]>
- stop: test --all and --ignore (containers#9051)
- build: test /run/secrets (containers#8679, but see below)
- sensitive mount points: deal with 'stat' failures
- selinux: confirm useful diagnostics on unknown labels (containers#8946)

The 'build' test is intended as a fix for containers#8679, in which
'podman build' does not mount secrets from mounts.conf.
Unfortunately, as of this writing, 'podman build' does
not pass the --default-mounts-file option to buildah,
so there's no reasonable way to test this path. Still,
we can at least confirm /run/secrets on 'podman run'.

The /sys thing is related to containers#8949: RHEL8, rootless, cgroups v1.
It's just a workaround to get gating tests to pass on RHEL.

Signed-off-by: Ed Santiago <[email protected]>
There was a potential race where two handlers could be added at
the same time. Go Maps are not thread-safe, so that could do
unpleasant things. Add a mutex to keep things safe.

Also, swap the order or Register and Start for the handlers in
Libpod runtime created. As written, there was a small gap between
Start and Register where SIGTERM/SIGINT would be completely
ignored, instead of stopping Podman. Swapping the two closes this
gap.

Signed-off-by: Matthew Heon <[email protected]>
The --default-mounts-file path was not being handled in
podman build.  This will enable it to use for testing.

Signed-off-by: Daniel J Walsh <[email protected]>
Currently podman implements --override-arch and --overide-os
But Podman has made these aliases for --arch and --os.  No
reason to have to specify --override, since it is clear what
the user intends.

Currently if the user specifies an --override-arch field but the
image was previously pulled for a different Arch, podman run uses
the different arch.  This PR also fixes this issue.

Fixes: containers#8001

Signed-off-by: Daniel J Walsh <[email protected]>
podman-remote search had some FIXMEs in tests that were failing.
So I reworked the search handler to use the local abi.  This
means the podman search and podman-remote search will use the
same functions.

While doing this, I noticed we were just outputing errors via
logrus.Error rather then returning them, which works ok for
podman but the messages get lost on podman-remote.  Changed
the code to actually return the error messages to the caller.

This allows us to turn on the remaining podman-remote FIXME
tests.

Signed-off-by: Daniel J Walsh <[email protected]>
The podman documentation site uses javascript to display
API documentation at:

http://docs.podman.io/en/latest/Reference.html

As input, the javascript sources from a CORS-enabled Google Cloud
Storage object.  This commit ensures the storage object is present and
updated for every Cirrus-CI execution context: Tags, Branches, and PRs.

As of this commit, the documentation site only utilizes the object
uploaded by the Cirrus-CI run on the `master` branch:
`swagger-master.yaml`.  The file produced and uploaded due to a PR is
intended for testing purposes: Confirm it's generation and uploading are
both functional.

Signed-off-by: Chris Evich <[email protected]>
Add an API to libpod to resolve a path on the container.  We can
refactor the code that was originally written for copy.  Other
functions are requiring a proper path resolution, so libpod seems
like a reasonable home for sharing that code.

Signed-off-by: Valentin Rothberg <[email protected]>
A container's workdir can be specified via the CLI via `--workdir` and
via an image config with the CLI having precedence.

Since images have a tendency to specify workdirs without necessarily
shipping the paths with the root FS, make sure that Podman creates the
workdir.  When specified via the CLI, do not create the path, but check
for its existence and return a human-friendly error.

NOTE: `crun` is performing a similar check that would yield exit code
127.  With this change, however, Podman performs the check and yields
exit code 126.  Since this is specific to `crun`, I do not consider it
to be a breaking change of Podman.

Fixes: containers#9040
Signed-off-by: Valentin Rothberg <[email protected]>
The podman API implementation only accepts image uploads with the
applicatoin/x-tar content type, however the generated swagger
documentation currently states this should be a form encoded file with
the content type application/x-www-form-urlencoded which does not work.

Signed-off-by: Ryan Goodfellow <[email protected]>
we have not updated the state of the restful service. it is no longer
considered under development.  additionally, clarified our support of
remote clients.

Fixes: containers#9104

Signed-off-by: baude <[email protected]>
Signed-off-by: Achilleas Tzenetopoulos <[email protected]>
Signed-off-by: Achilleas Tzenetopoulos <[email protected]>
@iwita iwita closed this Jan 26, 2021
@iwita iwita deleted the ls-before-deleting branch January 26, 2021 23:45
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFE: Let podman volume prune show the volumes that are going to be removed