Remove TODOs, add metrics error when node name is an IP:Port combo.

pkg/costmodel/containerkeys.go

@@ -2,6 +2,8 @@ package costmodel
 
 import (
 	"errors"
+	"net"
+	"strconv"
 	"strings"
 
 	"github.com/opencost/opencost/core/pkg/log"
@@ -23,6 +25,11 @@ var (
 	ErrNoNamespaceName error = errors.New("vector does not have string namespace")
 	ErrNoNodeName      error = errors.New("vector does not have string node")
 	ErrNoClusterID     error = errors.New("vector does not have string cluster id")
+	ErrIpPortNodeName  error = errors.New(`node name is an IP:PORT combo, not a node name. cAdvisor scrape configs require the following:
+	relabel_configs:
+	- action: labelmap
+	  regex: __meta_kubernetes_node_name
+	  replacement: node`)
 )
 
 //--------------------------------------------------------------------------
@@ -212,6 +219,10 @@ func NewContainerMetricFrom(result *source.ContainerMetricResult, defaultCluster
 	}
 
 	nodeName := result.Node
+	if isIpPortCombo(nodeName) {
+		return nil, ErrIpPortNodeName
+	}
+
 	if nodeName == "" {
 		log.Debugf("metric vector does not have node name")
 		nodeName = ""
@@ -231,3 +242,25 @@ func NewContainerMetricFrom(result *source.ContainerMetricResult, defaultCluster
 		key:           containerMetricKey(namespace, podName, containerName, nodeName, clusterID),
 	}, nil
 }
+
+func isIpPortCombo(value string) bool {
+	// handle IPv6 values as well
+	idx := strings.LastIndex(value, ":")
+	if idx == -1 {
+		return false
+	}
+
+	ipStr, portStr := value[:idx], value[idx+1:]
+
+	ip := net.ParseIP(ipStr)
+	if ip == nil {
+		return false
+	}
+
+	port, err := strconv.Atoi(portStr)
+	if err != nil {
+		return false
+	}
+
+	return port > 0 && port <= 65535
+}

pkg/costmodel/costmodel_test.go

@@ -10,6 +10,41 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
+func TestIpPortCombo(t *testing.T) {
+	tests := []string{
+		"192.168.1.1:80",
+		"10.0.0.1:443",
+		"127.0.0.1:8080",
+		"172.16.254.1:22",
+		"0.0.0.0:5000",
+		"::1:80",
+		"2001:db8::1:443",
+		"2001:0db8:85a3:0000:0000:8a2e:0370:7334:8080",
+		"fe80::1:22",
+		"10.1.2.3:10240",
+		":::80",
+	}
+
+	for _, test := range tests {
+		result := isIpPortCombo(test)
+		if !result {
+			t.Errorf("Expected %s to be a valid IP:Port combo", test)
+		}
+	}
+
+	fails := []string{
+		"foo:bar",
+		"ip-10-1-2-3.ec2.internal",
+	}
+
+	for _, fail := range fails {
+		result := isIpPortCombo(fail)
+		if result {
+			t.Errorf("Expected %s to be an invalid IP:Port combo", fail)
+		}
+	}
+}
+
 func TestGetGPUCount(t *testing.T) {
 	tests := []struct {
 		name          string

pkg/costmodel/metrics.go

@@ -679,7 +679,6 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 					Parameters: parameters,
 				}
 
-				// TODO: GetPVCost should be a method in CostModel?
 				cmme.Model.GetPVCost(cacPv, pv, region)
 				c, _ := strconv.ParseFloat(cacPv.Cost, 64)
 				cmme.PersistentVolumePriceRecorder.WithLabelValues(pv.Name, pv.Name, cacPv.ProviderID).Set(c)