When we do `satellite run api --placement '...'`, the placement rules are not parsed well.
The problem is based on `viper.AllSettings()`, and the main logic is sg. like this (from a new unit test):
```
r := ConfigurablePlacementRule{}
err := r.Set(p)
require.NoError(t, err)
serialized := r.String()
r2 := ConfigurablePlacementRule{}
err = r2.Set(serialized)
require.NoError(t, err)
require.Equal(t, p, r2.String())
```
All settings evaluates the placement rules in `ConfigurablePlacementRules` and stores the string representation.
The problem is that we don't have proper `String()` implementation (it prints out the structs instead of the original definition.
There are two main solutions for this problem:
1. We can fix the `String()`. When we parse a placement rule, the `String()` method should print out the original definition
2. We can switch to use pure string as configuration parameter, and parse the rules only when required.
I feel that 1 is error prone, we can do it (and in this patch I added a lot of `String()` implementations, but it's hard to be sure that our `String()` logic is inline with the parsing logic.
Therefore I decided to make the configuration value of the placements a string (or a wrapper around string).
That's the main reason why this patch seems to be big, as I updated all the usages.
But the main part is in beginning of the `placement.go` (configuration parsing is not a pflag.Value implementation any more, but a separated step).
And `filter.go`, (a few more String implementation for filters.
https://github.com/storj/storj/issues/6248
Change-Id: I47c762d3514342b76a2e85683b1c891502a0756a
as GetParticipatingNodes and GetNodes, respectively.
We now want these functions to include offline and suspended nodes as
well, so that we can force immediate repair when pieces are out of
placement or in excluded countries. With that change, the old names no
longer made sense.
Change-Id: Icbcbad43dbde0ca8cbc80a4d17a896bb89b078b7
Once uppon a time, at the dawn of the implementation of Storj, when all the nodes are read from the database directly, every time.
After a while -- due to performance reasons -- it has been changed for upload and download: where all the nodes are read for a short period of time, and used from memory.
This is the version which was improved recently to support advanced node selections using placement.
But stil we have an old configuration value `service.config.NodeSelectionCache.Disabled`, and the db based implementation: `service.FindStorageNodesWithPreferences(ctx, req, &service.config.Node)`.
For safety, we need to remove this option, to make sure that we use the cache, which has the advanced features.
This patch was supposed to be a very small one (just removing a method and a config: https://review.dev.storj.io/c/storj/storj/+/11074/1/satellite/overlay/service.go), but it turned out that we need to update a lot of unit tests.
These unit tests used the old implementation (which is not used in production any more).
The tests which used both implementation are just updated to use only the new one
The tests which used only the old implementation are refactored (but keeping the test cases).
Using real unit tests (without DB, working on OSX, fast)
Closes https://github.com/storj/storj/issues/6217
Change-Id: I023f92c7e34235665cf8474513e67b2fcc4763eb
It's statefull, therefore it can hit naive users. (NodeFilters couldn't be reused for more than one iterations).
But looks like we don't need it, as `SelectBySubnet` doest the same job.
Change-Id: Ie85b7f9c2bd9a47293f4e3b359f8b619215c7649
Current node selection logic (in case of using SelectBySubnet):
1. selects one subnet randomly
2. selects one node randomly from the subnet
3. applies the placement NodeFilters to the node and ignore it, if doesn't match
This logic is wrong:
1. Imagine that we have a subnet with two DE and one GB nodes.
2. We would like to select DE nodes
2. In case of GB node is selected (randomly) in step2, step3 will ignore the subnet, even if there are good (DE) nodes in there.
Change-Id: I7673f52c89b46e0cc7b20a9b74137dc689d6c17e
stats/size/count is not used by any production code, and it's not required, as we can assert the state with other checks.
real motivation: next commits will make the Selector of the State configurable, therefore we won't have one single Stat, it depends on the request parameters.
(we plan to support both network and id based randomization)
Change-Id: I631828fc0046d2fef5b7a674fc0268a0446e9655
All the files in uploadselection are (in fact) related to generic node selection, and used not only for upload,
but for download, repair, etc...
Change-Id: Ie4098318a6f8f0bbf672d432761e87047d3762ab
We use two different Node types in `overlay` and `uploadnodeselection` and converting back and forth.
Using the same object would allow us to use a unified node selection interface everywhere.
Change-Id: Ie71e29d60184ee0e5b4547eb54325f09c418f73c
Up to now, we have been implementing the DistinctIP preference with code
in two places:
1. On check-in, the last_net is determined by taking the /24 or /64
(in ResolveIPAndNetwork()) and we store it with the node record.
2. On node selection, a preference parameter defines whether to return
results that are distinct on last_net.
It can be observed that we have never yet had the need to switch from
DistinctIP to !DistinctIP, or from !DistinctIP to DistinctIP, on the
same satellite, and we will probably never need to do so in an automated
way. It can also be observed that this arrangement makes tests more
complicated, because we often have to arrange for test nodes to have IP
addresses in different /24 networks (a particular pain on macOS).
Those two considerations, plus some pending work on the repair framework
that will make repair take last_net into consideration, motivate this
change.
With this change, in the #2 place, we will _always_ return results that
are distinct on last_net. We implement the DistinctIP preference, then,
by making the #1 place (ResolveIPAndNetwork()) more flexible. When
DistinctIP is enabled, last_net will be calculated as it was before. But
when DistinctIP is _off_, last_net can be the same as address (IP and
port). That will effectively implement !DistinctIP because every
record will have a distinct last_net already.
As a side effect, this flexibility will allow us to change the rules
about last_net construction arbitrarily. We can do tests where last_net
is set to the source IP, or to a /30 prefix, or a /16 prefix, etc., and
be able to exercise the production logic without requiring a virtual
network bridge.
This change should be safe to make without any migration code, because
all known production satellite deployments use DistinctIP, and the
associated last_net values will not change for them. They will only
change for satellites with !DistinctIP, which are mostly test
deployments that can be recreated trivially. For those satellites which
are both permanent and !DistinctIP, node selection will suddenly start
acting as though DistinctIP is enabled, until the operator runs a single
SQL update "UPDATE nodes SET last_net = last_ip_port". That can be done
either before or after deploying software with this change.
I also assert that this will not hurt performance for production
deployments. It's true that adding the distinct requirement to node
selection makes things a little slower, but the distinct requirement is
already present for all production deployments, and they will see no
change.
Refs: https://github.com/storj/storj/issues/5391
Change-Id: I0e7e92498c3da768df5b4d5fb213dcd2d4862924
Create global config to specify a list of country codes that should be
excluded from node selection during uploads.
This exclusion is not implemented when the upload selection cache is
disabled.
Change-Id: Ic41e8b4f18857a11045668eac23107da99668a72
