Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Configure uncacheable delivery services in Varnish#7771

Merged
limited merged 1 commit intoapache:masterfrom
AbdelrahmanElawady:add-uncacheable-dses
Sep 12, 2023
Merged

Configure uncacheable delivery services in Varnish#7771
limited merged 1 commit intoapache:masterfrom
AbdelrahmanElawady:add-uncacheable-dses

Conversation

@AbdelrahmanElawady
Copy link
Copy Markdown
Member

Skip caching delivery services with type HTTP_NO_CACHE.

Closes: #7770


Which Traffic Control components are affected by this PR?

  • Traffic Control Cache Config (t3c, formerly ORT)

What is the best way to verify this PR?

unit tests added.
CIAB doesn't have a DS with no cache type, however if manually changed the DS profile to HTTP_NO_CACHE requests should not be cached with Varnish.

PR submission checklist

@AbdelrahmanElawady AbdelrahmanElawady added low impact affects only a small portion of a CDN, and cannot itself break one cache-config Cache config generation Varnish related to Varnish labels Sep 1, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 1, 2023

Codecov Report

Merging #7771 (63fca4f) into master (75ec56f) will decrease coverage by 36.24%.
Report is 132 commits behind head on master.
The diff coverage is 40.57%.

@@              Coverage Diff              @@
##             master    #7771       +/-   ##
=============================================
- Coverage     65.05%   28.82%   -36.24%     
  Complexity       98       98               
=============================================
  Files           314      599      +285     
  Lines         12365    77053    +64688     
  Branches        907       90      -817     
=============================================
+ Hits           8044    22210    +14166     
- Misses         3968    52753    +48785     
- Partials        353     2090     +1737     
Flag Coverage Δ
golib_unit 53.66% <54.88%> (?)
grove_unit 12.02% <ø> (?)
t3c_unit 5.96% <2.74%> (?)
traffic_monitor_unit 26.44% <ø> (?)
traffic_ops_unit 21.45% <ø> (?)
traffic_portal_v2 ?
traffic_stats_unit 10.78% <ø> (?)
unit_tests 25.63% <40.57%> (-48.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
cache-config/t3c-apply/config/config.go 0.00% <0.00%> (ø)
cache-config/t3c-apply/torequest/cmd.go 0.00% <0.00%> (ø)
cache-config/t3c-check-refs/t3c-check-refs.go 1.78% <0.00%> (ø)
cache-config/t3c-generate/cfgfile/varnish.go 0.00% <0.00%> (ø)
cache-config/t3c-generate/config/config.go 0.81% <0.00%> (-0.08%) ⬇️
cache-config/t3c-generate/t3c-generate.go 0.00% <ø> (ø)
lib/go-atscfg/parentabstraction.go 80.00% <ø> (ø)
lib/go-tc/cdns.go 0.00% <ø> (ø)
lib/go-tc/deliveryservice_servers.go 0.00% <0.00%> (ø)
lib/go-tc/federation_resolver.go 45.45% <0.00%> (ø)
... and 13 more

... and 484 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@limited
Copy link
Copy Markdown
Contributor

limited commented Sep 12, 2023

Here's the generated VCL from this change:

sub vcl_recv {
	if ((req.method == "PUSH" || req.method == "PURGE" || req.method == "DELETE") && !client.ip ~ allow_all) {
		return (synth(405));
	}
	if (req.method == "PURGE") {
		return (purge);
	}
	if (req.http.host == "edge.demo1.mycdn.ciab.test") {
		set req.backend_hint = demo1.backend();
	}
	if (req.http.host == "video.demo2.mycdn.ciab.test") {
		set req.backend_hint = demo2.backend();
	}
}
sub vcl_backend_response {
	if (bereq.http.host == "origin.infra.ciab.test") {
		set beresp.uncacheable = true;
	}
}
sub vcl_backend_fetch {
	if (bereq.http.host == "edge.demo1.mycdn.ciab.test") {
		set bereq.http.host = "origin.infra.ciab.test";
	}
	if (bereq.http.host == "video.demo2.mycdn.ciab.test") {
		set bereq.http.host = "origin.infra.ciab.test";
	}
}

Because the same origin is shared between both delivery services, this will mark all requests to origin.infra.ciab.test as uncacheable.

Is there a way we can restrict the DS_NO_CACHE behavior just to the appropriate delivery service? (Maybe based on bereq.http.Host?)

@limited
Copy link
Copy Markdown
Contributor

limited commented Sep 12, 2023

Because the same origin is shared between both delivery services, this will mark all requests to origin.infra.ciab.test as uncacheable.

After comparing with the existing ATS implementation, this matches current behavior, so keeping it for consistency.

@limited limited merged commit 6f527a1 into apache:master Sep 12, 2023
@AbdelrahmanElawady AbdelrahmanElawady deleted the add-uncacheable-dses branch September 12, 2023 17:28
rimashah25 pushed a commit to rimashah25/trafficcontrol that referenced this pull request Sep 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cache-config Cache config generation low impact affects only a small portion of a CDN, and cannot itself break one Varnish related to Varnish

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Set uncacheable delivery services in Varnish config

2 participants