From 479e8dc640dff49d007f38ec3204a6925cffd234 Mon Sep 17 00:00:00 2001
From: zoetrope <a.ikezoe@gmail.com>
Date: Fri, 8 Sep 2023 13:45:48 +0900
Subject: [PATCH] Reflect review comments

---
 metrics/collector.go    |  7 +++
 metrics/metrics.go      |  6 +--
 metrics/updater_test.go | 96 +++++++++++------------------------------
 sabakan/generator.go    |  2 -
 4 files changed, 34 insertions(+), 77 deletions(-)

diff --git a/metrics/collector.go b/metrics/collector.go
index c7b32d563..108d69e93 100644
--- 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
index e8b0c28ba..4737a00a7 100644
--- 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
index 54670c687..d824c3511 100644
--- 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
index b28128406..4ddf51941 100644
--- 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"