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

Fix for Strategies.yaml to include parent_is_proxy and fix some tech debt #7639

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cybertunnel
Copy link
Contributor

Closes: #7638

This PR is to fix an issue where as in #7638, the strategies.yaml does not include a parent_is_proxy value for any servers in the file. This PR also fixes an issue from tech debt where the strategiesdotconfig.go is supposed to be strategiesdotyaml.go since the file changed from .config to .yaml


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?

I have added a test in strategiesdotyaml_tests.go, the function is TestMakeStrategiesDotYAMLParentIsProxy which tests when opt.ParentIsProxy is set to true and when it is set to false.

If this is a bugfix, which Traffic Control versions contained the bug?

I would imagine all existing versions have this issue since switching to .yaml structure.

PR submission checklist

This PR doesn't need documentation since it has everything already documented in ATS's page. This is just fixing a behavior we were expecting.

@ocket8888 ocket8888 added bug something isn't working as intended low impact affects only a small portion of a CDN, and cannot itself break one cache-config Cache config generation labels Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #7639 (6ea8511) into master (08d037f) will increase coverage by 7.24%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #7639      +/-   ##
============================================
+ Coverage     30.12%   37.37%   +7.24%     
  Complexity       98       98              
============================================
  Files           794      250     -544     
  Lines         84077    12676   -71401     
  Branches        907       90     -817     
============================================
- Hits          25332     4738   -20594     
+ Misses        56615     7620   -48995     
+ Partials       2130      318    -1812     
Flag Coverage Δ
golib_unit ?
grove_unit 4.60% <ø> (ø)
t3c_unit ?
traffic_monitor_unit ?
traffic_ops_unit ?
traffic_portal_v2 ?
traffic_stats_unit 10.14% <ø> (ø)
unit_tests 6.00% <ø> (-21.26%) ⬇️

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

see 546 files with indirect coverage changes

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working as intended cache-config Cache config generation low impact affects only a small portion of a CDN, and cannot itself break one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

parent_is_proxy missing from Strategies.yaml
2 participants