Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/monitor peers flag #7672

Merged
merged 16 commits into from
Aug 4, 2023

Conversation

cybertunnel
Copy link
Contributor

@cybertunnel cybertunnel commented Jul 24, 2023

During strategies.yaml creation, the file contains hosts which are &peer# if they are inside the same cache group. The health of peers is currently not provide value. So having the ability to turn off the monitoring of peers since no traffic should be going to them, helps reduce computation and memory utilization. This also slims down the logs from unnecessary chatter.


Which Traffic Control components are affected by this PR?

  • Traffic Control Health Client (tc-health-client)

What is the best way to verify this PR?

This PR includes an updated copy of tc-health-client.json for testing. Additionally, the tmagent_test.go test has updated the TestReadStrategiesDotYaml with tests for testing from reading from the tc-health-client.json config file, but also a manual true and false value, with the proper expected results checked.

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@cybertunnel
Copy link
Contributor Author

NOTE: This Does require yamlv3 pkg to be installed. Both v2 and v3 can co-exist, but v3 is needed for the yaml.Node struct. Which is how this is able to parse the peers.

@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Merging #7672 (579d49c) into master (75ec56f) will decrease coverage by 38.11%.
Report is 46 commits behind head on master.
The diff coverage is 6.09%.

@@              Coverage Diff              @@
##             master    #7672       +/-   ##
=============================================
- Coverage     65.05%   26.95%   -38.11%     
  Complexity       98       98               
=============================================
  Files           314      686      +372     
  Lines         12365    80539    +68174     
  Branches        907       90      -817     
=============================================
+ Hits           8044    21706    +13662     
- Misses         3968    56761    +52793     
- Partials        353     2072     +1719     
Flag Coverage Δ
golib_unit 48.10% <0.00%> (?)
grove_unit 4.60% <ø> (?)
t3c_unit 4.95% <5.88%> (?)
traffic_monitor_unit 21.30% <0.00%> (?)
traffic_ops_integration 69.39% <90.62%> (-0.03%) ⬇️
traffic_ops_unit 22.29% <4.32%> (?)
traffic_portal_v2 ?
traffic_stats_unit 10.14% <ø> (?)
unit_tests 23.64% <4.26%> (-50.14%) ⬇️
v3 57.79% <ø> (ø)
v4 79.18% <ø> (ø)
v5 78.52% <90.62%> (-0.12%) ⬇️

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/t3c-apply.go 0.00% <0.00%> (ø)
lib/go-atscfg/parentdotconfig.go 65.76% <ø> (ø)
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%> (ø)
traffic_monitor/tmclient/tmclient.go 0.00% <0.00%> (ø)
...c_ops/traffic_ops_golang/cachegroup/cachegroups.go 14.93% <0.00%> (ø)
...raffic_ops/traffic_ops_golang/cdn_lock/cdn_lock.go 0.00% <0.00%> (ø)
...ops/traffic_ops_golang/deliveryservice/eligible.go 55.27% <0.00%> (ø)
... and 20 more

... and 568 files with indirect coverage changes

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

@ocket8888 ocket8888 added low impact affects only a small portion of a CDN, and cannot itself break one cache-config Cache config generation improvement The functionality exists but it could be improved in some way. tc-health-client Traffic Control Health Client labels Aug 2, 2023
Copy link
Contributor

@jpappa200 jpappa200 left a comment

Choose a reason for hiding this comment

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

Looks good, works as intended.

@shamrickus shamrickus merged commit da7bd38 into apache:master Aug 4, 2023
cybertunnel added a commit to cybertunnel/trafficcontrol that referenced this pull request Aug 16, 2023
* Adding functions and config for monitor peer flag

* Adding tests for the changes and updating files

* Adding proper config file

* Adding readme documentation for the config change

* Adding prior health client default value

* Updating CHANGELOG file

* Adding yamlv3 vendor file(s)

* Adding updated go.mod file

* Fixing yamlv3 dependancy issue

* Trying this module.txt file again

* Adding the license lines.

* Fixing default value setting

* Removing some logging which was meant for testing.

---------

Co-authored-by: Tyler Morgan <tylermorgan210@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cache-config Cache config generation improvement The functionality exists but it could be improved in some way. low impact affects only a small portion of a CDN, and cannot itself break one tc-health-client Traffic Control Health Client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants