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.

Sign up for GitHub

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

Jump to bottom

t3c: sort peers in strategies.yaml #7182

Merged
merged 1 commit into from Nov 16, 2022
Merged

t3c: sort peers in strategies.yaml #7182

merged 1 commit into from Nov 16, 2022

Conversation

traeak
Copy link
Contributor

@traeak traeak commented Nov 9, 2022 •
edited

During T3C strategy.yaml generation, make sure the peers are sorted by key before writing out. This can cause false positives for reloading ATS.

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?

With old t3c:

Run t3c. Change a delivery service (header rewrite value, etc).
Rerun t3c. Observe that the strategies.yaml changes due to arbitrary sort order of peers.

Run the same with new t3c and note the peers are now sorted and that a new strategies.yaml file isn't installed by t3c.

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

7.1.0

PR submission checklist

@traeak traeak force-pushed the t3c_strategy_sort branch 2 times, most recently from 0a36d80 to 9c72060 Compare November 9, 2022 19:39
@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 Nov 9, 2022
jpappa200
jpappa200 approved these changes Nov 16, 2022
View reviewed changes
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 to me, tested and works as described.

ocket8888
ocket8888 approved these changes Nov 16, 2022
View reviewed changes
Copy link
Contributor

@ocket8888 ocket8888 left a comment

Choose a reason for hiding this comment

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

Looks fine. Has conflicts preventing merge.

@ocket8888 ocket8888 merged commit 323a465 into apache:master Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ocket8888 ocket8888 ocket8888 approved these changes

@jpappa200 jpappa200 jpappa200 approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@traeak @ocket8888 @jpappa200