Repository: bacalhau-project/bacalhau (language: python)

Issue:
Fix labelling in NewNode/NewComputeNode/NewRequesterNode
The NodeLabelProvider is complex in that different paths through the code collect data from different places, and in one place we pass a label provider through to the compute node, then fetch it again and create a new labelprovider.

The result of this is that labels are not always assigned to the correct places.  Rather than try to create one label provider and extend it and pass it around, we should create a label provider for each node type with shared inputs. 

Currently when using NATS, requester nodes have the configured labels and the compute nodes the generated ones (but no configured labels).


Discussion / hints:

Fix patch:
diff --git a/pkg/node/compute.go b/pkg/node/compute.go
index 5d9adcd3b3..ff56dcecd1 100644
--- a/pkg/node/compute.go
+++ b/pkg/node/compute.go
@@ -57,6 +57,7 @@ func NewComputeNode(
 	publishers publisher.PublisherProvider,
 	computeCallback compute.Callback,
 	managementProxy compute.ManagementEndpoint,
+	configuredLabels map[string]string,
 ) (*Compute, error) {
 	executionStore := config.ExecutionStore
 
@@ -227,6 +228,7 @@ func NewComputeNode(
 
 	// Node labels
 	labelsProvider := models.MergeLabelsInOrder(
+		&ConfigLabelsProvider{staticLabels: configuredLabels},
 		&RuntimeLabelsProvider{},
 		capacity.NewGPULabelsProvider(config.TotalResourceLimits),
 		repo_storage.NewLabelsProvider(),
diff --git a/pkg/node/node.go b/pkg/node/node.go
index db8cca135d..736eeaf636 100644
--- a/pkg/node/node.go
+++ b/pkg/node/node.go
@@ -261,7 +261,10 @@ func NewNode(
 
 	var requesterNode *Requester
 	var computeNode *Compute
-	var labelsProvider models.LabelsProvider = &ConfigLabelsProvider{staticLabels: config.Labels}
+	labelsProvider := models.MergeLabelsInOrder(
+		&ConfigLabelsProvider{staticLabels: config.Labels},
+		&RuntimeLabelsProvider{},
+	)
 
 	// setup requester node
 	if config.IsRequesterNode {
@@ -353,6 +356,7 @@ func NewNode(
 			publishers,
 			transportLayer.CallbackProxy(),
 			transportLayer.ManagementProxy(),
+			config.Labels,
 		)
 		if err != nil {
 			return nil, err
@@ -363,17 +367,16 @@ func NewNode(
 			return nil, err
 		}
 
-		labelsProvider = models.MergeLabelsInOrder(
-			computeNode.autoLabelsProvider,
-			labelsProvider,
-		)
 		debugInfoProviders = append(debugInfoProviders, computeNode.debugInfoProviders...)
 	}
 
+	// Create a node info provider for LibP2P, and specify the default node approval state
+	// of Approved to avoid confusion as approval state is not used for this transport type.
 	nodeInfoProvider := routing.NewNodeInfoProvider(routing.NodeInfoProviderParams{
-		NodeID:          config.NodeID,
-		LabelsProvider:  labelsProvider,
-		BacalhauVersion: *version.Get(),
+		NodeID:              config.NodeID,
+		LabelsProvider:      labelsProvider,
+		BacalhauVersion:     *version.Get(),
+		DefaultNodeApproval: models.NodeApprovals.APPROVED,
 	})
 	nodeInfoProvider.RegisterNodeInfoDecorator(transportLayer.NodeInfoDecorator())
 	if computeNode != nil {
diff --git a/pkg/routing/node_info_provider.go b/pkg/routing/node_info_provider.go
index 5c65d7e307..507d7fedbc 100644
--- a/pkg/routing/node_info_provider.go
+++ b/pkg/routing/node_info_provider.go
@@ -7,25 +7,35 @@ import (
 )
 
 type NodeInfoProviderParams struct {
-	NodeID          string
-	LabelsProvider  models.LabelsProvider
-	BacalhauVersion models.BuildVersionInfo
+	NodeID              string
+	LabelsProvider      models.LabelsProvider
+	BacalhauVersion     models.BuildVersionInfo
+	DefaultNodeApproval models.NodeApproval
 }
 
 type NodeInfoProvider struct {
-	nodeID             string
-	labelsProvider     models.LabelsProvider
-	bacalhauVersion    models.BuildVersionInfo
-	nodeInfoDecorators []models.NodeInfoDecorator
+	nodeID              string
+	labelsProvider      models.LabelsProvider
+	bacalhauVersion     models.BuildVersionInfo
+	nodeInfoDecorators  []models.NodeInfoDecorator
+	defaultNodeApproval models.NodeApproval
 }
 
 func NewNodeInfoProvider(params NodeInfoProviderParams) *NodeInfoProvider {
-	return &NodeInfoProvider{
-		nodeID:             params.NodeID,
-		labelsProvider:     params.LabelsProvider,
-		bacalhauVersion:    params.BacalhauVersion,
-		nodeInfoDecorators: make([]models.NodeInfoDecorator, 0),
+	provider := &NodeInfoProvider{
+		nodeID:              params.NodeID,
+		labelsProvider:      params.LabelsProvider,
+		bacalhauVersion:     params.BacalhauVersion,
+		nodeInfoDecorators:  make([]models.NodeInfoDecorator, 0),
+		defaultNodeApproval: params.DefaultNodeApproval,
 	}
+
+	// If we were not given a default approval, we default to PENDING
+	if !provider.defaultNodeApproval.IsValid() {
+		provider.defaultNodeApproval = models.NodeApprovals.PENDING
+	}
+
+	return provider
 }
 
 // RegisterNodeInfoDecorator registers a node info decorator with the node info provider.
@@ -39,11 +49,16 @@ func (n *NodeInfoProvider) GetNodeInfo(ctx context.Context) models.NodeInfo {
 		BacalhauVersion: n.bacalhauVersion,
 		Labels:          n.labelsProvider.GetLabels(ctx),
 		NodeType:        models.NodeTypeRequester,
-		Approval:        models.NodeApprovals.PENDING,
+		Approval:        n.defaultNodeApproval,
 	}
 	for _, decorator := range n.nodeInfoDecorators {
 		res = decorator.DecorateNodeInfo(ctx, res)
 	}
+
+	if !res.Approval.IsValid() {
+		res.Approval = models.NodeApprovals.PENDING
+	}
+
 	return res
 }
 


Test patch:
diff --git a/pkg/test/compute/setup_test.go b/pkg/test/compute/setup_test.go
index c051cebf2c..f5ecceae33 100644
--- a/pkg/test/compute/setup_test.go
+++ b/pkg/test/compute/setup_test.go
@@ -116,7 +116,8 @@ func (s *ComputeSuite) setupNode() {
 		provider.NewNoopProvider[executor.Executor](s.executor),
 		provider.NewNoopProvider[publisher.Publisher](s.publisher),
 		callback,
-		nil, // until we switch to testing with NATS
+		nil,                 // until we switch to testing with NATS
+		map[string]string{}, // empty configured labels
 	)
 	s.NoError(err)
 	s.stateResolver = *resolver.NewStateResolver(resolver.StateResolverParams{