daemon returns errors when receiving unsupported prune filters by gdevillele · Pull Request #33023 · moby/moby

gdevillele · 2017-05-04T18:10:50Z

- What I did

container prune
volume prune
image prune
network prune

...now return errors if an unsupported filter is received.

container prune only accepts the following filters:

label
until

volume prune only accepts the following filters:

label

image prune only accepts the following filters:

dangling
label
until

network prune only accepts the following filters:

label
until

- How to verify it

Example:

docker container prune --filter <filter>=<key>=<value>

if is not "label" or "until", you should get a daemon error.

- Description for the changelog

return errors to the client when receiving unsupported prune filters

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

cc @thaJeztah @vdemeester

- container prune - volume prune - image prune - network prune Signed-off-by: Gaetan de Villele <gdevillele@gmail.com>

allencloud · 2017-05-14T12:19:13Z

daemon/prune.go

@@ -25,6 +25,27 @@ var (
 	// ErrPruneRunning is returned when a prune request is received while
 	// one is in progress
 	ErrPruneRunning = fmt.Errorf("a prune operation is already running")


I am also wondering if we could disable the export of error ErrPruneRunning ?
Since it is only used here.
But maybe out of the scope of this PR. 🐱

Thanks.
Might be a candidate for another PR.

thaJeztah · 2017-05-15T16:44:38Z

daemon/prune.go

+
+	containersAcceptedFilters = map[string]bool{
+		"label":  true,
+		"label!": true,


Should the ! actually be part of the filter name? Not sure how it's handled later on, but looks a bit tricky (i.e. label!=foo.bar would be accepted, but not label != foo.bar, correct?)

I don't think the later is a valid syntax for filters (e.g. filters="label != foo.bar")

@thaJeztah

I agree that this is strange, following our discussion, I will open another PR to rewrite this logic.

I don't think filters can contain any space, either.

Looked up the PR that implemented it, and indeed the format is label!=key=val or label!=key. Negative matches on value alone don't seem to be supported (i.e., no label=key!=val), nor spaces in between (label != key=val etc.)

moby/cli/command/utils.go

Lines 95 to 116 in 7025247

for _, f := range dockerCli.ConfigFile().PruneFilters {

parts := strings.SplitN(f, "=", 2)

if len(parts) != 2 {

continue

}

if parts[0] == "label" {

// CLI label filter supersede config.json.

// If CLI label filter conflict with config.json,

// skip adding label! filter in config.json.

if pruneFilters.Include("label!") && pruneFilters.ExactMatch("label!", parts[1]) {

continue

}

} else if parts[0] == "label!" {

// CLI label! filter supersede config.json.

// If CLI label! filter conflict with config.json,

// skip adding label filter in config.json.

if pruneFilters.Include("label") && pruneFilters.ExactMatch("label", parts[1]) {

continue

}

}

pruneFilters.Add(parts[0], parts[1])

}

mlaventure

LGTM

thaJeztah

LGTM

thaJeztah · 2017-05-16T14:54:43Z

@gdevillele can you open an issue for tracking the changes on the way we match the filters? (perhaps write out which formats we want to support, if we want to support whitespace)

gdevillele · 2017-05-16T17:52:08Z

@thaJeztah will do. Thanks for merging!

GordonTheTurtle added the status/0-triage label May 4, 2017

gdevillele changed the title ~~daemon returns error when receiving unsuppo unknown filters in prune functions~~ daemon returns error when receiving unsupported prune filters May 4, 2017

gdevillele changed the title ~~daemon returns error when receiving unsupported prune filters~~ daemon returns errors when receiving unsupported prune filters May 4, 2017

aduermael mentioned this pull request May 4, 2017

prune commands : make sure label filters are considered docker/cli#21

Merged

gdevillele force-pushed the pr-fix-prune-filter branch 2 times, most recently from f83ec12 to 7722e4f Compare May 5, 2017 16:36

daemon refuses unknown filters in prune functions

71760ae

- container prune - volume prune - image prune - network prune Signed-off-by: Gaetan de Villele <gdevillele@gmail.com>

gdevillele force-pushed the pr-fix-prune-filter branch from 7722e4f to 71760ae Compare May 5, 2017 16:37

vdemeester added status/1-design-review and removed status/0-triage labels May 7, 2017

allencloud reviewed May 14, 2017

View reviewed changes

vdemeester requested review from cpuguy83, mlaventure, thaJeztah and vdemeester May 15, 2017 08:42

thaJeztah mentioned this pull request May 15, 2017

Unable to use reference filter in prune commands #33142

Closed

thaJeztah reviewed May 15, 2017

View reviewed changes

mlaventure approved these changes May 15, 2017

View reviewed changes

thaJeztah approved these changes May 16, 2017

View reviewed changes

thaJeztah added status/2-code-review and removed status/1-design-review labels May 16, 2017

thaJeztah merged commit f944183 into moby:master May 16, 2017

GordonTheTurtle added this to the 17.06.0 milestone May 16, 2017

thaJeztah added the impact/changelog label May 16, 2017

thaJeztah added the impact/api label May 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

daemon returns errors when receiving unsupported prune filters#33023

daemon returns errors when receiving unsupported prune filters#33023
thaJeztah merged 1 commit intomoby:masterfrom
gdevillele:pr-fix-prune-filter

gdevillele commented May 4, 2017 •

edited

Loading

Uh oh!

allencloud May 14, 2017

Uh oh!

gdevillele May 15, 2017

Uh oh!

thaJeztah May 15, 2017

Uh oh!

mlaventure May 15, 2017

Uh oh!

gdevillele May 15, 2017

Uh oh!

thaJeztah May 16, 2017 •

edited

Loading

Uh oh!

mlaventure left a comment

Uh oh!

thaJeztah left a comment

Uh oh!

thaJeztah commented May 16, 2017

Uh oh!

gdevillele commented May 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	for _, f := range dockerCli.ConfigFile().PruneFilters {
	parts := strings.SplitN(f, "=", 2)
	if len(parts) != 2 {
	continue
	}
	if parts[0] == "label" {
	// CLI label filter supersede config.json.
	// If CLI label filter conflict with config.json,
	// skip adding label! filter in config.json.
	if pruneFilters.Include("label!") && pruneFilters.ExactMatch("label!", parts[1]) {
	continue
	}
	} else if parts[0] == "label!" {
	// CLI label! filter supersede config.json.
	// If CLI label! filter conflict with config.json,
	// skip adding label filter in config.json.
	if pruneFilters.Include("label") && pruneFilters.ExactMatch("label", parts[1]) {
	continue
	}
	}
	pruneFilters.Add(parts[0], parts[1])
	}

Conversation

gdevillele commented May 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allencloud May 14, 2017

Choose a reason for hiding this comment

Uh oh!

gdevillele May 15, 2017

Choose a reason for hiding this comment

Uh oh!

thaJeztah May 15, 2017

Choose a reason for hiding this comment

Uh oh!

mlaventure May 15, 2017

Choose a reason for hiding this comment

Uh oh!

gdevillele May 15, 2017

Choose a reason for hiding this comment

Uh oh!

thaJeztah May 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlaventure left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented May 16, 2017

Uh oh!

gdevillele commented May 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gdevillele commented May 4, 2017 •

edited

Loading

thaJeztah May 16, 2017 •

edited

Loading