Skip to content
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.

Sign up for GitHub

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

Local publisher address tf #3660

Merged
merged 5 commits into from
Mar 20, 2024
+39 −29
Merged

Local publisher address tf #3660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
816ee7d
Configurable IP address
rossjones Mar 19, 2024
67316fb
Update values
rossjones Mar 19, 2024
c2f8733
ensure publisher accepts non-name address
rossjones Mar 19, 2024
a89cb64
Handle error case with address lookup
rossjones Mar 19, 2024
485cf14
Local publisher server listen on all interfaces
rossjones Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions ops/terraform/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export BACALHAU_NODE_NETWORK_TYPE=${var.network_type}
export BACALHAU_NODE_NETWORK_ORCHESTRATORS="${var.internal_ip_addresses[0]}:4222"
export BACALHAU_NODE_NETWORK_ADVERTISEDADDRESS="${var.public_ip_addresses[count.index]}:4222"
export BACALHAU_NODE_NETWORK_CLUSTER_PEERS=""
export BACALHAU_LOCAL_PUBLISHER_ADDRESS="${var.public_ip_addresses[count.index]}"


### secrets are installed in the install-node.sh script
export SECRETS_GRAFANA_CLOUD_PROMETHEUS_API_KEY="${var.grafana_cloud_prometheus_api_key}"
Expand Down Expand Up @@ -204,8 +206,8 @@ EOF

resource "google_compute_address" "ipv4_address" {
region = var.region
name = "bacalhau-ipv4-address-${terraform.workspace}-${count.index}"
count = var.protect_resources ? var.instance_count : 0
name = "bacalhau-ipv4-address-${terraform.workspace}-${count.index}"
count = var.protect_resources ? var.instance_count : 0
lifecycle {
prevent_destroy = true
}
Expand Down Expand Up @@ -308,8 +310,8 @@ resource "google_compute_firewall" "bacalhau_ingress_firewall" {
allow {
protocol = "udp"
ports = [
"4001", // ipfs swarm
"1235", // bacalhau swarm
"4001", // ipfs swarm
"1235", // bacalhau swarm
]
}

Expand All @@ -325,18 +327,18 @@ resource "google_compute_firewall" "bacalhau_egress_firewall" {
allow {
protocol = "tcp"
ports = [
"4001", // ipfs swarm
"1235", // bacalhau swarm
"4222", // nats
"6222", // nats cluster
"4001", // ipfs swarm
"1235", // bacalhau swarm
"4222", // nats
"6222", // nats cluster
]
}

allow {
protocol = "udp"
ports = [
"4001", // ipfs swarm
"1235", // bacalhau swarm
"4001", // ipfs swarm
"1235", // bacalhau swarm
]
}

Expand Down
6 changes: 4 additions & 2 deletions ops/terraform/remote_files/scripts/start-bacalhau.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: redundant to pass the cli flag as BACALHAU_LOCAL_PUBLISHER_ADDRESS env variable translates to the same config

Copy link
Contributor

@frrist frrist Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have pushed a code change, will redeploy to staging this evening.


else
function getMultiaddress() {
Expand Down Expand Up @@ -97,5 +98,6 @@ else
--web-ui-port 80 \
--labels owner=bacalhau \
--requester-job-translation-enabled \
--default-publisher ipfs
--default-publisher local \
--local-publisher-address "${BACALHAU_LOCAL_PUBLISHER_ADDRESS}"
fi
27 changes: 12 additions & 15 deletions pkg/publisher/local/publisher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ type Publisher struct {
func NewLocalPublisher(ctx context.Context, directory string, host string, port int) *Publisher {
p := &Publisher{
baseDirectory: directory,
host: resolveAddress(ctx, host),
host: ResolveAddress(ctx, host),
port: port,
urlPrefix: fmt.Sprintf("http://%s:%d", host, port),
}

p.server = NewLocalPublisherServer(ctx, p.baseDirectory, p.host, p.port)
p.server = NewLocalPublisherServer(ctx, p.baseDirectory, p.port)
go p.server.Run(ctx)

return p
Expand Down Expand Up @@ -93,20 +93,17 @@ func (p *Publisher) PublishResult(

var _ publisher.Publisher = (*Publisher)(nil)

func resolveAddress(ctx context.Context, address string) string {
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
Comment on lines +96 to +108
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
8 changes: 8 additions & 0 deletions pkg/publisher/local/publisher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ func (s *PublisherTestSuite) SetupTest() {
s.pub = local.NewLocalPublisher(s.ctx, s.baseDir, defaultHost, defaultPort)
}

func (s *PublisherTestSuite) TestAddressResolving() {
ctx := context.Background()

s.Require().Equal("127.0.0.1", local.ResolveAddress(ctx, "127.0.0.1"), "address did not resolve to itself")
s.Require().Equal("192.168.1.100", local.ResolveAddress(ctx, "192.168.1.100"), "address did not resolve to itself")
s.Require().Equal("127.0.0.1", local.ResolveAddress(ctx, "local"))
}

func (s *PublisherTestSuite) TestPublishFolder() {
source := s.T().TempDir()

Expand Down
5 changes: 3 additions & 2 deletions pkg/publisher/local/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ type LocalPublisherServer struct {
const (
readHeaderTimeout = 3 * time.Second
readTimeout = 3 * time.Second
allAddresses = "0.0.0.0"
)

func NewLocalPublisherServer(ctx context.Context, directory, address string, port int) *LocalPublisherServer {
func NewLocalPublisherServer(ctx context.Context, directory string, port int) *LocalPublisherServer {
return &LocalPublisherServer{
rootDirectory: directory,
address: address,
address: allAddresses, // we listen on all addresses
port: port,
}
}
Expand Down
Loading