Project

General

Profile

Actions

Bug #19126

closed

Disable nginx proxy caching for controller

Added by Ward Vandewege about 2 months ago. Updated about 1 month ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
05/12/2022
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

When deployed behind nginx for tls termination, controller's responses can get truncated when nginx's reverse proxy tries to cache responses which exceed its temp file size. We don't really need nginx to do any caching in this scenario.

  • update documentation accordingly
  • update the arvados-server boot template (also used in our test suite)
  • update our internal salt configs

Subtasks 1 (0 open1 closed)

Task #19130: review 19126-nginx-proxy-settings-changeResolvedWard Vandewege05/12/2022

Actions
Actions #1

Updated by Ward Vandewege about 2 months ago

  • Status changed from New to In Progress
Actions #2

Updated by Ward Vandewege about 2 months ago

  • Description updated (diff)
  • Subject changed from Disable nginx proxy caching on controller to Disable nginx proxy caching for controller
Actions #3

Updated by Ward Vandewege about 2 months ago

  • Description updated (diff)
Actions #5

Updated by Tom Clegg about 2 months ago

The doc update refers to large responses, but the proxy_request_buffering setting is about large request bodies. Should we add "proxy_buffering off" as well?

Actions #6

Updated by Tom Clegg about 2 months ago

Also, nginx doc says "When HTTP/1.1 chunked transfer encoding is used to send the original request body, the request body will be buffered regardless of the directive value unless HTTP/1.1 is enabled for proxying." ...and the default is HTTP/1.0. So I think we need to add "proxy_http_version 1.1;" too.

Actions #7

Updated by Ward Vandewege about 2 months ago

Tom Clegg wrote:

Also, nginx doc says "When HTTP/1.1 chunked transfer encoding is used to send the original request body, the request body will be buffered regardless of the directive value unless HTTP/1.1 is enabled for proxying." ...and the default is HTTP/1.0. So I think we need to add "proxy_http_version 1.1;" too.

As we discussed, the thing that actually works around the problem here is disabling the use of temporary files in response buffering, i.e. the proxy_max_temp_file_size 0; line.

So it seems like we should either limit this change to that line, or go the whole way and disable buffering in every direction:

    proxy_max_temp_file_size 0;
    proxy_request_buffering  off;
    proxy_buffering          off;
    proxy_http_version       1.1;
Actions #8

Updated by Tom Clegg about 2 months ago

I think we should go all the way and disable buffering in both directions. I don't think we get any benefit from buffering -- afaict all it is is a source of problems.

Actions #9

Updated by Ward Vandewege about 2 months ago

Tom Clegg wrote:

I think we should go all the way and disable buffering in both directions. I don't think we get any benefit from buffering -- afaict all it is is a source of problems.

e3363715769a2503fdcbbb1274d7d04c6852c9c3 on branch 19126-nginx-proxy-settings-change

developer-run-tests: #3122

Actions #10

Updated by Tom Clegg about 2 months ago

LGTM.

Seems to be based on the #16345 branch, but I've just merged that now, so I don't think it needs to be rebased.

Actions #11

Updated by Ward Vandewege about 2 months ago

Tom Clegg wrote:

LGTM.

Seems to be based on the #16345 branch, but I've just merged that now, so I don't think it needs to be rebased.

Looks like I missed making the change in the nginx template used for our tests, running the tests one more time:

10d497fe4eb9ab1dc10171ba90c0c1b148108a35 on branch 19126-nginx-proxy-settings-change

developer-run-tests: #3124

And I also missed the change in the installer templates:

f076c4497f2fce89f8fef9ccd601fcf316901993 on branch 19126-nginx-proxy-settings-change

test-provision: #358
test-provision-debian10: #357

Actions #12

Updated by Tom Clegg about 2 months ago

LGTM, thanks

Actions #13

Updated by Ward Vandewege about 2 months ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved

Applied in changeset arvados-private:commit:arvados|91dc80d9060c5aeb04228f0f280ecc3db182ff94.

Actions #14

Updated by Peter Amstutz about 1 month ago

  • Release set to 51
Actions

Also available in: Atom PDF