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

Add TM integration and separate docker-compose services for Varnish#7746

Merged
limited merged 4 commits intoapache:masterfrom
AbdelrahmanElawady:varnish-traffic-monitor-integration
Sep 8, 2023
Merged

Add TM integration and separate docker-compose services for Varnish#7746
limited merged 4 commits intoapache:masterfrom
AbdelrahmanElawady:varnish-traffic-monitor-integration

Conversation

@AbdelrahmanElawady
Copy link
Copy Markdown
Member

@AbdelrahmanElawady AbdelrahmanElawady commented Aug 25, 2023

Add Traffic Monitor integration with a daemon running on Varnish cache servers to report whether Varnish is running or not and to report load average and interface stats. Also, running Varnish now with CIAB is more user friendly and does not require changing any file manually just specifying varnish docker compose file with the default one.


Which Traffic Control components are affected by this PR?

  • Traffic Monitor
  • CDN in a Box

What is the best way to verify this PR?

After building rpms needed for CIAB:

  • Run using docker-compose -f docker-compose.yml -f docker-compose.varnish.yml up that should run all cache servers with Varnish.
  • Open Traffic Portal and navigate to Cache stats. Cache servers should be healthy.
  • Stop Varnish in any cache server with docker-compose exec edge systemctl stop varnish.service.
  • Traffic Portal will show edge as unhealthy.

PR submission checklist

@AbdelrahmanElawady AbdelrahmanElawady added Traffic Monitor related to Traffic Monitor low impact affects only a small portion of a CDN, and cannot itself break one cdn-in-a-box related to the Docker-based CDN-in-a-Box system low difficulty the estimated level of effort to resolve this issue is low Varnish related to Varnish labels Aug 25, 2023
@AbdelrahmanElawady
Copy link
Copy Markdown
Member Author

marked draft until #7727 is reviewed

@zrhoffman zrhoffman removed the low difficulty the estimated level of effort to resolve this issue is low label Aug 25, 2023
@AbdelrahmanElawady AbdelrahmanElawady force-pushed the varnish-traffic-monitor-integration branch from 009275d to d024cb1 Compare September 1, 2023 16:16
@AbdelrahmanElawady AbdelrahmanElawady marked this pull request as ready for review September 1, 2023 16:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 1, 2023

Codecov Report

Merging #7746 (e840378) into master (75ec56f) will decrease coverage by 36.22%.
Report is 121 commits behind head on master.
The diff coverage is 49.04%.

@@              Coverage Diff              @@
##             master    #7746       +/-   ##
=============================================
- Coverage     65.05%   28.83%   -36.22%     
  Complexity       98       98               
=============================================
  Files           314      598      +284     
  Lines         12365    77009    +64644     
  Branches        907       90      -817     
=============================================
+ Hits           8044    22206    +14162     
- Misses         3968    52720    +48752     
- Partials        353     2083     +1730     
Flag Coverage Δ
golib_unit 53.64% <60.68%> (?)
grove_unit 12.02% <ø> (?)
t3c_unit 5.96% <2.74%> (?)
traffic_monitor_unit 26.46% <48.48%> (?)
traffic_ops_unit 21.50% <ø> (?)
traffic_portal_v2 ?
traffic_stats_unit 10.78% <ø> (?)
unit_tests 25.64% <49.04%> (-48.14%) ⬇️

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 20 more

... and 476 files with indirect coverage changes

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

@AbdelrahmanElawady AbdelrahmanElawady force-pushed the varnish-traffic-monitor-integration branch from fdaa193 to 1d3a639 Compare September 5, 2023 14:11
checkInterval time.Duration
}

type vstats struct {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this struct be shared with anything that Traffic Monitor already deserializes?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely should be shared however it's implemented as just "main" package separated from TC so it's not a package that TM can call. However, I think it should be a package under TC and be built with pkg considering statistics will be added too. Should that be in this PR or done separately?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If its ok with you, let's keep it separate, since this one is pretty well baked already

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, will open an issue for it to be done later.

}

infSpeedFile := fmt.Sprintf("/sys/class/net/%s/speed", inf)
speedStr, err := os.ReadFile(infSpeedFile)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1).
@AbdelrahmanElawady AbdelrahmanElawady force-pushed the varnish-traffic-monitor-integration branch from 2fb179b to e840378 Compare September 8, 2023 14:44
@limited limited merged commit b31e003 into apache:master Sep 8, 2023
@AbdelrahmanElawady AbdelrahmanElawady deleted the varnish-traffic-monitor-integration branch September 9, 2023 08:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cdn-in-a-box related to the Docker-based CDN-in-a-Box system low impact affects only a small portion of a CDN, and cannot itself break one Traffic Monitor related to Traffic Monitor Varnish related to Varnish

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants