-
Notifications
You must be signed in to change notification settings - Fork 846
Proposed parents configuration file to be used with remap.config #3870
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
proxy/config/parents.yaml.example
Outdated
| ## | ||
| # hosts.yaml | ||
| hosts: | ||
| p1: # shorthand name of host object |
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.
Do you need to have a shorthand name? If not then make the host a list.
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 yes - Looking at primary_ring above.
|
We had some discussions yesterday, so this proposal will be updated from that (soon hopefully) |
proxy/config/parents.yaml.example
Outdated
|
|
||
| strategy: | ||
| protocol: h2 # allowed protocol. optional by default uses the remapped protocol | ||
| name: # Name of the strategy: Enum of 'consistent_hash' or 'first_live' or 'rr_strict' or 'rr_ip' or 'latched' |
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.
Why can't this be an arbitrary name?
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.
this is a list of the supported selection strategies that we know about so far. Others can be added. Is that what you're asking @SolidWallOfCode
proxy/config/parents.yaml.example
Outdated
| include: [hosts.yaml] # Optional, but must specify hosts somehow | ||
|
|
||
| strategy: | ||
| protocol: h2 # allowed protocol. optional by default uses the remapped protocol |
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.
Put "upstream" or "outbound" in the key, for clarity?
proxy/config/parents.yaml.example
Outdated
| weights: # optional: weights used for the servers | ||
| p1: 1.0 | ||
| p2: 2.0 | ||
| hosts: # required list of servers |
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.
Why duplicate the host names? Can't the hosts table be derived from the weight? Can there be hosts without weights? You could make the handler accept a map or an array, the latter meaning "all the same weight".
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.
In our discussion we talked about defining all the hosts and their attributes in a separate file that could imported. Then in the strategy section which would be for a particular remap, you could choose the set of hosts you want for this remap and then choose how they should be weighted.
|
Here is the updated Parent configuration. |
|
@bryancall and @SolidWallOfCode can you guys have a look at the new file? |
|
Yeh, I like this new format. In particular, it lets someone minimize duplications, by using anchors for shared pieces, and appropriate inheritance. Like, one could take the above one step further, and isolating the protocol specifications, healthchecks etc. as an anchored resource, and then include that. Or, one could exclude the "host" portion from those anchored sections, and override that locally in the policy sections. It's a lot nicer than all the duplications that we had initially. |
|
It does bring Bryan's question back - what is the use of the short name in the primary and secondary ring definitions? |
|
@SolidWallOfCode it allows you to avoid repeating all the unchanging data about a host in every strategy file. For instance, imagine:
You could write all the unchanging data about the hosts in 1 file -- their IPs; ports and protocols (etc).
You could write all the IP/port/protocol data under the strategy.primary_ring.hosts and strategy.secondary_ring.hosts, depending if you like normalized or de-normalized things. In the duplicated scenario, if something in big3 does change, you'd need to change it in 3 places above, where you'd only have to change a single big3 entry the other way. |
|
"it allows you to avoid repeating all the unchanging data about a host in every strategy file. For instance, imagine" - but as Leif points out, YAML does this with anchors and references, as is done in the latest example configuration file. In that file, what purpose does "p1" in the "strategy" section serve? When would it be referenced instead of "*anchornameforp1"? Contrariwise, in your description, what purpose does the "hosts" table serve? It seems to me one is useful in the manner you describe, but I don't see why both are. |
3ee5f34 to
0c94164
Compare
|
I added an '#include' to the example to show that a strategy yaml file, bar.yaml, could use '#include our hosts.yaml'. so that the two files are concatenated into one yaml document. In the example yaml parser I've started on, I've implemented this. See include example |
0e4d232 to
6a7a721
Compare
…s per remap. remove example yaml parser.
2ece43b to
22f60d3
Compare
|
Will this be updated in accordance with the L7R Working Group meeting last week? |
|
Alan, I’ll update that today.
|
|
It’s been updated, see #3870
|
|
Primary and secondary group names give an impression that there are only two groups but there is no reason not to have multiple groups and transfer from group to another can happen based on failover. |
|
Sure -- just wondering how we'd configure the consistent hash feature that tells ATS to choose from group 2 if the specific chosen host in group 1 is down (as opposed to choosing another host in group 1). |
|
good point @mlibbey, I don't see anyway to configure that so that primary/secondary order either and what should the behavior be if there are three groups? Should another weight or order be added to the strategy group list as in this: strategy: Also, how are the multiple groups used with the round robin strategies? |
|
This is one of the reasons I suggested a recursive structure where a ring could have as elements other rings. Then it's easy - you define ring A to be a linear fail over of ring B (the primary pod) and ring C (the secondary). I think the structure suggested in the working group, you would have ring B and ring C, and then a strategy that said "try B, then failover to C". |
|
@mlibbey Isn't g1 primary and g2 secondary because it's a list and g1 is first? |
bc8f39e to
a300e3e
Compare
|
using the order in which the groups are in the list makes sense ie, g1 is primary because it's first. |
|
@vmamidi @mlibbey @a-canary @SolidWallOfCode there are two keys, in the example, "health_check" and "healthcheck". they have different meanings where in a 'host' context it refers to a health check url and then in strategy failover context it means either an 'active' or 'passive' health check. Is there any reason they can't be spelled the same ie, 'healthcheck' or 'health_check' to avoid confusion on which spelling is used in the differing contexts or should one be renamed entirely? Maybe just combine 'healthcheck' and 'url' in the host context to 'healcheck_url'? |
|
Is the proposed format now shown okay with everyone? |
|
[approve ci] |
|
[approve ci fedora] |
|
[approve ci rat] |
|
@bryancall, @zwoop, @vmamidi, @SolidWallOfCode I don't think we need this PR any-longer and it should be closed provided everyone is okay with #5960 and the new format of strategies.yaml |
|
@jrushford sounds good |
This PR is to add an example parents.yaml file that eventually will be used with remap.config. A new tag in remap say, @parent=parents.yaml may be used to configure parent/origin next hop on a per remap basis.
Looking for comments on this file.