Extracting large chunks of the PUT method into smaller
methods to improve maintainability and reuse of code.
Based on the work that Clay Gerrard started:
https://review.openstack.org/#/c/77812/
Co-Authored-By: Clay Gerrard <clay.gerrard@gmail.com>
Change-Id: Id479fc5b159a2782361ac4a6e4a6d8bbaee4fe85
Signed-off-by: Thiago da Silva <thiago@redhat.com>
If the used tool to send header doesn't support empty headers (older versions
of curl), x-remove can be used to remove metadata.
sync-key and sync-to metadata, used by container-sync, can now be removed using
x-remove headers.
Change-Id: I0edb4d5425a99d20a973aa4fceaf9af6c2ddecc0
When an account was not found, ContainerController would
return 404 unconditionally for a container GET or HEAD request,
without checking that the request was authorized.
This patch modifies the GETorHEAD method to first call any
callback method registered under 'swift.authorize' in the
request environ and prefer any response from that over the 404.
Closes-Bug: 1415957
Change-Id: I4f41fd9e445238e14af74b6208885d83698cc08d
According to documentation dlo manifest files should not
be versioned. This patch fixes this issue and adds
some unit and functional for this scenario.
Change-Id: Ib5b29a19e1d577026deb50fc9d26064a8da81cd7
Signed-off-by: Thiago da Silva <thiago@redhat.com>
The proxy was storing the error count and last-error time in the
ring's internal data, specifically in the device dictionaries. This
works okay, but it means that whenever a ring changes, all the error
stats reset.
Now the error stats live in the proxy server object, so they survive a
ring reload.
Better yet, the error stats are now keyed off of the node's
IP/port/device triple, so if you have the same device in two rings
(like with multiple storage policies), then the error stats are
combined. If the proxy server sees a 507 for an objec request in
policy X, then that will now result in that particular object disk
being error-limited for requests in policies Y and Z as well.
Change-Id: Icc72b68b99f37367bb16d43688e7e45327e3e022
When a X-Backend-Timestamp is available it would generally preferred
over a less specific value and sorts correctly against any X-Timestamp
values anyway.
Change-Id: I08b7eb37ab8bd6eb3afbb7dee44ed07a8c69b57e
The default MAX_FILE_SIZE of (5 * 2 ** 30 + 2) exceeds the
capacity of an int on 32 bit systems; adjust this default
down to (2 ** 30 + 2) if the default exceeds the maxsize of
an int on the platform running the tests.
Closes-Bug: #1378810
Change-Id: Iafa2ce90ceb2de4e968ad48580270c8c572a9c9c
This change adds an optional overrides map to _make_request method
in the base Controller class.
def make_requests(self, req, ring, part, method, path, headers,
query_string='', overrides=None)
Which will be passed on the the best_response method. If set and
no quorum it reached, the override map is used to attempt to find
quorum.
The overrides map is in the form:
{ <response>: <override response>, .. }
The ObjectController, in the DELETE method now passes an override map
to make_requests method in the base Controller class in the form of:
{ 404: 204 }
Statuses/responses that have been overridden are used in calculation
of the quorum but never returned to the user. They are replaced by:
(STATUS, '', '', '')
And left out of the search for best response.
Change-Id: Ibf969eac3a09d67668d5275e808ed626152dd7eb
Closes-Bug: 1318375
The proxy sends requests to all storage nodes, but it only needs a
quorum of them to respond before the proxy can, in turn, respond to
the client. Thus, it gets quorum, and then briefly waits to see if the
remainder of the storage nodes respond before giving up on them.
However, the proxy was not paying any attention to the responses from
the non-quorum storage nodes. This would lead to some odd behavior,
like a container PUT where the backends returned (201, 201, 202) would
become a 201 to the client, but the permutation (201, 202, 201) would
become 202. Further, on object PUT, if the last node responded with an
error code, that error wouldn't count towards error-limiting.
The fix is quite simple: after getting quorum, the make_requests()
method was calling a method that returns responses from the remainder
of the nodes, but it was ignoring that return value and making up
responses with dummy values instead. Now, prior to making up dummy
responses, the proxy first uses the responses it already has, and only
fills in dummy responses for nodes that really didn't respond in time.
Change-Id: I0206b6b2272b0e7dcc80fb6c51840d8dae25cee2
If any node had an error on object PUT, the proxy would count the
error against the last-connected-to node instead of the one with the
error. Now it counts the error against the right node.
Change-Id: I884eb73cebe0c723473a6d5e390a148fcad0d3ed
When deleteing versioned objects proxy will try to restore the previous
copy. The COPY request will fail if the previous version is expired but
not handled by object-expirer.
This patch checks COPY respones on the previous copy, if it's
HTTP_NOT_FOUND(mostly because it's expired) proxy will try to restore
with the version before previous.
Closes-Bug #1308446
Change-Id: I17f049ea3ef62723effae8086ec427f6e151cd9c
All the expiring objects for a given X-Delete-At are funnelled into the
same expiring object container- this can act as a bottleneck.
Change-Id: I288a177a7ae3e213c727a2a81fa76d4ef9cf7eb3
Adds ability to copy objects between different accounts (on server side)
Adds new header to `PUT` request:
`X-Copy-From-Account: <account name>`
Account name corresponds to the last part of storage URL.
Adds new header to `COPY` request:
`Destination-Account: <account name>`
Account name corresponds to the last part of storage URL.
If your storage URL is: http://server:8080/v1/AUTH_test
Then the account name is `AUTH_test`
These headers should be used alongside `X-Copy-From` and `Destination` headers
The legacy headers should specify `<container name>/<object name>` path as usual.
DocImpact
Change-Id: I0285fe6a47df9e699ac20ae4a83b0bf23829e1e6
There was some duplication of code in both POST and PUT
methods to handle object-expiration headers.
A method was created to remove this duplication,
which should help with maintainability of code.
Change-Id: I85cc4a7b0d688760c97598d80b9e9a39288c5f34
Signed-off-by: Thiago da Silva <thiago@redhat.com>
The keystoneauth middleware supports cross-tenant access
control using the syntax <tenant>:<user> in container ACLs,
where <tenant> and <user> may currently be either a unique
id or a name. As a result of the keystone v3 API introducing
domains, names are no longer globally unique and are only
unique within a domain. The use of unqualified tenant and
user names in this ACL syntax is therefore not 'safe' in a
keystone v3 environment.
This patch modifies keystoneauth to restrict cross-tenant
ACL matching to use only ids for accounts that are not in
the default domain. For backwards compatibility,
names will still be matched in ACLs when both the requesting
user and tenant are known to be in the default domain AND the
account's tenant is also in the default domain (the default
domain being the domain to which existing tenants are
migrated).
Accounts existing prior to this patch are assumed to be for
tenants in the default domain. New accounts created using a
v2 token scoped on the tenant are also assumed to be in the
default domain. New accounts created using a v3 token scoped
on the tenant will learn their domain membership from the
token info. New accounts created using any unscoped token,
(i.e. with a reselleradmin role) will have unknown domain
membership and therefore be assumed to NOT be in the default
domain.
Despite this provision for backwards compatibility, names
must no longer be used when setting new ACLs in any account,
including new accounts in the default domain.
This change obviously impacts users accustomed to specifying
cross-tenant ACLs in terms of names, and further work will be
necessary to restore those use cases. Some ideas are
discussed under the bug report. With that caveat, this patch
removes the reported vulnerability when using
swift/keystoneauth with a keystone v3 API.
Note: to observe the new 'restricted' behaviour you will need
to setup keystone user(s) and tenant(s) in a non-default domain
and set auth_version = v3.0 in the auth_token middleware config
section of proxy-server.conf. You may also benefit from the
keystone v3 enabled swiftclient patch under review here:
https://review.openstack.org/#/c/91788/
DocImpact
blueprint keystone-v3-support
Closes-Bug: #1299146
Change-Id: Ib32df093f7450f704127da77ff06b595f57615cb
A long, long time ago, on a GET request, the proxy would go look on 3*
nodes for the requested thing. If one of the primary nodes was
error-limited, it'd look on two primaries and a handoff. Since this
indicated some failure somewhere, the proxy would emit a warning:
"Handoff requested (1)". If two primaries were down, there'd be a
second message "Handoff requested (2)", and so on.
Some StatsD messages were emitted too.
A somewhat shorter time ago (commit d79a67eb), the proxy started
looking into handoffs if it got 404s from the primaries. While this
was a good idea, it resulted lots of "Handoff requested (N)" log spam;
you'd see these messages on every single 404. Also, the StatsD
handoff_count and handoff_all_count metrics shot way up and turned
into noise.
This commit restores the original intent (and usefulness) of the log
messages and StatsD metrics: if the proxy only looks at the normal
number of handoff nodes, nothing is logged. However, if a primary is
down, then the message "Handoff requested (1)" will be logged,
indicating that the proxy looked at one more handoff than it normally
would, and this happened because a primary node was error-limited.
Closes-Bug: 1297214
* or whatever the replica count was
Change-Id: If1b77c18c880b096e8ab1df3008db40ce313835d
If container counter per account is equal or greater than
max_container_per_account, then all PUT requests are failed and
403 is returned.
This is correct behaviour if the request is to create a new
container, however if container already exists PUT should be
allowed, even the max_container_per_account condition has met.
This patch allows to process PUT requests for existing containers,
even if max_container_per_account > = container count.
It indirectly resolve the bug 1306711, since swift-client
uses internally PUT requests for container, prior it upload an
object there.
Change-Id: I2dcf20b6feb27e346111466a565695eba4b4b1da
The get_part method is fast and stable given a consistent hash path
suffix/prefix, so there's no absolute requirement for the fake
implementation other than convenience. OTOH, removing the fake
implementation and fixing the tests that were relying on it should make
it easier to write better tests going forward and harder to hide bugs
that don't show up when using the fakes.
There may be some overhead when writing new tests that use the ring if
you're making assertions on partitions or paths, but with a part power
of zero it's normally trivially obvious when a 1 needs to be a 0 or vice
versa. Or you can just drop the assertions around the parts you were
faking anyway.
Change-Id: I8bfc388a04eff6491038991cdfd7686c9d961545
Per information filed in bug (and related bugs referenced in the
report) it appears that in TestObjectController.test_client_timeout
having a matching timeout value for both self.app.client_timeout
and SlowBody() appears to work however, as expected, a smaller
value in self.app.client_timeout also works where a larger
value fails. With that short test combined with speculation and
related fixes, seems reasonable to merge what is suggested in
the bug report, drop the self.app.client_timeout to 0.05
Fixes Bug #1316716
Change-Id: Ib4c6d3bb275f6c50c62505c90656efa7ee566bc0
Timeout isn't an Exception, so Timeouts in tests weren't getting
raised. Instead, you'd sometimes have an HTTPResponse's .status be a
Timeout object, not an integer, which greatly confuses code that
expects an integer.
Also reorder the test that exposed the failure in the gate so it blows
up most times instead of sometimes do demonstrate the failure with out
this fix to FakeConn.
Change-Id: I76367a0575f84cad6b2f03e814f3f16bf96bc7d1
TestObjectController.test_POST_backend_headers was being too picky about the
order of backend requests which when pushed through eventlet will not have a
stable order. This change preserves the expectations and assertions while
removing the dependency on the order of the requests.
Change-Id: I7176ccb9223cd3dfc3c922b8b3d81eb514891d05
In the proxy, container_info can return a 'storage_policy' of None. When
you set a header value on a swob.Request to None that effectively just
delete's the key. One path through the proxy during container sync was
counting on the the 'X-Backend-Storage-Policy-Index' being set which isn't
the case if the cached container_info if for a pre-policies container.
Also clean up some test cruft, tighten up the interface on FakeConn, and add
some object controller tests to exercise more interesting failure and handoff
code paths.
Change-Id: Ic379fa62634c226cc8a5a4c049b154dad70696b3
Replaced throughout code base & tox'd. Functional as well
as probe tests pass with and without policies defined.
POLICY --> 'X-Storage-Policy'
POLICY_INDEX --> 'X-Backend-Storage-Policy-Index'
Change-Id: Iea3d06de80210e9e504e296d4572583d7ffabeac
* add get_object
* allow extra headers passthrough on HEAD/metadata reqeusts
* expose (account|container|get_object)_ring properties
Pipeline propety access to the auto_create_account_prefix also allows us to
bypass the early exit on a container HEAD for auto_create_accounts if the
container-updater hasn't cycled yet.
Allow overriding of storage policy index.
This is something the reconciler will need so that it can GET from one
policy, PUT in another, and then DELETE from the first one again.
DocImpact
Implements: blueprint storage-policies
Change-Id: I9b287d15f2426022d669d1186c9e22dd8ca13fb9
Objects now have a storage policy index associated with them as well;
this is determined by their filesystem path. Like before, objects in
policy 0 are in /srv/node/$disk/objects; this provides compatibility
on upgrade. (Recall that policy 0 is given to all existing data when a
cluster is upgraded.) Objects in policy 1 are in
/srv/node/$disk/objects-1, objects in policy 2 are in
/srv/node/$disk/objects-2, and so on.
* 'quarantined' dir already created 'objects' subdir so now there
will also be objects-N created at the same level
This commit does not address replicators, auditors, or updaters except
where method signatures changed. They'll still work if your cluster
has only one storage policy, though.
DocImpact
Implements: blueprint storage-policies
Change-Id: I459f3ed97df516cb0c9294477c28729c30f48e09
Containers now have a storage policy index associated with them,
stored in the container_stat table. This index is only settable at
container creation time (PUT request), and cannot be changed without
deleting and recreating the container. This is because a container's
policy index will apply to all its objects, so changing a container's
policy index would require moving large amounts of object data
around. If a user wants to change the policy for data in a container,
they must create a new container with the desired policy and move the
data over.
Keep status_changed_at up-to-date with status changes.
In particular during container recreation and replication.
When a container-server receives a PUT for a deleted database an extra UPDATE
is issued against the container_stat table to notate the x-timestamp of the
request.
During replication if merge_timestamps causes a container's status to change
(from DELETED to ACTIVE or vice-versa) the status_changed_at field is set to
the current time.
Accurate reporting of status_changed_at is useful for container replication
forensics and allows resolution of "set on create" attributes like the
upcoming storage_policy_index.
Expose Backend container info on deleted containers.
Include basic container info in backend headers on 404 responses from the
container server. Default empty values are used as placeholders if the
database does not exist.
Specifically the X-Backend-Status-Changed-At, X-Backend-DELETE-Timestamp and
the X-Backend-Storage-Policy-Index value will be needed by the reconciler to
deal with reconciling out of order object writes in the face of recently
deleted containers.
* Add "status_changed_at" key to the response from ContainerBroker.get_info.
* Add "Status Timestamp" field to swift.cli.info.print_db_info_metadata.
* Add "status_changed_at" key to the response from AccountBroker.get_info.
DocImpact
Implements: blueprint storage-policies
Change-Id: Ie6d388f067f5b096b0f96faef151120ba23c8748
FakeLogger gets better log level handling
Parameterize logger on some daemons which were previously
unparameterized and try and use the interface in tests.
FakeRing use more real code
The existing FakeRing mock's implementation bit me on some pretty subtle
character encoding issue by-passing the hash_path code that is normally
part of get_part_nodes. This change tries to exercise more of the real
ring code paths when it makes sense and provide a better Fake for use in
testing.
Add write_fake_ring helper to test.unit for when you need a real ring.
DocImpact
Implements: blueprint storage-policies
Change-Id: Id2e3740b1dd569050f4e083617e7dd6a4249027e
The basic idea here is to replace the use of a single object ring in
the Application class with a collection of object rings. The
collection includes not only the Ring object itself but the policy
name associated with it, the filename for the .gz and any other
metadata associated with the policy that may be needed. When
containers are created, a policy (thus a specific obj ring) is
selected allowing apps to specify policy at container creation time
and leverage policies simply by using different containers for object
operations.
The policy collection is based off of info in the swift.conf file.
The format of the sections in the .conf file is as follows:
swift.conf format:
[storage-policy:0]
name = chicken
[storage-policy:1]
name = turkey
default = yes
With the above format:
- Policy 0 will always be used for access to existing containers
without the policy specified. The ring name for policy 0 is always
'object', assuring backwards compatiblity. The parser will always
create a policy 0 even if not specified
- The policy with 'default=yes' is the one used for new container
creation. This allows the admin to specify which policy is used without
forcing the application to add the metadata.
This commit simply introduces storage policies and the loading
thereof; nobody's using it yet. That will follow in subsequent
commits.
Expose storage policies in /info
DocImpact
Implements: blueprint storage-policies
Change-Id: Ica05f41ecf3adb3648cc9182f11f1c8c5c678985
The value of the X-Trans-Id-Extra header on the request (if any) will
now be appended to the transaction ID. This lets users put their own
information into transaction IDs.
For example, Glance folks upload images as large objects, so they'd
like to be able to tie together all the segment PUTs and the manifest
PUT with some operation ID in the logs. This would let them pass in
that operation ID as X-Trans-Id-Extra, and then when things went
wrong, it'd be much easier to find all the requests in Swift's logs.
Also, this aids debuggability when requests fail to receive
responses. If a user is sending in their own X-Trans-Id-Extra strings,
then that gives operators something to search for in the logs. The
normal txid won't work since that's in the response, but the client
didn't receive one.
Swift will only use the first 32 characters of X-Trans-Id-Extra so
that its log lines stay a manageable length. Also, it's URL-quoted so
that users cannot inject double quotes into X-Trans-Id-Extra and screw
up log parsers.
DocImpact
Change-Id: I3c51d0c5ac55697ac230001840da219e73a03157
Add test to check that only the expected keys are
reported by proxy in /info, and add comments to
raise awareness that default constraints will be
automatically published by proxy in response to /info
requests.
Change-Id: Ia5f6339b06cdc2e1dc960d1f75562a2505530202
Two of the default constraints, max_header_size and
max_meta_overall_size, don't get registered for the
/info response by the proxy server.
Rather than adding them individually to the proxy's
register_swift_info call, this patch proposes to
register all the constraints for /info at once
using the constraints.EFFECTIVE_CONSTRAINTS dict.
Any future additions to default constraints will
then be automatically included in /info.
Change-Id: I5c10d4c8eda90ba94745b6f89df85aafbb50f8ef
Prior to this patch both mainline code and testing modules imported
and used constraints directly into their own namespace, or relied on
the namespace of other modules that were not the constraints
module. This meant that if a unit test wanted to change a constraint
for its operation, it had to know how that module was using the
constraint, instead of referencing the constraint module itself.
This patch unifies the use of constraints so that all constraints are
referenced via the constraints module. In turn, this allows a test to
leverage the re-loadable nature of the constraints in the constraints
module.
It addition, a number of functional tests where using the default
values for constraints, instead of the configured value discovered in
a test.conf or in an existing swift.conf. This patch removes those
direct references in favor of the load_constraint() method from the
test/functional/tests.py module.
Change-Id: Ia5313d653c667dd9ca800786de59b59334c34eaa
It seemed like some of the tests ment to exercise the proxy's timeout could
would occastionally race and the backend mock would be able to cough up a
chunk before the timeout fired in the proxy - typically resulting in a failed
assertion like " lalala" != 'lalala'.
We paramaterize the timeout value, and bump it up for those cases where we are
expecting to hit the timeout. Timing shows this change is just as fast for
passing tests. And if you want to break the node_timeout in
proxy.controller.base you can verify the tests are still just as effective -
if a bit slower to detect the failure path.
Fixes bug #1272509
Change-Id: Iaf91d9d551e94fc317a08e8c0ee02daeed331b60
