Short circuit remap reload when non-existent remap file is specified

[sudheerv]

This fixes issue #7767

Ensures resiliency against config errors erasing remaps previously loaded.

[SolidWallOfCode]

Shouldn't that be false?

[sudheerv]

I had false earlier but, that fails some autest that seems to expect true for a non-existent file.

https://ci.trafficserver.apache.org/job/autest-github/15470/console

file /var/tmp/ausb-7768.15470/thread_config/ts-1_exec-0_accept-1_task-1_aio/log/diags.log : diags.log should not contain errors - Failed Reason: Contents of /var/tmp/ausb-7768.15470/thread_config/ts-1_exec-0_accept-1_task-1_aio/log/diags.log contains expression: "FATAL:" Details: [Apr 30 11:29:45.164] traffic_server FATAL: remap.config failed to load : 34

[SolidWallOfCode]

If PR #7777 were committed, the switch could be removed and have something more like

if (ec) {
  // log the error
  return false;
}

If ec is marked as an error, there is nothing in the content to parse.

[SolidWallOfCode]

Thinking more deeply on this and after a discussion with @sudheerv, I think the approach would be

• Remove the check on UrlRewrite::load from init_reverse_proxy.
• Change UrlRewrite::load to generate a Warning and fail on any failure to load "remap.config".

This would change startup behavior such that a missing "remap.config" would not prevent startup. As we've wanted to move toward fewer required files, I think this is a feature, not a bug. It would be consistent with the more general rule that has been adopted to treat a missing file as if it were an empty file. This would be detected (if the missing file is actually a problem) by 404 responses and the Warning in "diags.log".

[sudheerv]

Thinking more deeply on this and after a discussion with @sudheerv, I think the approach would be

• Remove the check on UrlRewrite::load from init_reverse_proxy.
• Change UrlRewrite::load to generate a Warning and fail on any failure to load "remap.config".

This would change startup behavior such that a missing "remap.config" would not prevent startup. As we've wanted to move toward fewer required files, I think this is a feature, not a bug. It would be consistent with the more general rule that has been adopted to treat a missing file as if it were an empty file. This would be detected (if the missing file is actually a problem) by 404 responses and the Warning in "diags.log".

+1 .

That sounds reasonable, I just left the check in init_reverse_proxy() intact and changed the Fatal to Warning to allow for some local visibility to that method (even though, UrlRewrite::load() also prints a warning).

[sudheerv]

If PR #7777 were committed, the switch could be removed and have something more like

if (ec) {
  // log the error
  return false;
}

If ec is marked as an error, there is nothing in the content to parse.

Ah gotcha! Removed the switch/case as well.

[sudheerv]

Closing this in favor of #7782