Local publisher address tf #3660
Conversation
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
ops/terraform/main.tf
Outdated
| export BACALHAU_NODE_COMPUTE_LOCAL_PUBLISHER_ADDRESS="${var.public_ip_addresses[count.index]}" | ||
|
|
There was a problem hiding this comment.
You won't have to set cli flag if you used the right env variable BACALHAU_LOCAL_PUBLISHER_ADDRESS="${var.public_ip_addresses[count.index]}"
| @@ -43,7 +43,8 @@ if [[ "${BACALHAU_NODE_NETWORK_TYPE}" == "nats" ]]; then | |||
| --web-ui-port 80 \ | |||
| --labels owner=bacalhau \ | |||
| --requester-job-translation-enabled \ | |||
| --default-publisher ipfs | |||
| --default-publisher local \ | |||
| --local-publisher-address "${BACALHAU_LOCAL_PUBLISHER_ADDRESS}" | |||
There was a problem hiding this comment.
nit: redundant to pass the cli flag as BACALHAU_LOCAL_PUBLISHER_ADDRESS env variable translates to the same config
There was a problem hiding this comment.
I don't think this flag actually expects an IP address, it expects one of these values: https://github.com/bacalhau-project/bacalhau/blob/main/pkg/lib/network/network.go#L111
via: https://github.com/bacalhau-project/bacalhau/blob/main/pkg/publisher/local/publisher.go#L31 -> https://github.com/bacalhau-project/bacalhau/blob/main/pkg/publisher/local/publisher.go#L97
There was a problem hiding this comment.
No it can be an IP address as well. The problem here is that 'public' won't resolve to anything for google cloud as it doesn't give out public addresses to the VMs at runtime.
There was a problem hiding this comment.
Have pushed a code change, will redeploy to staging this evening.
rossjones
force-pushed
the
local-publisher-address-tf
branch
from
2da4203 to
05ff043
Compare
| func ResolveAddress(ctx context.Context, address string) string { | ||
| addressType, ok := network.AddressTypeFromString(address) | ||
| if !ok { | ||
| log.Ctx(ctx).Debug().Stringer("AddressType", addressType).Msgf("unable to find address type: %s, binding to 0.0.0.0", address) | ||
| return "0.0.0.0" | ||
| } | ||
|
|
||
| // If we were provided with an address type and not an address, so we should look up | ||
| // an address from the type. | ||
| addrs, err := network.GetNetworkAddress(addressType, network.AllAddresses) | ||
| if err == nil && len(addrs) > 0 { | ||
| return addrs[0] | ||
| if ok { | ||
| addrs, err := network.GetNetworkAddress(addressType, network.AllAddresses) | ||
| if err == nil && len(addrs) > 0 { | ||
| return addrs[0] | ||
| } else { | ||
| log.Ctx(ctx).Error().Err(err).Msg("failed to resolve network address by type, using 127.0.0.1") | ||
| return "127.0.0.1" | ||
| } | ||
| } | ||
|
|
||
| log.Ctx(ctx).Error().Err(err).Stringer("AddressType", addressType).Msgf("unable to find address for type, using 127.0.0.1") | ||
| return "127.0.0.1" | ||
| return address |
There was a problem hiding this comment.
This just tries to resolve a name (public etc) to an address, and leaves IP addresses untouched. We might want to re-assess whether the name (public, local, etc) lookup is worthwhile as it doesn't solve the actual problem of potentially not having a public address.
rossjones
force-pushed
the
local-publisher-address-tf
branch
from
05ff043 to
372be9a
Compare
Now the server for the local publisher will listen on all interfaces, but the publisher itself will advertise on the configured address.
rossjones
force-pushed
the
local-publisher-address-tf
branch
from
372be9a to
485cf14
Compare
Set the address to use for the local publisher explicitly as a command line parameter, and this is used for the link that is generated in the response. The actual publisher server will bind to all interfaces as we don't know which (if any) are actually the public address.