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

Change ORT to not update ip_allow except badass#5041

Merged
dneuman64 merged 2 commits intoapache:masterfrom
rob05c:change-ort-ipallow-error-except-badass
Sep 18, 2020
Merged

Change ORT to not update ip_allow except badass#5041
dneuman64 merged 2 commits intoapache:masterfrom
rob05c:change-ort-ipallow-error-except-badass

Conversation

@rob05c
Copy link
Member

@rob05c rob05c commented Sep 17, 2020

ATS has a known bug where changing ip_allow.config causes random
blocking on config reload. We changed ORT a while back to not reload
when it changes, but other files can later trigger a reload.

This changes ORT to not update the file at all, and log an error.
This will cause any added servers to not be added to the allow,
likely breaking Edges. But breaking an Edge is better than
breaking a Mid.

Further, the error log will allow users to create alarms, so
they know to go in and manually badass and restart the machine.

I've manually tested, verified server IP changes log the error but don't update the file, changes to other files apply as expected, badass mode updates ip_allow.config as expected.

Includes changelog.
No docs, ORT config file details are not documented.
No tests, ORT itself doesn't have an integration test framework yet.

  • This PR is not related to any other Issue

Which Traffic Control components are affected by this PR?

  • Traffic Ops ORT

What is the best way to verify this PR?

Change or add a Server in Traffic Ops with a new IP, run ORT in syncds mode, verify an Error is logged but the file is not updated. Change something else, like a DS Origin, verify change still applies correctly. Run ORT in badass move, verify ip_allow.config is updated.

If this is a bug fix, what versions of Traffic Control are affected?

It's behaved this way forever

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

@rob05c rob05c added the Traffic Ops ORT *DEPRECATED* related to the traffic_ops_ort.pl script label Sep 17, 2020
@rob05c rob05c force-pushed the change-ort-ipallow-error-except-badass branch 3 times, most recently from 194d2fa to 37de458 Compare September 17, 2020 20:22
ATS has a known bug where changing ip_allow.config causes random
blocking on config reload. We changed ORT a while back to not reload
when it changes, but other files can later trigger a reload.

This changes ORT to not update the file at all, and log an error.
This will cause any added servers to not be added to the allow,
likely breaking Edges. But breaking an Edge is better than
breaking a Mid.

Further, the error log will allow users to create alarms, so
they know to go in and manually badass and restart the machine.
@rob05c rob05c force-pushed the change-ort-ipallow-error-except-badass branch from 37de458 to 36332e2 Compare September 17, 2020 20:30
@rob05c rob05c added 4.1.x-backport-candidate bug something isn't working as intended labels Sep 17, 2020
@rob05c rob05c marked this pull request as ready for review September 17, 2020 20:47
@dneuman64
Copy link
Contributor

Since this only affects Mids and not Edges, is there a way to only apply this to Mids? Maybe we can make it a config or flag and we can set it up such that edges update the file and mids do not?

@rob05c
Copy link
Member Author

rob05c commented Sep 17, 2020

Edges have the same problem. They just don't update their config very often.
But if an Edge ip_allow.config changes, ATS starts blocking random addresses the same as a Mid
.

@dneuman64
Copy link
Contributor

Are you sure because I don't think we have seen that issue on edges before? We have only seen this issue on Mids.
I still think we should consider making this a flag. It would be beneficial for if/when the bug gets fixed to be able to put ip_allow.config in place without having to BA ATS every time

@rob05c
Copy link
Member Author

rob05c commented Sep 18, 2020

I am positive. We haven't seen it on Edges because they're basically open to the internet, and almost never change. If ip_allow.config changed on an Edge for any reason, ATS would start blocking random addresses.

I can add a flag.

@dneuman64 dneuman64 merged commit 492290d into apache:master Sep 18, 2020
rob05c added a commit to rob05c/trafficcontrol that referenced this pull request Sep 18, 2020
* Change ORT to not update ip_allow except badass

ATS has a known bug where changing ip_allow.config causes random
blocking on config reload. We changed ORT a while back to not reload
when it changes, but other files can later trigger a reload.

This changes ORT to not update the file at all, and log an error.
This will cause any added servers to not be added to the allow,
likely breaking Edges. But breaking an Edge is better than
breaking a Mid.

Further, the error log will allow users to create alarms, so
they know to go in and manually badass and restart the machine.

* Add ORT flag to update ip_allow.config in syncds

(cherry picked from commit 492290d)
rawlinp pushed a commit that referenced this pull request Sep 18, 2020
* Change ORT to not update ip_allow except badass

ATS has a known bug where changing ip_allow.config causes random
blocking on config reload. We changed ORT a while back to not reload
when it changes, but other files can later trigger a reload.

This changes ORT to not update the file at all, and log an error.
This will cause any added servers to not be added to the allow,
likely breaking Edges. But breaking an Edge is better than
breaking a Mid.

Further, the error log will allow users to create alarms, so
they know to go in and manually badass and restart the machine.

* Add ORT flag to update ip_allow.config in syncds

(cherry picked from commit 492290d)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended Traffic Ops ORT *DEPRECATED* related to the traffic_ops_ort.pl script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants