backend.list does not return all backends when labels are used

[fgsch]

Reported by @michbsd on IRC.

Test:

varnishtest "backend.list does not return all backends when labels are used" 

varnish v1 -vcl {
        backend b1 {
                .host = "1.2.3.4";
        }
} -start
 
varnish v1 -cliok "vcl.label vclA vcl1"
 
varnish v1 -vcl {
        backend b2 {
                .host = "5.6.7.8";
        }
        sub vcl_recv {
                return (vcl(vclA));
        }
}
 
varnish v1 -cliok "vcl.label vclB vcl2"
 
process p1 "varnishadm -n ${v1_name} backend.list" -run
 
shell "grep -q vcl2.b2 ${tmpdir}/p1/stdout"
shell "grep -q vcl1.b1 ${tmpdir}/p1/stdout"

[fgsch]

Currently the dependency information is not available outside vclprog.

[michbsd]

Right - any idea when this will amended?

[fgsch]

@michbsd I cannot give you a date but hopefully this will be reviewed at the next bugwash either this or next week, so sit tight!

[dridi]

More details on the actual problem here: this is not a bug unless we decide it's one, and I don't think this was discussed properly during the bugwash.

This is rather an open question: when we run backend.list without parameters, it lists the backends belonging to the active VCL. Do we want to list backends from VCLs referenced with labels from the active VCL as well?

Should we broaden the meaning of active in the context of VCL switching using labels?

Another possibility could be to add a flag like -L to follow labels, making it explicit.

[fgsch]

From my point of view this is a bug and we should treat it as such.
Adding a parameter to print the backends in labels makes little to no sense.

Any backend used directly or indirectly in the active VCL should be displayed following pre-labels
behaviour and POLA.

[bsdphk]

I am going to blackball this ticket.

Yes, it sounds obvious and intuitive up front, but once you start to think through details, for instance related to backend.list patterns, it becomes very hairy, very fast.

I will also argue that at this point we do not have sufficient operational experience with labels to make the right decisions with any reasonable probability.

(This should also be viewed through the prism of the "global backends" we have been talking about at VDDs, but not gotten around to implement yet.)

[hermunn]

Backport review: Labels are 5.0 only, and there were no commits.

[hermunn]

PHK wrote:

I will also argue that at this point we do not have sufficient operational experience with labels to make the right decisions with any reasonable probability.

I am reopening this ticket now, hoping that we have gained enough experience with labels, and can find a better solution for this. Some points:

• It is not possible to see which of the in-use backends (for the time being defined as backends that can get requests as a result of the active VCL running) are healthy or sick. Users who use monitoring tools cannot use backend.list as effectively when using labels.
• Setting a backend in a label to sick is possible but difficult. If you inspect the VCLs for your labels, you can find the names of the backends, and use backend.set_health. Again this is unsatisfactory, as there is no way to see the health without temporarily switching VCLs.

For the reasons above I consider this a bug. If we can agree on what the correct behavior should be, then I can attempt a pull request for its implementation.

My current view is that the first column of the output of backend.list should give you the right name for the backend.set_health command, and that all in-use backends should be present in the output. This does not change the output for users who do not use labels, so I think we should change the default behavior.

[bsdphk]

That sounds like something we can work with.

Unanswered: Should/Does backend.set_health (and other commands) work through labels ?

[mbgrydeland]

Summary from discussion at bugwash:

• Generally we think of labels as symlinks. Ie 'vcl.use ' will evaluate the labelname at immediately, and map to a real VCL execution. (VCLs 'return (vcl())' evaluates on execution of that statement)

• It should be possible to replace a valid label wherever a VCL name is used.

• We suggest adding a -r (recursive) option to backend.list to allow recursing over all the backends that can be reached from a given input VCL.

• Suggestion: -r will list a backend once regardless of how many of the VCLs it can be reached from, and -R will list the backend as many times as it was referenced.

• No consensus on output format. Suggestions:

  • Adding a column showing which VCL name it came from,
  • Doing headings for each VCL name before listing the ones from that.

• The JSON version needs to be thought of as well

• Some mock-ups of possible outputs would be great if anyone has the time to make them

[bsdphk]

Just a note: I think I said in the bugwash that the label is resolved when the command vcl.use some_label is run. It is not, it is resolved whenever the active VCL is grabbed.

I have looked over the code this morning, and I do not think that causes any problems.

I can't quite make up my mind if I think it is more or less POLA that you can change the active VCL with a vcl.label commmand, or if issuing vcl.use foo only to see bar become the active VCL because foo was a label.

I think I lean retaining current behaviour.

[bsdphk]

We decided that vcl.use should resolve labels when executed.

[bsdphk]

...and looking at the code, there seems to be no locking penalty for active vcl being a label, so I dont think changing harmless behaviour which people may depend on is a good idea.

[bsdphk]

Bugwash: summary above is consensus, move ticket to vip, close here.

[dridi]

We decided that vcl.use should resolve labels when executed.

I think it goes against the original intent and against operations to do that.

If by resolving you mean having the target VCL as the active VCL instead of the label. That means that if your production label is in use, then reload_prod_${DATE} is effectively active and labeling a previous or next version of the production label has no effect on the active VCL.

A net loss.

It's as if you said return(vcl(www)) was changed at compile time to return(vcl(reload_www_${DATE})), what's the point then of having a label?

[nigoroll]

@bsdphk I agree with @dridi's last feedback, label assignments need to be dynamic, anything else contradicts the reason we got labels in the first place.
I would not go ahead and move @mbgrydeland's ummary to a VIP.

OK?

[bsdphk]

Now VIP29: https://github.com/varnishcache/varnish-cache/wiki/VIP29:-VCL-labels-and-backend.list-(-r-option-etc.)