Marathon constraints filtering

[aantono]

What does this PR do?

Add support for filtering based on Marathon-defined constraints, as described in https://mesosphere.github.io/marathon/docs/constraints.html

Motivation

Many Marathon clusters have applications deployed with various "constraints", configuring things like application service affinity, cluster binding, etc. It would be very helpful to not have to add additional/duplicate traefik.tags labels to the applications and to be able to filter using already defined Marathon constraints instead.

More

• Added/updated tests
• Added/updated documentation

[ldez]

@timoreimann WDYT ?

[timoreimann]

The feature definitely makes sense to me since Marathon constraints are a very canonical way to filter with this provider.

I left a few comments.

[timoreimann]

Since this is a boolean flag, can we have a verb-ish prefix? How about something like filterMarathonConstraints?

[timoreimann]

I believe we can drop the right-hand expression and merge the left-hand one with the previous line like this:

if p.MarathonConstraints && app.Constraints != nil {
    for _, list := range *app.Constraints {

[timoreimann]

Can we group by expected outcome, going with false first? Nevermind actually, it doesn't make sense here.

[timoreimann]

I think we should elaborate just a bit what "enable" means (i.e., that we parse Marathon constraints and treat the compound constraint parts as a single tag to match literally).

[aantono]

Done

[timoreimann]

Shall we phrase this as Enable filtering by Marathon constraints.?

[timoreimann]

Looking at the function signature, it might not be immediately clear to the caller that colons must be used as separators. I know it's only a few lines below, but how about making the function variadic at the second parameter (i.e., constraint(value1 string, values... string) func(*marathon.Application)) to increase clarity?

[aantono]

I was really trying to keep it as a single string, as in the Marathon config (not using the json API), it is defined as a single string with 3 components, separated with a :. Even the Marathon UI uses just a single text field to add the constrain labels, so I'd rather keep this aligned, to be consistent.

[timoreimann]

That's a reasonable argument. Let's keep a single string then.

[timoreimann]

Can we rename list to constraintParts? That's also how the official documentation calls them.

[timoreimann]

Should we omit marathonConstraint (and possibly other parameters) when they are not needed / relevant for a test?

[timoreimann]

Some small bits left.

[timoreimann]

Shall we phrase this as Enable filtering by Marathon constraints.?

[timoreimann]

Typo: constraints

[timoreimann]

Typo: verbatim

[timoreimann]

We need to update the name

[timoreimann]

We can omit the last expression on the constraint length.

[timoreimann]

Let's prepend this by filter too to match the real flag name.

[timoreimann]

LGTM -- thanks, Alex. 👏

[juliens]

LGTM

[ldez]

LGTM