Fix TO update_status to exclude ADMIN_DOWN #3689
Conversation
rob05c
added
bug
rob05c
force-pushed
the
to-fix-updatestatus-admindown
branch
from
ffa15a3
to
690732b
Compare
Fixes apache#3687 Specifically, changed to only include ONLINE and REPORTED, and thus also exclude PREPROD and any other similar future states.
rob05c
force-pushed
the
to-fix-updatestatus-admindown
branch
from
690732b
to
f2bbe61
Compare
mitchell852
added
the
high impact
|
Refer to this link for build results (access rights to CI server needed): |
| @@ -49,7 +49,7 @@ func getServerUpdateStatus(tx *sql.Tx, cfg *config.Config, hostName string) ([]t | |||
| baseSelectStatement := | |||
| `WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps | |||
| LEFT JOIN status AS pstatus ON pstatus.id = ps.status | |||
| WHERE pstatus.name != 'OFFLINE' ), | |||
| WHERE (pstatus.name = '` + string(tc.CacheStatusOnline) + `' OR pstatus.name = '` + string(tc.CacheStatusReported) + `') ), | |||
There was a problem hiding this comment.
this seems like it could cause other problems. i.e. does an admin_down mid still need to be marked as updated from a upd_pending / reval_pending perspective? if it is excluded and an admin_down mid is flipped to reported suddenly, it might not have the required regex_revalidate.config file that it needs to invalidate content correctly.
my point is, since the perl endpoint only excluded OFFLINE, maybe that was done for a very valid reason.
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/UI/Server.pm#L917
There was a problem hiding this comment.
You're right, the ideal solution would handle that. But the alternative is Edges never being able to finish a Reval, if any Mid is ADMIN_DOWN and not running ORT. Until we can get the ideal solution in place, this seems like the less bad option.
There was a problem hiding this comment.
@rob05c - do you know off the top of your head what happens when a cache is moved from admin_down/offline/pre_prod to online/reported? i'm guessing nothing happens (except queue updates is triggered on all "child" caches").
my point is - if a mid is moved from admin_down/offline/pre_prod to online/reported, it could end up with an incorrect regex_revalidate.config file (which could screw up content invalidation) but maybe it's standard operating procedure to "badass" a cache when moving it to online/reported status?
There was a problem hiding this comment.
Adding my thoughts, belive it may help some way.
I assume, the ADMIN_DOWN for the cache server will be set by the admin person through trafficops UI( ie., OFFLINE the server)
Consider the below setup and scenario:SETUP:
mid_cache_group1: mid1 and mid2
edge_cache_group1: edge1 (Its parent is mid_cache_group1)SCENARIO:
If operator ADMIN_DOWN mid1 for some reason(say mid1 is dead).
Then upd_pending should set to true for mid1,mid2 and edge1. Hope this will done by the operator through trafficops Queue update.
But immediate ORT run on edge1 should wait only for mid2 (for to complete upd_pending) NOT for mid1 (since mid1 is ADMIN_DOWN).
Though mid1 is ADMIN_DOWN, on its cron schedule ORT job, it will update its required configuration since upd_pending is set to True.SUGGESTION:
Like OFFLINE in that query, can we handle something like <<<...WHERE pstatus.name != 'OFFLINE' AND pstatus.name != 'ADMIN_DOWN')..>>
Specifically, changed to only include ONLINE and REPORTED, and thus
also exclude PREPROD and any other similar future states.
Fixes #3687
Still WIP, haven't tested.
What does this PR (Pull Request) do?
Which Traffic Control components are affected by this PR?
What is the best way to verify this PR?
If this is a bug fix, what versions of Traffic Ops are affected?
The following criteria are ALL met by this PR