broker: use enclosing instance tbon.interface-hint unless overridden #6283
Conversation
|
The thing that's bugging me about this solution is it adds a synchronous RPC (as well as a One tactic to reduce the latency would be to pre-populate a PMI key like I'll mark this WIP pending some feedback on the approach. |
garlick
changed the title
|
Reworked to transfer the enclosing instance's |
garlick
changed the title
Problem: the enclosing instance tbon.interface-hint attribute is needed when flux launches flux. Set it in the PMI kvs under the flux.tbon-interface-hint key.
Problem: adding support for reading flux.tbon-interface-hint from the PMI KVS is awkward due to the structure of an internal function. Refactor set_instance_level_attr() so that it returns a boolean 'under_flux' flag indicating whether this instance was launched by Flux. Use that that flag to determine whether to set the jobid attr. The flag will also be useful for gating whether to check for the interface hint in the PMI KVS.
Problem: we don't have a way to configure subinstances to use a particular network, other than setting the global environment variable FLUX_IPADDR_INTERFACE. Change the way the default tbon.interface-hint is set. Instead of hard coding "default-route", use the value that is set in the enclosing instance's PMI KVS, if any. This would allow a system instance to configure tbon.interface-hint and batch/alloc jobs that it starts would use that value unless overridden. If the value is not configured in the system instance, then the attribute will have the default value which will be inherited by the child. Fixes flux-framework#6272
| @@ -447,19 +447,19 @@ test_expect_success 'FLUX_IPADDR_INTERFACE=lo works' ' | |||
| flux getattr tbon.endpoint >endpoint3.out && | |||
| grep "127.0.0.1" endpoint3.out | |||
| ' | |||
| test_expect_success 'tcp.interface-hint=lo works' ' | |||
There was a problem hiding this comment.
very minor commit message nit: 'use the "tcp.interface-hint" in the test titles' drop the the or add "string" or "label"?
| flux start -o,-Stbon.interface-hint=hostname \ | ||
| flux alloc -N1 flux getattr tbon.interface-hint >childhint.out && | ||
| grep hostname childhint.out | ||
| ' |
There was a problem hiding this comment.
Should a test be added that ensures setting the attribute in the subinstance uses the set value and not the inherited one?
Problem: tests recently added to t0001-basic.t for tbon.interface-hint use the wrong attribute name in the test titles. Fix test titles.
Problem: t0001-basic.t takes too long to run. Move the tbon config related tests to a new script called t0019-tbon-config.t.
Problem: There is no test coverage for tbon.interface-hint inheritance from enclosing instance. Add a test.
Problem: the description of tbon.interface-hint does not mention that the default value is taken from the enclosing instance, if any. Rework the description of this attribute.
Problem: the description of tbon.interface hint does not mention the other ways it can be set. Note that the broker attribute overrides and add a pointer to flux-broker-attributes(7) where the other ways of setting it are mentioned.
Problem: a test expects to be terminated but sometimes it just fails with a non-zero exit code. Use test_must_fail_or_be_terminated.
|
Thanks! I addressed those comments and forced a push. Will set MWP. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6283 +/- ##
==========================================
- Coverage 83.33% 83.32% -0.02%
==========================================
Files 522 522
Lines 86057 86093 +36
==========================================
+ Hits 71717 71738 +21
- Misses 14340 14355 +15
|
Problem: we don't have a way to configure subinstances to use a particular network, other than setting the global environment variable FLUX_IPADDR_INTERFACE.
Change the way the default
tbon.interface-hintis set. Instead of hard coding "default-route", use the value that is set in the enclosing instance, if any.This would allow a system instance to configure
tbon.interface-hintand batch/alloc jobs that it starts would use that value unless overridden. If the value is not configured, then the attribute will have the default value which will be inherited by the child.