[08:27:52] ema, bblack: cp1067 flags a diskspace warning since five hours, the biggest chunk is the 4.4 dbg kernel image, but you could probably free up some space by ditching the old 3.16 and 3.19 kernels [08:45:35] moritzm: thanks [08:49:56] I've removed both linux-image-3.16.0-4-amd64 and linux-image-4.4.0-1-amd64-dbg [08:52:28] we also have some huge varnishkafka logs there [08:52:38] -rw-r--r-- 1 varnishlog root 136M Mar 7 06:24 statsv.stats.json.1 [08:52:38] -rw-r--r-- 1 varnishlog root 139M Mar 7 06:24 eventlogging.stats.json.1 [08:52:41] -rw-r--r-- 1 varnishlog root 283M Mar 11 08:51 webrequest.stats.json [08:52:43] -rw-r--r-- 1 varnishlog root 551M Mar 7 06:25 webrequest.stats.json.1 [08:52:46] (under /var/cache/varnishkafka/) [10:27:27] ema: o/ [10:27:54] 'morning! [10:27:58] new channel !! [10:28:03] welcome [10:28:16] so that is where all our l33t people are hiding out [10:29:02] so lets talk about having varnishtest plugged in CI ( https://phabricator.wikimedia.org/T128188 ) [10:29:02] where the cabal meets [10:29:08] yes let's do that [10:29:39] so for CI, we are migrating to instances that are spawned dynamically and deleted after a job is completed. The software supporting that is Nodepool [10:30:00] the instances are Debian/Jessie and are provisioned with some custom puppet script that rely on operations/puppet.git for some bits (such as properly setting up apt) [10:30:08] nice [10:30:16] and yesterday I got varnish installed on those instances. So they get varnish 3.x as pinned by our wmf apt conf [10:30:22] perfect [10:30:26] (although varnish 4 is available, the pin resolve the install to varnish 3) [10:30:36] now we need to run varnishtest I guess :) [10:30:41] the way tests are organized now is pretty simple: they're text files under /usr/share/varnish/tests/$role [10:30:52] and can be executed like this: PATH=/bin:/usr/sbin:/usr/bin varnishtest /usr/share/varnish/tests/maps/*.vtc [10:30:57] now for CI. We have Zuul that defines a workflow for each repositories and determines which job to trigger when a patchset is proposed [10:31:07] thus we need a Jenkins job that do the magic wrapping around varnishtest command [10:32:45] oh [10:32:59] ema: we dont install the role::cache::maps or whatever role::cache::* roles we might have [10:33:10] but we can git clone and checkout the proposed patchset [10:33:33] oh I see [10:33:44] so maybe it is enough to: git clone puppet.git && git checkout PATCH then just find -name '*.vtc' -print 0 | xargs -0 varnishtest [10:34:09] not really, those tests depend on vcl files installed under /etc/varnish/ and /usr/share/varnish/ [10:34:18] another thing we have worked on, is to have the Jenkins job be very dumb. We are making them to invoke a command that is directly in the git repository. [10:34:24] and the files themselves are created according to the role [10:34:32] for example the 'npm' job really does: git clone && git checkout && npm install && npm tst [10:34:52] so potentially for varnish test we could introduce a 'make testvarnish' at the root of the repo [10:35:16] this way Jenkins just invoke that and it gives puppet people the freedom to choose how the command is run / report etc [10:35:26] ah vcl grr [10:35:33] the vcl being erb temlaptes right? [10:35:41] right [10:36:31] see for example modules/varnish/templates/vcl/ [10:37:00] and ./templates/varnish/ [10:38:01] oh [10:38:14] and the VTC files have include "/usr/share/varnish/tests/wikimedia_maps-backend.vcl [10:38:21] so yeah, the tests would need to be executed *after* a puppet run [10:38:27] hashar: correct [10:38:42] which in turn includes stuff from /etc/varnish/ [10:39:07] so how would one run those tests on his local machine? [10:39:19] heh [10:39:25] one wouldn't, I guess [10:39:41] I'm currently running them on a self-hosted puppetmaster [10:39:57] so I essentially do my VCL changes, run puppet agent -tv, run varnishtest [10:40:39] I wonder if we can get a script to bootstrap the vcl using puppet [10:40:43] i.e. without having to install all the role::caches::* on the instances [10:43:21] that would be nice [10:43:39] what I am thinking about is that a script would expand templates using pre set parameters [10:44:09] and vary the hardcoded path to point to the current working directory instead of system wide paths such as /etc or /usr/share/varnish [10:45:22] looking at modules/varnish/manifests/instance.pp the varnish::instance does take care of expanding templates and get them installed system wide, so potentially that can be made to vary [10:45:41] then varnish::instance also set up the service ;-/ [10:49:30] hashar: oh so would you use something like 'puppet apply' locally on the CI node? [10:49:44] there is no root access [10:49:58] that would solve it for sure [10:50:17] but I havent found the time to figure out how to sanely provide some kind of sudo access to the unprivileged jenkins user [10:53:08] mmhh [10:53:39] so the dirty way is to have the CI instances to be provisioned with all the role::cache:** classes we need for testing [10:54:02] that would grow the base image by an order of magnitude and I am not sure we can actually have several such roles installed in paralell [10:54:21] a long way is to figure out how to get proper sudo access [10:54:56] hashar: I don't think you need to be root to run puppet-apply (in general) [10:55:08] $ puppet apply --execute 'notice("my test")' [10:55:08] Notice: Scope(Class[main]): my test [10:55:08] Notice: Compiled catalog for cp1048.eqiad.wmnet in environment production in 0.01 seconds [10:55:09] and the idealistic way would be to refactor all the manifests / vcl / vct so we can populate the files without installing varnish service itself and in a user writable dir [10:55:11] Notice: Finished catalog run in 0.01 seconds [10:55:23] yup [10:55:39] but the manifests do write operations to /usr/ /etc/ etc and that would need root [10:56:01] unless we refactor them to avoid writing there, right? [10:56:09] yeah [10:56:13] that is no so simple though [10:56:18] but would let people test locally as well [10:56:43] no it's not that simple but it's a cleaner approach :) [10:58:28] seems the definitions are handled from modules/role/manifests/cache/* files [10:58:51] if we can figure out a way to grab the varying parameters from those role class and then expand the .erb templates from those parameter that would work [11:00:21] hashar: so then in the job you'd do something like puppet apply modules/role/manifests/cache/tests.pp && varnishtest blablabla ? [11:00:42] yup [11:00:51] but that one would realize a lot of other classes [11:01:20] such as geoip / installing the service / populating stuff under /etc /usr , install lvs real server ip etc [11:02:09] if we got the config parameters extracted out to some manifest such as role::cache::text::config [11:02:16] we could then include that conf and just realize the templates [11:02:32] but that is a lot of refactoring :-/ [11:09:46] $ puppet parser dump --e 'notice("my test"); $foo = 3' [11:09:46] (block [11:09:47] (invoke notice 'my test') [11:09:47] (= $foo 3) [11:09:49] ) [11:09:54] ema: ^^^ parsing the DSL :D [11:10:50] :) [11:14:54] so I think the key would be to decouple the installation of varnish service from the config generation [11:15:24] and have the destination path of expanded VCL to vary [11:15:39] or we do sudo somehow :-D [11:16:47] something that got mentionned a few months ago would be to offer the ability to spawn lightweight containers and grant root access in them [11:17:07] so the test suite would spawn a container, puppet apply in it the role::cache::whatever and run the tests [11:18:42] but that's not going to happen anytime soon I guess [11:18:57] nop :-/ [11:19:19] maybe systemd-nspawn has some trick to boot a container out of the host and grant root access [11:19:38] yeah but you need to be root [11:19:44] fo [11:19:51] for nspawn ? [11:19:56] y [11:20:24] maybe we can grant sudo to a shell script that takes care of spawning such a container [11:20:32] this way the test run in the container [11:20:41] (I am not sure what I am thinking about really) [11:49:04] hashar: perhaps we could find a simple way to just render the VCL templates with some test data somehow? [11:50:39] yeah [11:51:39] and inject some test "@vcl_config" [11:52:27] looking at templates/vcl/wikimedia-frontend.vcl.erb [11:52:42] it just needs @varnish_include_path @vcl and @vcl_config [12:00:03] ema: (echo '<% @vcl="a" %><% @varnish_include_path="/path/to" %><% @vcl_config={ "a" => 2} %>' ; cat templates/vcl/wikimedia-frontend.vcl.erb)|erb -T - [12:00:09] ok that one is crazy [12:00:40] but the erb command line supports loading a ruby file (erb -r fixture.rb) which could populate @ variables to the erb [12:04:03] hashar: right, I think there's no reason to cat actually [12:04:34] erb -T - -r fixture.rb templates/vcl/wikimedia-frontend.vcl.erb [12:04:44] \O/ [12:05:33] that one could be pass to expand vcl and then at least verify the syntax via varnishd -C (compiles to C language) [12:05:47] absolutely [12:06:40] and that can be run right now by using the /Rakefile that has a 'test' task [12:06:49] it is run already on changes made to operations/puppet , the job is 'rake jessie' [12:06:58] and it really "just": bundle install && bundle exec rake test [12:08:03] however we cannot compile single VCL files in general, they might include other ones [12:08:39] so we do need to figure out a good way of rendering them all in some test directory and changing the include lines [12:09:25] similarly to what we already do in wikimedia-frontend.vcl.erb, for example [12:09:32] include "<%= @varnish_include_path %>wikimedia-common_<%= @vcl %>.inc.vcl"; [12:11:48] yeah that is nice [12:11:59] maybe we can get a rake task / some ruby code to do it [12:12:06] bblack, ema: as per our earlier discussion some: I've uploaded backport of linux-tools 4.4-4 (providing kbuild) to carbon [12:15:09] moritzm: nice, thanks! [12:15:24] * ema is hungry [12:15:30] lunch time, bbl! [13:34:21] hashar: thinking about that a little more, we probably do not even need to prefix our includes with @varnish_include_path [13:34:52] I've prepared a patch for that already, but yeah I think it's unnecessary [13:35:47] we can override the directory where varnishd/varnishtest looks for VCL files IIRC [13:36:51] varnish v1 -arg "-p vcc_err_unref=false -p vcl_dir=/tmp/varnish" -vcl+backend { [13:51:29] keep in mind eventually we'll have puppet-compiler integrated with jenkins too, or at least I think that was a plan in the past [13:51:57] puppet-compiler actually can check a set of canary roles/sites with their actual real puppet values [13:52:29] if we assume that eventually happens, it argues for a couple of things: [13:53:00] 1) For the basic jenkins test here, it doesn't matter so much that we cover all cases, just one basic invented case to validate syntax and such [13:53:33] 2) Maybe puppet-compiler can later be extended to run vtc tests in the context of real node catalog compilation [13:54:51] although if it's easy to cover a lot of cases, then sure, let's do several fixtures that mock known configs of current clusters [13:56:07] bblack: the advantage I see in renderning the VCL erb templates with ruby is that we could then have a relatively simple script to run on our development machines to do some basic VCL validation [13:56:15] without puppet and stuff [13:56:48] yeah true [13:57:39] more in general, the idea of rendering all our VCL under, say, /tmp/test_varnish/ would probably simplify the current testing logic [13:57:57] if you want to look at it that way and go all out with the maximum reasonable set, it would be 32 different fixtures [13:58:06] shit [13:58:07] :) [13:58:17] for cluster * site * layer, essentially [13:58:39] so there'd be a fixtured run for text_esams_frontend, and so-on [13:59:52] in other news, we're getting this again: [13:59:53] E: Cache is out of sync, can't x-ref a package file [13:59:55] I guess probably the easiest tradeoff on structuring that would be 8x files for text_frontend, maps_backend, etc... and pull in a second .rb file to define $::site [13:59:56] on cp1057 [14:00:28] I'm running apt-get update ; puppet agent -tv [14:00:30] but at some point, we're getting too close to reinventing what the puppet compiler stuff can do better [14:01:35] bblack: yes, you're right [14:01:50] the risk is reinventing the compiler poorly [14:03:28] (and having to manually keep mock fixture data in sync with the puppet manifests + hieradata) [14:03:39] yep [14:04:08] what sucks is we have a lot of conditional code [14:04:30] but still, we can do one set of basic mock data that tries to turn on all the conditionals it can and check for syntax errs and such [14:04:38] as a low-hanging fruit we can try to identify the minimum set of variables to pass to erb in order to render our templates but without trying to mimic real situations, just to make the VCL compiler happy [14:05:02] or really, I guess I'm thinking backwards about the problem [14:05:23] the point of VCL unit tests shouldn't be to simulate our nodes, it should be to exercise the conditionals in our VCL [14:05:45] so all the vcl_config flags that turn code on and off, like https_redirects and such [14:06:15] make a list of the flags, and have the mock data either iterate every combination (if it's a reasonable number), or just several common sets of them [14:06:53] in other words as a VCL unit test, it should be testing what our VCL code claims it offers as configurable possibilities to real sites/clusters/nodes, not mocking actual sites/clusters/nodes' known configs [14:07:44] the danger in doing just 1x fixed config to check compile at all, is it will fail to test some lines of code excluded by some vcl_config flags, and so we make a syntax error there and see +1 and assume it's ok [14:08:10] I guess this is analogous to having the CI for a C codebase re-run the tests with various --configure flags turning features on and off [14:08:20] right [14:09:40] not that the set of vcl_config flags is even documented as an interface, it's just implicit by what we see conditionals for in our current templates :P [14:10:51] bits_domain, static_host, and upload_host are easy, they can just get singular mock values with "a" "b" "c" or whatever [14:11:19] ditto for purge_host_regex [14:12:04] and ttl_cap ttl_fixed cache_4xx allowed_methods are all in the same bin [14:13:06] do_gzip https_redirects secure_post pass_random <- those 4 are the ifdef-like booleans that conditionally include/exclude code [14:13:31] having it run through all 16x combos of those isn't awful [14:14:56] do_gzip is now on for all clusters, there's an optimization we could do today to remove that one [14:15:34] so 8x combos: the rest legitimately need to vary in the real world for now [14:18:22] so this would be about producing all the possible VCL output starting from our templates [14:18:43] another topic is writing VTC tests to actually follow those codepaths [14:18:54] (and there is no code-coverage indication in varnishtest AFAIK) [14:21:20] yeah [14:21:43] in my mind, the fixtures with vcl_config combos are just for syntax checks [14:22:20] yep, and we should probably have two different jenkins jobs [14:22:24] for real testing, really step one is better outlining what our contracts with the application layer really are [14:22:41] the idea of documenting that better in a few places has been bubbling in the back of my head for a while now [14:22:59] essentially defining the Traffic API in a sense, but it's better called a contract since it's not really an API [14:23:25] defining it in english documentation on wikitech, and building VCL tests around exercising those contracts [14:23:36] and being able to trace back most of our lines of VCL code to supporting one or more of those contracts [14:24:08] if your service emits header X, the traffic infrastructure will do Y [14:24:20] the traffic infrastructure always emits headers Z, A, B to your code [14:24:21] etc [14:25:05] you can define various HTTP (mostly header, but also other) interaction contracts like that, as well as how purging behavior works, etc... [14:25:40] for the common VCL, and then also the extended contracts per-cluster. especially text has a lot of cluster-specific contracts, e.g. about how we handle authenticated users' token/session cookies, etc [14:26:05] defining those for human developers to reference, and defining them in a way that we can regression test with mock applayer service backends, would be a huge step forward [14:26:26] and a goal for another quarter probably :) [14:26:30] :) [14:28:32] the nice thing is if we can get rigorous enough about that, it prepares us much better for future Big Changes. Having those kinds of contracts documented and codified would make a transition to Varnish 5 easier, or ATS or whatever steps up [14:28:54] because those contracts are no longer about varnish, they're about what the black box of Traffic sitting between user and app does. [14:29:09] the VTC tests are just an implementation of testing them [14:29:55] o/ [14:30:24] was talking with gehel about a CI dashboard system he is playing with (teaser: it looks great) [14:31:33] NameError: undefined local variable or method `sip' for #<#:0x007f8148a1d918> [14:31:33] I love error messages [14:31:57] we can even put developer requests in that context: what today is "can you please hack varnish to do X for my service" is now "can we please extend the cluster-specific or general-case contract to support X" [14:33:12] which also means people will have an understandable description of what the caching layer does (VCL code is clearly not intelligible) [14:33:22] right [14:33:54] (we can include in the contracts the basics that lie beneath our VCL, too, which varnish or any replacement does, like honoring Cache-Control in certain mostly-RFC-compliant ways) [14:34:49] some of the contracts go the other way, too, like putting restrictions on applications that if they're going to emit objects with CC ages shorter than X, they need to document that and negotiate it with us and make sure we're all ok with it [14:35:38] I am playing a bit at expanding the vcl.erb templates and providing them a context (eg: different @vcl_config ) [14:35:45] although maybe that's a bit specific to current deficiencies. in an ideal world we don't care, and if we need longer real lifetimes in the cache to support outage scenarios, our VCL bumps the internal TTLs as appropriate just to handle grace-like behaviors [14:35:52] looks like I manage to generate raw vcl now which we could run tests against [14:36:41] (and we put the grace-like behaviors in the contract text too: "hey, if ops has an outage/failure scenario to deal with, we may sometimes cache objects past their lifetime up to X temporarily" [14:40:36] hashar: to check if the resulting VCL compiles you can use something like /usr/sbin/varnishd -p vcc_err_unref=false -p vcl_dir=/tmp/varnish -C -f /tmp/varnish/${filename}.vcl [14:41:02] progress! [14:41:03] (assuming all required VCL files have been rendered under /tmp/varnish/) [14:41:26] good to know varnishd support that [14:43:00] bblack: just to be the party pooper here a little bit, the issue I can see is that the human-readable contract might end up getting outdated compared to our VCL, or whatever implementation we use [14:44:52] well yeah, that gets back to the whole code-as-documentation thing, or embedded documentation [14:45:19] at least having the contract written in some form inside the puppet repo helps, and we can try to be disciplined about updating it in appropriate commits and reviewing for that. [14:45:52] it would still be nice to have a human-written thing on wikitech, but *that* would be hard to keep updated, too [14:46:07] maybe it can just have a pointer to the current docs in ops/puppet [14:53:01] hashar: one more thing, you will probably need to pass -n in order to compile as a non-root user /usr/sbin/varnishd -n /tmp/whatever -d -C -f /etc/varnish/errorpage.inc.vcl -p vcc_err_unref=false -p vcl_dir=/tmp/varnish -f /tmp/varnish/${filename}.vcl [14:53:50] I wonder if we really have unrefs now with other refactors having happened [14:53:56] well I guess we still definitely have unref'd probes [14:55:06] hashar: the proper command would be this one actually /usr/sbin/varnishd -n /tmp/whatever -C -p vcc_err_unref=false -p vcl_dir=/tmp/varnish -f /tmp/varnish/${filename}.vcl [14:55:11] it's kind of an odd leaky abstraction to how this is interpreted into C that unref'd probes are warning and not unref'd backends [14:55:31] bblack: oh I didn't think about that [14:57:02] the pedantic me wants to fix that and abstract away which clusters actually invoke which probes, but it's not woth the effort with vcc_err_unref=false and only like 4 of them existing anyways [14:57:40] really we should only have the varnish probe, and that one gets used everywhere [14:58:29] logstash and wdqs are because misc-web's backends for those are a manual set of N hosts, but to fit the correct pattern we use elsewhere, logstash and wdqs should be pybal/LVS services that pybal probes and manages, and the VCL backend should be foo.svc.eqiad.wmnet without a probe [14:59:31] the reason we can't trivially move them to misc-backend VCL is wikimedia-common is included first and needs them [15:55:26] so my ruby is dirty but I came up with a basic system to expand vcl.erb files [15:55:38] and pass them a very rough fixture context [15:56:13] I basically set dummy variables for stuff like @vcl_config @directors etc [15:56:20] which let ERB expand the templates [15:59:47] hashar: nice [16:00:10] so in theory for each of the vcl.erb template we have, we could forge 1..n fixtures with different contexts [16:00:31] and generate from a template as many expanded VCL we want based on the fixtures context [16:00:38] that is not implemented [16:01:01] should all be done out of the Rakefile and there is probably a way nicer way to do that [16:01:10] as a first test, I guess we can try to turn all flags :) [16:01:17] *all flags on [16:01:22] flags? [16:01:34] yeah as in all the puppet conditionals [16:05:18] hashar: can we see the script? [16:07:12] oh it's on gerrit [16:07:38] ah yeah forgot to paste here sorry https://gerrit.wikimedia.org/r/276733 [16:11:45] yeah I made a list earlier of all the vcl_config vars, but it was ill-formatted and realtime heh [16:12:21] these can be given a single value, they're not really conditionals: bits_domain static_host upload_host purge_host_regex ttl_cap ttl_fixed cache_4xx allowed_methods [16:12:48] these are boolean ifdef-like code conditionals: https_redirects secure_post pass_random [16:13:38] you could set all of the first category to fixed nonsense values, and then permute through the 8x combos of the others [16:14:13] really secure_post is meaningless unless https_redirects is on, but that's getting too detailed for a generic framework for future flags and such [16:14:16] so generate 8 x vcl ? [16:14:52] well run the syntax check 8x times with different flags on/off at the ruby fixture level [16:15:04] I guess that means generate 8x VCL? [16:15:17] yeah I am usually just rephrasing what smart folks are writing [16:15:26] that is a way to make sure I properly understood ;} [16:15:28] aren't we all? :) [16:16:31] if we don't want to get into permutations, of course, we could just turn them all on and assume all-flags-true means maximal code inclusion for syntax check [16:16:46] 16:01 < ema> as a first test, I guess we can try to turn all flags :) [16:16:54] yes that's what I was trying to say [16:16:59] I'm like 15 minutes behind the conversation now after taking a side road trip :) [16:18:00] at this point we "just" want to see if the VCL compiles, so I guess maximal code inclusion would be nice [16:18:07] currently whether cache_route is "direct" or some other value is a boolean too [16:18:15] when looking at secure_post , it is only used in modules/varnish/templates/vcl/wikimedia-frontend.vcl.erb [16:18:22] I am wondering if we could drop the erb conditional [16:18:42] some frontends have it set to false [16:18:54] modules/role/manifests/cache/text.pp: 'secure_post' => false, [16:18:59] and do: if (secure_post && req.request == "POST" && !(req.http.Host ~ "(?i)\.beta\.wmflabs\.org$")) { [16:19:20] well sure, but "secure_post" is not anything legal from a VCL perspective [16:19:21] not sure whether vcl recognizes true / false [16:19:32] yeah sorry my bad [16:19:50] it is interesting though, I honestly don't know if there's some hack for explicit true/false in VCL [16:20:06] I mean: if( <% @vcl_config.fetch('secure_post') %> && req.request == "POST" ... ) [16:20:10] but that does not help for varnishtest [16:20:15] where secure_post can be <%= secure_post %> and evaluate to "if (0 &&" or "if (1 &&" [16:20:28] ideally varnishtest would be able to hold the logic as to whether it want to test with our without secure_post [16:20:49] the problem is VCL doesn't really have generic variables [16:21:11] which is why we often pass values from one routine to another through request headers, even though we have no need/intent of emitting a header anywhere [16:22:10] if VCL even just had a set of vars called "global.foo" which can be set in vcl_init for the lifetime of the loaded VCL, and tested in conditionals and used in string substititions and so-on.... [16:22:15] VCL templating would get way way saner [16:22:41] could it be done with a custom sub ? [16:22:46] * ema feels like a real ruby hacker [16:22:47] File.write('/tmp/emaistesting/' + File.basename(template.filename), renderer.result) [16:22:58] VCL "subs" aren't actual subroutines. they have no arguments or return values. [16:23:24] when ytou define "sub foo { ... }" and later in some real function do "call foo;", all it's really doing is pasting the subroutine code inline [16:23:27] sub has_secure_post { return "<%= secure_post -%>" == "true" } [16:23:58] (i am just throwing random non sense ideas really) [16:24:13] yeah, it's a good idea in any sane language, but VCL is not fully general [16:24:35] the most-confusing aspect of the non-subroutines in VCL is that "return (foo)" in a user-defined sub returns all the way out of the outer scope [16:24:44] there is no early return from a subroutine back to its caller [16:26:28] it's best to think of VCL subroutines this way: [16:27:00] 1. There are predefine entrypoint subroutines named vcl_foo. Varnish internals invoke these, and they eventually return (foo) back to varnish internals, leaving VCL-land. [16:27:25] 2. Any other "sub foo" that's not "sub vcl_foo" is a user-defined subroutine to be "call foo;" from elsewhere (entrypoint subs, or other user-defined subs) [16:27:51] 3. The way "sub foo" actually works is more like a macro for inlined code. They're recursively pasted into the appropriate spot in the top-level "sub vcl_foo" that invokes them. [16:28:35] and then most-horribly is: [16:29:12] 4. include "vcl_foo.inc" just pastes the include file into wherever it was included in the including VCL, which is how everything gets included in the One VCL File specified on the commandline [16:29:16] + [16:29:42] 5. within one file or many include recursions, "sub vcl_foo" entrypoints can be defined multiple times. if they are, they're concatenated in inclusion order into a single linear "sub vcl_foo" [16:29:46] + [16:29:51] * hashar listens [16:30:20] that last concatenation is evil really [16:30:26] 6. There's an implicit default version of each "sub vcl_foo" built into the compiler, which is always concatened at the end of all the user defined ones. The only way to prevent it's execution is to ensure the user-defined ones do "return (foo)" in all cases before reaching its code [16:31:39] for sanity you kinda want unconditional returns at the bottom of vcl_foo to avoid the implicit code that's not otherwise even in your repo to look at and think about. [16:31:54] but on the other hand, the defaults do lots of sane things, and you have to be wary about leaving them out [16:32:24] also, in recent refactors we've eliminated multiple definitions of "sub vcl_foo" in our VCL code, and that's a standard we want to stick to for sanity. [16:32:36] so for varnishtest, we first want to expand the erb templates taking care of the different conditional combos we want [16:32:49] then compose the final vcl file we will want to be test taking care of the order? [16:33:11] oh no. composing of final VCL is done via include statements.. [16:33:42] yeah [16:33:52] hashar: right, we will need to do something like setting @vcl to begin with [16:34:25] which is used eg in modules/varnish/templates/vcl/wikimedia-backend.vcl.erb [16:34:48] right [16:34:49] include "<%= @vcl %>.inc.vcl"; [16:35:06] wikimedia-backend and wikimedia-frontend templates are the only two that are ever the top-level VCL file on the varnish commandline [16:35:13] all others are (often nested) includes from those [16:35:48] so re: single-definition, only wikimedia-(frontend|backend) gets to define "sub vcl_foo". all other files use custom subroutine names that get "call foo;";d [16:36:55] so I guess you will want to define test environments combo in some simple format (like yaml / text file whatever). Then the change I wrote would use those environments to expand the VCL erb templates with each env landing in a different directory. Then run tests against those dirs [16:39:20] though that is already defined via the puppet role classes grblblbl [16:42:35] hashar: I'm trying to compile the VCL for maps-backend starting from your rake target (or whatever they're called) [16:42:49] we need to set @vcl = 'maps-backend' and @ipaddress = '127.0.0.1' [16:42:53] and probably other stuff [16:42:56] my script is very lame. it is really just to expand the erb templates [16:43:14] no, it works! [16:43:51] writing rendered templates to the FS should probably be done in a nicer way, but so far I'm proud of my oneliner File.write('/tmp/emaistesting/' + File.basename(template.filename, ".erb"), renderer.result) [16:44:03] well except the hardcoded path lol [16:44:31] also, we will need to save wikimedia-common.inc.vcl as wikimedia-common_maps-backend.inc.vcl [16:44:31] Dir.mktmpdir() :D [16:45:03] oh yeah Dir.mktmpdir() sounds better [16:45:28] there is also ./modules/varnish/templates/vcl/directors.vcl.tpl.erb which I have no idea what it is for [16:46:33] that one creates /etc/varnish/directors.${instance}.vcl but we can ignore it I guess [16:47:34] for example by setting @varnish_testing=true or something [16:48:15] it's included by wikimedia-common.inc.vcl.erb but with a nice conditional [16:49:34] (even though excluding it would go against the idea of maximal code inclusion heh) [16:51:08] yeah directors.vcl is a whole other complex piece of the puzzle here, and best avoided if possible [16:51:50] (one we haven't touched for v4 yet I don't think, btw?) [16:51:59] I forget if the last one did or not [16:52:24] bblack: untouched [16:52:54] in theory it's trivial, but in practice the double-templating kinda sucks :) [16:56:00] ema: feel free to follow up on the change I proposed [16:56:19] I am going out myself, I feel exhausted [16:56:38] hashar: yeah I'm making a mess of Rakefile as we speak [16:56:40] lets get in touch next week and try to at least get a set of VCL files on which we can run a single varnishtest vtc [16:57:10] also building everything in the /Rakefile is a terrible idea. We would want to move that code to some other directory. But that is not really important right now [16:57:39] hashar: OK, thanks a lot! [16:58:17] and have a nice weekend :) [17:01:09] OMG he is gone now and I don't know how to access attributes in a ruby class /o\ [17:03:08] yeah I'm retarded [17:08:36] ema: I have little ruby understanding myself [17:08:56] marxarelli would know, but he is probably at the office event [17:08:56] oh you're still here :) [17:10:07] hashar: I have the modest goal of accessing one of the attributes set in initialize from within the glob_vcl_files.each loop [17:10:52] but I'm confident I'll manage eventually [17:11:40] ema: maybe try: puts VclContext.vcl_config['bits_domain'] [17:12:06] and use 'next' to short circuit the loop [17:12:41] dosent work :D [17:12:49] hashar: it thinks I'm trying to execute a method [17:13:07] same with renderer.vcl_config [17:13:27] if I wanted to execute a method I would have used parentheses man [17:14:48] puts renderer.inspect [17:14:51] they are there at least [17:14:56] #<#:0x007fb471a538e0 @sip="127.0.0.1", @vcl_config={"bits_domain"=>"bits.wikimedia.org", "static_host"=>"en.wikipedia.org", "upload_domain"=>"upload.wikimedia.org", "purge_host_regex"=>"^(?!upload\\.wikimedia\\.org)"}, @cache_route="fake_cache_route", @varnish_directors={}, @directors={}, @name="Some_name", @hostname="hostname.example.org", @scope=#<#:0x007fb471a53340>> [17:15:02] no clue how to retrieve the @foo though :( [17:15:35] renderer.instance_variable_get(:@vcl_config) [17:15:38] WTF [17:16:25] imagine people doing this for a living [17:16:34] like coding ruby all the time [17:17:37] ema: + puts renderer.instance_variable_get(:@vcl_config) [17:17:48] the @ are instance variables and are not directly accessible [17:18:21] unless we declare them in VclContext with attr_reader [17:18:27] my ruby is lame for sure :D [17:18:38] hashar: s/my// [17:18:46] renderer = template.def_class(VclContext).new [17:18:46] puts renderer.instance_variable_get(:@vcl_config) [17:18:52] yields me: [17:18:52] {"bits_domain"=>"bits.wikimedia.org", "static_host"=>"en.wikipedia.org", "upload_domain"=>"upload.wikimedia.org", "purge_host_regex"=>"^(?!upload\\.wikimedia\\.org)"} [17:18:54] :-} [17:19:00] rushing back home. i am late! [17:19:12] see you next week and thanks again! [17:21:08] like when did they decide that calling a function without parentheses is a cool thing to do? [17:37:59] keep in mind ruby is of japanese origin. you should consider it lucky that calling a function doesn't involve tentacles or crazy game shows. [17:38:55] bblack: renderer.instance_variable_get(:@vcl_config) [17:39:01] enough said [17:50:29] in other news we can now check if a VSL tag exists from varnishapi.py https://github.com/xcir/python-varnishapi/pull/26 [17:50:30] bblack: thanks for the wisdom :D [17:51:08] * elukey picturing ema battling with cthulhu [18:08:53] 10Traffic, 6Operations, 5codfw-rollout, 3codfw-rollout-Jan-Mar-2016: Varnish support for shutting users out of a DC - https://phabricator.wikimedia.org/T129424#2112252 (10BBlack) This basically needs a hieradata switch that defaults to off called something like `cache::traffic_shutdown`, so we can set it o... [18:09:55] there's a relatively-trivial subtask left for the codfw rollout support, above, which I didn't manage to finish up yet [18:10:03] in https://phabricator.wikimedia.org/T129424 that just wikibugs-notified above [18:10:29] if anyone wants to take that on next week, it would be much appreciated, but if not I'll whip it up when I get back, or when I have time in the evening while I'm away [18:22:57] bblack: sure I'll give it a shot [18:23:08] and now I'm off, see you :) [18:23:35] 10Traffic, 6Operations, 5codfw-rollout, 3codfw-rollout-Jan-Mar-2016: Varnish support for shutting users out of a DC - https://phabricator.wikimedia.org/T129424#2112338 (10ema) a:3ema [18:25:04] see you :) [19:07:31] 10Traffic, 6Operations, 6Services, 7Performance: Look into a solution for replaying traffic for load testing - https://phabricator.wikimedia.org/T129682#2112642 (10GWicke) [19:07:48] 10Traffic, 6Operations, 6Services, 7Performance: Look into a solution for replaying traffic for load testing - https://phabricator.wikimedia.org/T129682#2112654 (10GWicke) p:5Triage>3Normal [19:15:22] 10Traffic, 6Operations, 6Services, 7Performance: Look into a solution for replaying traffic for load testing - https://phabricator.wikimedia.org/T129682#2112709 (10Pchelolo) [21:53:12] 7Varnish, 10MobileFrontend, 3Reading-Web-Sprint-68-j: Stop default redirecting Samsung Smart TVs to mobile web - https://phabricator.wikimedia.org/T127021#2113319 (10dr0ptp4kt) [21:53:37] 7Varnish, 10MobileFrontend, 3Reading-Web-Sprint-67-If, Then, Else...?: Stop default redirecting Samsung Smart TVs to mobile web - https://phabricator.wikimedia.org/T127021#2029958 (10dr0ptp4kt) [23:27:15] 10Traffic, 10MobileFrontend, 6Operations, 3Reading-Web-Sprint-67-If, Then, Else...?, and 3 others: Incorrect TOC and section edit links rendering in Vector due to ParserCache corruption via ParserOutput::setText( ParserOutput::getText() ) - https://phabricator.wikimedia.org/T124356#2113612 (10Jdlrobson) >...