Reflect review comments

cybozu-go · Sep 8, 2023 · 479e8dc · 479e8dc
1 parent be22819
commit 479e8dc
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 77 deletions.
diff --git a/metrics/collector.go b/metrics/collector.go
@@ -123,6 +123,7 @@ func (c collector) Collect(ch chan<- prometheus.Metric) {
 	wg.Wait()
 }
 
+// nodeMetricsCollector implements prometheus.Collector interface.
 type nodeMetricsCollector struct {
 	storage storage
 }
@@ -141,11 +142,17 @@ func (c nodeMetricsCollector) Collect(ch chan<- prometheus.Metric) {
 
 	rqEntries, err := c.storage.GetRebootsEntries(ctx)
 	if err != nil {
+		log.Error("failed to get reboots entries", map[string]interface{}{
+			log.FnError: err,
+		})
 		return
 	}
 
 	cluster, err := c.storage.GetCluster(ctx)
 	if err != nil {
+		log.Error("failed to get reboots entries", map[string]interface{}{
+			log.FnError: err,
+		})
 		return
 	}
 	itemCounts := cke.CountRebootQueueEntries(rqEntries)

diff --git a/metrics/metrics.go b/metrics/metrics.go
@@ -34,21 +34,21 @@ var operationPhaseTimestampSeconds = prometheus.NewGauge(
 )
 
 var rebootQueueEntries = prometheus.NewDesc(
-	namespace+"_reboot_queue_entries",
+	prometheus.BuildFQName(namespace, "", "reboot_queue_entries"),
 	"The number of reboot queue entries remaining.",
 	nil,
 	nil,
 )
 
 var rebootQueueItems = prometheus.NewDesc(
-	namespace+"_reboot_queue_items",
+	prometheus.BuildFQName(namespace, "", "reboot_queue_items"),
 	"The number of reboot queue entries remaining per status.",
 	[]string{"status"},
 	nil,
 )
 
 var nodeRebootStatus = prometheus.NewDesc(
-	namespace+"_node_reboot_status",
+	prometheus.BuildFQName(namespace, "", "node_reboot_status"),
 	"The reboot status of a node.",
 	[]string{"node", "status"},
 	nil,

diff --git a/metrics/updater_test.go b/metrics/updater_test.go
@@ -258,21 +258,15 @@ func testUpdateRebootQueueEntries(t *testing.T) {
 		{
 			name: "one",
 			input: []*cke.RebootQueueEntry{
-				{
-					Status: cke.RebootStatusQueued,
-				},
+				{Status: cke.RebootStatusQueued},
 			},
 			expected: 1,
 		},
 		{
 			name: "two",
 			input: []*cke.RebootQueueEntry{
-				{
-					Status: cke.RebootStatusQueued,
-				},
-				{
-					Status: cke.RebootStatusRebooting,
-				},
+				{Status: cke.RebootStatusQueued},
+				{Status: cke.RebootStatusRebooting},
 			},
 			expected: 2,
 		},
@@ -320,24 +314,12 @@ func testUpdateRebootQueueItems(t *testing.T) {
 		{
 			name: "zero",
 			input: []*cke.RebootQueueEntry{
-				{
-					Status: cke.RebootStatusQueued,
-				},
-				{
-					Status: cke.RebootStatusDraining,
-				},
-				{
-					Status: cke.RebootStatusDraining,
-				},
-				{
-					Status: cke.RebootStatusRebooting,
-				},
-				{
-					Status: cke.RebootStatusRebooting,
-				},
-				{
-					Status: cke.RebootStatusRebooting,
-				},
+				{Status: cke.RebootStatusQueued},
+				{Status: cke.RebootStatusDraining},
+				{Status: cke.RebootStatusDraining},
+				{Status: cke.RebootStatusRebooting},
+				{Status: cke.RebootStatusRebooting},
+				{Status: cke.RebootStatusRebooting},
 			},
 			expected: map[string]float64{
 				"queued":    1.0,
@@ -349,51 +331,21 @@ func testUpdateRebootQueueItems(t *testing.T) {
 		{
 			name: "one",
 			input: []*cke.RebootQueueEntry{
-				{
-					Status: cke.RebootStatusQueued,
-				},
-				{
-					Status: cke.RebootStatusQueued,
-				},
-				{
-					Status: cke.RebootStatusQueued,
-				},
-				{
-					Status: cke.RebootStatusQueued,
-				},
-				{
-					Status: cke.RebootStatusDraining,
-				},
-				{
-					Status: cke.RebootStatusDraining,
-				},
-				{
-					Status: cke.RebootStatusDraining,
-				},
-				{
-					Status: cke.RebootStatusDraining,
-				},
-				{
-					Status: cke.RebootStatusDraining,
-				},
-				{
-					Status: cke.RebootStatusCancelled,
-				},
-				{
-					Status: cke.RebootStatusCancelled,
-				},
-				{
-					Status: cke.RebootStatusCancelled,
-				},
-				{
-					Status: cke.RebootStatusCancelled,
-				},
-				{
-					Status: cke.RebootStatusCancelled,
-				},
-				{
-					Status: cke.RebootStatusCancelled,
-				},
+				{Status: cke.RebootStatusQueued},
+				{Status: cke.RebootStatusQueued},
+				{Status: cke.RebootStatusQueued},
+				{Status: cke.RebootStatusQueued},
+				{Status: cke.RebootStatusDraining},
+				{Status: cke.RebootStatusDraining},
+				{Status: cke.RebootStatusDraining},
+				{Status: cke.RebootStatusDraining},
+				{Status: cke.RebootStatusDraining},
+				{Status: cke.RebootStatusCancelled},
+				{Status: cke.RebootStatusCancelled},
+				{Status: cke.RebootStatusCancelled},
+				{Status: cke.RebootStatusCancelled},
+				{Status: cke.RebootStatusCancelled},
+				{Status: cke.RebootStatusCancelled},
 			},
 			expected: map[string]float64{
 				"queued":    4.0,

diff --git a/sabakan/generator.go b/sabakan/generator.go
@@ -52,8 +52,6 @@ func MachineToNode(m *Machine, tmpl *cke.Node) *cke.Node {
 	n.Labels["cke.cybozu.com/rack"] = strconv.Itoa(m.Spec.Rack)
 	n.Labels["cke.cybozu.com/index-in-rack"] = strconv.Itoa(m.Spec.IndexInRack)
 	n.Labels["cke.cybozu.com/role"] = m.Spec.Role
-	n.Labels["cke.cybozu.com/retire-date"] = m.Spec.RetireDate.Format("2006-01")
-	n.Labels["cke.cybozu.com/register-date"] = m.Spec.RegisterDate.Format("2006-01")
 	n.Labels["node-role.kubernetes.io/"+m.Spec.Role] = "true"
 	if n.ControlPlane {
 		n.Labels["node-role.kubernetes.io/master"] = "true"