Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like i broke this in #15184 -
default_idle_timeout
was specific to fastcgi, so it's no longer being parsed and removed fromconfig
. this change should work, but it's probably better to remove the setting completelyit looks like the same issue is present in other upgrade tests:
mind removing those too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cbodley @liewegas I don't mind removing them altogether. I'm not sure we'd see the same issue in those tests, I think the problem we were seeing here was that the field was at the wrong level in the hierarchy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the
default_idle_timeout
field was global to the rgw task (see https://github.com/ceph/ceph/pull/15184/files#diff-995b04809fcabacc3e3ecfaea903a41aL1128). there was also anidle_timeout
that could be used per-clientthose global fields are removed from
config
as they're parsed, resulting in aconfig
whose only keys are clients. that's why it's now being mistakenly interpreted as a remote in this test failure. i'd expect the same thing to happen in the upgrade testsmoving the line under client.0 does work to pass the test - it just won't be used anywhere